argentite added inline comments.
================ Comment at: clang/include/clang/Interpreter/Interpreter.h:62 const CompilerInstance *getCompilerInstance() const; - const llvm::orc::LLJIT *getExecutionEngine() const; + llvm::Expected<llvm::orc::LLJIT &> GetExecutionEngine(); + ---------------- sgraenitz wrote: > Do we really want these functions capitalized? IIRC in the clang code-base > they are used to highlight core API functions. It looks like these two > functions are only used as helpers inside `Interpreter` itself. I guess we can keep them without capitalizing. ================ Comment at: clang/lib/Interpreter/Interpreter.cpp:223 +llvm::Error Interpreter::CreateExecutor() { + const clang::TargetInfo &TI = ---------------- sgraenitz wrote: > Factoring out this function looks like an independent change. Is it related > to the load-library command in any way? The `IncrementalExecutor` was being created on the first call to `Execute()`. It is created lazily for `-fsyntax-only` support. It holds the Execution Engine that we need for loading the library. So we need to be able to create it in `getExecutionEngine()` as well because a library can be loaded before any code is executed. ================ Comment at: clang/tools/clang-repl/ClangRepl.cpp:126 } + if (Line->rfind(R"(%lib )", 0) == 0) { + if (auto Err = Interp->LoadDynamicLibrary(Line->data() + 5)) { ---------------- sgraenitz wrote: > I see this is analog to existing code, but is there a good reason for using > the multi-line string syntax here? Yeah I am confused as well. But I tried to maintain the style. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141824/new/ https://reviews.llvm.org/D141824 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits