[clang-repl] Put CompilerInstance from IncrementalAction to use for non-assert/assert builds

[anutosh491]

See #137458 (comment)

Context: So the CompilerInstance CI being used with the Incremental Action are tightly coupled in any case. Which means
the CI put to use while constructing the IncrementalAction

  Act = TSCtx->withContextDo([&](llvm::LLVMContext *Ctx) {
    return std::make_unique<IncrementalAction>(*CI, *Ctx, ErrOut, *this,
                                               std::move(Consumer));
  });                      

Is also the CI through which the we call ExecuteAction on Act

CI->ExecuteAction(*Act);

So we need to use CI as a member variable in IncrementalAction for assert builds for

llvm-project/clang/lib/Interpreter/IncrementalAction.cpp

Lines 97 to 108 in bddac5e

	std::unique_ptr<llvm::Module> IncrementalAction::GenModule() {
	static unsigned ID = 0;
	if (CodeGenerator *CG = getCodeGen()) {
	// Clang's CodeGen is designed to work with a single llvm::Module. In many
	// cases for convenience various CodeGen parts have a reference to the
	// llvm::Module (TheModule or Module) which does not change when a new
	// module is pushed. However, the execution engine wants to take ownership
	// of the module which does not map well to CodeGen's design. To work this
	// around we created an empty module to make CodeGen happy. We should make
	// sure it always stays empty.
	assert(((!CachedInCodeGenModule ||
	!CI.getPreprocessorOpts().Includes.empty()) ||

The same can be put to use for CreateASTConsumer too as all of these are referring to the same CI

std::unique_ptr<ASTConsumer>
IncrementalAction::CreateASTConsumer(CompilerInstance & /*CI*/, StringRef InFile) {
  std::unique_ptr<ASTConsumer> C =
      WrapperFrontendAction::CreateASTConsumer(this->CI, InFile);
      

[llvmbot]

@llvm/pr-subscribers-clang

Author: Anutosh Bhat (anutosh491)

Changes

See #137458 (comment)

---

Full diff: https://github.com/llvm/llvm-project/pull/155400.diff

2 Files Affected:

• (modified) clang/lib/Interpreter/IncrementalAction.cpp (+7-7)
• (modified) clang/lib/Interpreter/IncrementalAction.h (+1-1)

diff --git a/clang/lib/Interpreter/IncrementalAction.cpp b/clang/lib/Interpreter/IncrementalAction.cpp
index 67313a8cd2a8c..bf804bd24a959 100644
--- a/clang/lib/Interpreter/IncrementalAction.cpp
+++ b/clang/lib/Interpreter/IncrementalAction.cpp
@@ -22,25 +22,25 @@
 #include "llvm/Support/ErrorHandling.h"
 
 namespace clang {
-IncrementalAction::IncrementalAction(CompilerInstance &CI,
+IncrementalAction::IncrementalAction(CompilerInstance &Instance,
                                      llvm::LLVMContext &LLVMCtx,
                                      llvm::Error &Err, Interpreter &I,
                                      std::unique_ptr<ASTConsumer> Consumer)
     : WrapperFrontendAction([&]() {
         llvm::ErrorAsOutParameter EAO(&Err);
         std::unique_ptr<FrontendAction> Act;
-        switch (CI.getFrontendOpts().ProgramAction) {
+        switch (Instance.getFrontendOpts().ProgramAction) {
         default:
           Err = llvm::createStringError(
               std::errc::state_not_recoverable,
               "Driver initialization failed. "
               "Incremental mode for action %d is not supported",
-              CI.getFrontendOpts().ProgramAction);
+              Instance.getFrontendOpts().ProgramAction);
           return Act;
         case frontend::ASTDump:
         case frontend::ASTPrint:
         case frontend::ParseSyntaxOnly:
-          Act = CreateFrontendAction(CI);
+          Act = CreateFrontendAction(Instance);
           break;
         case frontend::PluginAction:
         case frontend::EmitAssembly:
@@ -53,12 +53,12 @@ IncrementalAction::IncrementalAction(CompilerInstance &CI,
         }
         return Act;
       }()),
-      Interp(I), CI(CI), Consumer(std::move(Consumer)) {}
+      Interp(I), CI(Instance), Consumer(std::move(Consumer)) {}
 
 std::unique_ptr<ASTConsumer>
-IncrementalAction::CreateASTConsumer(CompilerInstance &CI, StringRef InFile) {
+IncrementalAction::CreateASTConsumer(CompilerInstance & /*CI*/, StringRef InFile) {
   std::unique_ptr<ASTConsumer> C =
-      WrapperFrontendAction::CreateASTConsumer(CI, InFile);
+      WrapperFrontendAction::CreateASTConsumer(this->CI, InFile);
 
   if (Consumer) {
     std::vector<std::unique_ptr<ASTConsumer>> Cs;
diff --git a/clang/lib/Interpreter/IncrementalAction.h b/clang/lib/Interpreter/IncrementalAction.h
index 92eabacd40074..83cec24caf274 100644
--- a/clang/lib/Interpreter/IncrementalAction.h
+++ b/clang/lib/Interpreter/IncrementalAction.h
@@ -42,7 +42,7 @@ class IncrementalAction : public WrapperFrontendAction {
   std::unique_ptr<llvm::Module> CachedInCodeGenModule;
 
 public:
-  IncrementalAction(CompilerInstance &CI, llvm::LLVMContext &LLVMCtx,
+  IncrementalAction(CompilerInstance &Instance, llvm::LLVMContext &LLVMCtx,
                     llvm::Error &Err, Interpreter &I,
                     std::unique_ptr<ASTConsumer> Consumer = nullptr);
 

[anutosh491]

This way

1. Code is more readable (no shadowing)
2. The warning should go away.

[anutosh491]

Thanks @mikaelholmen for the report.

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[anutosh491]

The failing test seems unrelated

  Failed Tests (1):
    lldb-unit :: DAP/./DAPTests/DisconnectRequestHandlerTest/DisconnectTriggersTerminateCommands
    

[vgvassilev]

LGTM!

[anutosh491]

Thanks for the review. Merging !