[clang] [clang-repl] Include consistency using the default clang actions. (PR #116610)
via cfe-commits
cfe-commits at lists.llvm.org
Mon Nov 18 05:23:05 PST 2024
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang
Author: Vassil Vassilev (vgvassilev)
<details>
<summary>Changes</summary>
This patch improves the code reuse of the actions system and adds several improvements for easier debugging via clang-repl --debug-only=clang-repl.
The change inimproves the consistency of the TUKind when actions are handled within a WrapperFrontendAction. In this case instead of falling back to default TU_Complete, we forward to the TUKind of the ASTContext which presumably was created by the intended action. This enables the incremental infrastructure to reuse code.
This patch also clones the first llvm::Module because the first PTU now can come from -include A.h and the presumption of llvm::Module being empty does not hold. The changes are a first step to fix the issues with `clang-repl --cuda`.
---
Full diff: https://github.com/llvm/llvm-project/pull/116610.diff
5 Files Affected:
- (modified) clang/include/clang/Frontend/FrontendAction.h (+7-1)
- (modified) clang/include/clang/Interpreter/Interpreter.h (+2-1)
- (modified) clang/include/clang/Interpreter/PartialTranslationUnit.h (+3)
- (modified) clang/lib/Interpreter/CMakeLists.txt (+2-1)
- (modified) clang/lib/Interpreter/Interpreter.cpp (+43-28)
``````````diff
diff --git a/clang/include/clang/Frontend/FrontendAction.h b/clang/include/clang/Frontend/FrontendAction.h
index 039f6f247b6d8c..718684a67771a2 100644
--- a/clang/include/clang/Frontend/FrontendAction.h
+++ b/clang/include/clang/Frontend/FrontendAction.h
@@ -21,6 +21,7 @@
#include "clang/Basic/LLVM.h"
#include "clang/Basic/LangOptions.h"
#include "clang/Frontend/ASTUnit.h"
+#include "clang/Frontend/CompilerInstance.h"
#include "clang/Frontend/FrontendOptions.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/Support/Error.h"
@@ -185,7 +186,12 @@ class FrontendAction {
virtual bool usesPreprocessorOnly() const = 0;
/// For AST-based actions, the kind of translation unit we're handling.
- virtual TranslationUnitKind getTranslationUnitKind() { return TU_Complete; }
+ virtual TranslationUnitKind getTranslationUnitKind() {
+ // The ASTContext, if exists, knows the exact TUKind of the frondend.
+ if (Instance && Instance->hasASTContext())
+ return Instance->getASTContext().TUKind;
+ return TU_Complete;
+ }
/// Does this action support use with PCH?
virtual bool hasPCHSupport() const { return true; }
diff --git a/clang/include/clang/Interpreter/Interpreter.h b/clang/include/clang/Interpreter/Interpreter.h
index 1230a3a7016fae..b1b63aedf86aba 100644
--- a/clang/include/clang/Interpreter/Interpreter.h
+++ b/clang/include/clang/Interpreter/Interpreter.h
@@ -177,7 +177,8 @@ class Interpreter {
CodeGenerator *getCodeGen() const;
std::unique_ptr<llvm::Module> GenModule();
- PartialTranslationUnit &RegisterPTU(TranslationUnitDecl *TU);
+ PartialTranslationUnit &RegisterPTU(TranslationUnitDecl *TU,
+ std::unique_ptr<llvm::Module> M = {});
// A cache for the compiled destructors used to for de-allocation of managed
// clang::Values.
diff --git a/clang/include/clang/Interpreter/PartialTranslationUnit.h b/clang/include/clang/Interpreter/PartialTranslationUnit.h
index bf91d559452b8a..c878e139fe70d0 100644
--- a/clang/include/clang/Interpreter/PartialTranslationUnit.h
+++ b/clang/include/clang/Interpreter/PartialTranslationUnit.h
@@ -31,6 +31,9 @@ struct PartialTranslationUnit {
/// The llvm IR produced for the input.
std::unique_ptr<llvm::Module> TheModule;
+ bool operator==(const PartialTranslationUnit &other) {
+ return other.TUPart == TUPart && other.TheModule == TheModule;
+ }
};
} // namespace clang
diff --git a/clang/lib/Interpreter/CMakeLists.txt b/clang/lib/Interpreter/CMakeLists.txt
index d5ffe78251d253..df7ea82e0dada5 100644
--- a/clang/lib/Interpreter/CMakeLists.txt
+++ b/clang/lib/Interpreter/CMakeLists.txt
@@ -10,7 +10,8 @@ set(LLVM_LINK_COMPONENTS
Support
Target
TargetParser
- )
+ TransformUtils
+ )
if (EMSCRIPTEN AND "lld" IN_LIST LLVM_ENABLE_PROJECTS)
set(WASM_SRC Wasm.cpp)
diff --git a/clang/lib/Interpreter/Interpreter.cpp b/clang/lib/Interpreter/Interpreter.cpp
index bc96da811d44cb..1859c6802c6f2c 100644
--- a/clang/lib/Interpreter/Interpreter.cpp
+++ b/clang/lib/Interpreter/Interpreter.cpp
@@ -50,6 +50,9 @@
#include "llvm/Support/ErrorHandling.h"
#include "llvm/Support/raw_ostream.h"
#include "llvm/TargetParser/Host.h"
+#include "llvm/Transforms/Utils/Cloning.h" // for CloneModule
+
+#define DEBUG_TYPE "clang-repl"
using namespace clang;
// FIXME: Figure out how to unify with namespace init_convenience from
@@ -339,19 +342,8 @@ class IncrementalAction : public WrapperFrontendAction {
}
void ExecuteAction() override {
- CompilerInstance &CI = getCompilerInstance();
- assert(CI.hasPreprocessor() && "No PP!");
-
- // Use a code completion consumer?
- CodeCompleteConsumer *CompletionConsumer = nullptr;
- if (CI.hasCodeCompletionConsumer())
- CompletionConsumer = &CI.getCodeCompletionConsumer();
-
- Preprocessor &PP = CI.getPreprocessor();
- PP.EnterMainSourceFile();
-
- if (!CI.hasSema())
- CI.createSema(getTranslationUnitKind(), CompletionConsumer);
+ WrapperFrontendAction::ExecuteAction();
+ getCompilerInstance().getSema().CurContext = nullptr;
}
// Do not terminate after processing the input. This allows us to keep various
@@ -385,8 +377,6 @@ Interpreter::Interpreter(std::unique_ptr<CompilerInstance> Instance,
return;
CI->ExecuteAction(*Act);
- ASTContext &C = CI->getASTContext();
-
IncrParser = std::make_unique<IncrementalParser>(*CI, ErrOut);
if (ErrOut)
@@ -394,18 +384,22 @@ Interpreter::Interpreter(std::unique_ptr<CompilerInstance> Instance,
if (getCodeGen()) {
CachedInCodeGenModule = GenModule();
+ // The initial PTU is filled by `-include` or by CUDA includes
+ // automatically.
+ if (!CI->getPreprocessorOpts().Includes.empty()) {
+ // We can't really directly pass the CachedInCodeGenModule to the Jit
+ // because it will steal it, causing dangling references as explained in
+ // Interpreter::Execute
+ auto M = llvm::CloneModule(*CachedInCodeGenModule);
+ ASTContext &C = CI->getASTContext();
+ RegisterPTU(C.getTranslationUnitDecl(), std::move(M));
+ }
if (llvm::Error Err = CreateExecutor()) {
ErrOut = joinErrors(std::move(ErrOut), std::move(Err));
return;
}
}
- // The initial PTU is filled by `-include` or by CUDA includes automatically.
- RegisterPTU(C.getTranslationUnitDecl());
-
- // Prepare the IncrParser for input.
- llvm::cantFail(Parse(""));
-
// Not all frontends support code-generation, e.g. ast-dump actions don't
if (getCodeGen()) {
// Process the PTUs that came from initialization. For example -include will
@@ -535,14 +529,25 @@ size_t Interpreter::getEffectivePTUSize() const {
return PTUs.size() - InitPTUSize;
}
-PartialTranslationUnit &Interpreter::RegisterPTU(TranslationUnitDecl *TU) {
+PartialTranslationUnit &
+Interpreter::RegisterPTU(TranslationUnitDecl *TU,
+ std::unique_ptr<llvm::Module> M /*={}*/) {
PTUs.emplace_back(PartialTranslationUnit());
PartialTranslationUnit &LastPTU = PTUs.back();
LastPTU.TUPart = TU;
- if (std::unique_ptr<llvm::Module> M = GenModule())
- LastPTU.TheModule = std::move(M);
+ if (!M)
+ M = GenModule();
+
+ assert((!getCodeGen() || M) && "Must have a llvm::Module at this point");
+ LastPTU.TheModule = std::move(M);
+ LLVM_DEBUG(llvm::dbgs() << "compile-ptu " << PTUs.size() - 1
+ << ": [TU=" << LastPTU.TUPart);
+ if (LastPTU.TheModule)
+ LLVM_DEBUG(llvm::dbgs() << ", M=" << LastPTU.TheModule.get() << " ("
+ << LastPTU.TheModule->getName() << ")");
+ LLVM_DEBUG(llvm::dbgs() << "]\n");
return LastPTU;
}
@@ -615,6 +620,14 @@ void Interpreter::ResetExecutor() { IncrExecutor.reset(); }
llvm::Error Interpreter::Execute(PartialTranslationUnit &T) {
assert(T.TheModule);
+ LLVM_DEBUG(llvm::dbgs()
+ << "execute-ptu "
+ << ((std::find(PTUs.begin(), PTUs.end(), T) != PTUs.end())
+ ? std::distance(PTUs.begin(),
+ std::find(PTUs.begin(), PTUs.end(), T))
+ : -1)
+ << ": [TU=" << T.TUPart << ", M=" << T.TheModule.get() << " ("
+ << T.TheModule->getName() << ")]\n");
if (!IncrExecutor) {
auto Err = CreateExecutor();
if (Err)
@@ -723,10 +736,12 @@ std::unique_ptr<llvm::Module> Interpreter::GenModule() {
// 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 || (CachedInCodeGenModule->empty() &&
- CachedInCodeGenModule->global_empty() &&
- CachedInCodeGenModule->alias_empty() &&
- CachedInCodeGenModule->ifunc_empty())) &&
+ assert(((!CachedInCodeGenModule ||
+ !getCompilerInstance()->getPreprocessorOpts().Includes.empty()) ||
+ (CachedInCodeGenModule->empty() &&
+ CachedInCodeGenModule->global_empty() &&
+ CachedInCodeGenModule->alias_empty() &&
+ CachedInCodeGenModule->ifunc_empty())) &&
"CodeGen wrote to a readonly module");
std::unique_ptr<llvm::Module> M(CG->ReleaseModule());
CG->StartModule("incr_module_" + std::to_string(ID++), M->getContext());
``````````
</details>
https://github.com/llvm/llvm-project/pull/116610
More information about the cfe-commits
mailing list