-
Notifications
You must be signed in to change notification settings - Fork 14.4k
[CIR] Don't generate ClangIR after an unrecoverable error occured #130971
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
@llvm/pr-subscribers-clangir @llvm/pr-subscribers-clang Author: Morris Hafner (mmha) ChangesFull diff: https://github.com/llvm/llvm-project/pull/130971.diff 1 Files Affected:
diff --git a/clang/lib/CIR/CodeGen/CIRGenerator.cpp b/clang/lib/CIR/CodeGen/CIRGenerator.cpp
index 91070eda7d45a..6fa31ab707139 100644
--- a/clang/lib/CIR/CodeGen/CIRGenerator.cpp
+++ b/clang/lib/CIR/CodeGen/CIRGenerator.cpp
@@ -43,6 +43,8 @@ void CIRGenerator::Initialize(ASTContext &astContext) {
mlir::ModuleOp CIRGenerator::getModule() const { return cgm->getModule(); }
bool CIRGenerator::HandleTopLevelDecl(DeclGroupRef group) {
+ if (diags.hasUnrecoverableErrorOccurred())
+ return true;
for (Decl *decl : group)
cgm->emitTopLevelDecl(decl);
|
@@ -43,6 +43,8 @@ void CIRGenerator::Initialize(ASTContext &astContext) { | |||
mlir::ModuleOp CIRGenerator::getModule() const { return cgm->getModule(); } | |||
|
|||
bool CIRGenerator::HandleTopLevelDecl(DeclGroupRef group) { | |||
if (diags.hasUnrecoverableErrorOccurred()) | |||
return true; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I chose to let parsing continue here mostly because this is what's being done in other ASTConsumers, but is there a reason to do so?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It honestly doesn't really matter, we currently delay the codegen in clang until parsing/sema is complete anyway (except for REPL). I'd probably leave it for now, as I wouldn't want this to be a changed behavior.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good, I don't think we really need a test for it. We have to make sure we get this in EVERY one of the external interfaces unfortunately (as they are added), but LGTM.
@@ -43,6 +43,8 @@ void CIRGenerator::Initialize(ASTContext &astContext) { | |||
mlir::ModuleOp CIRGenerator::getModule() const { return cgm->getModule(); } | |||
|
|||
bool CIRGenerator::HandleTopLevelDecl(DeclGroupRef group) { | |||
if (diags.hasUnrecoverableErrorOccurred()) | |||
return true; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It honestly doesn't really matter, we currently delay the codegen in clang until parsing/sema is complete anyway (except for REPL). I'd probably leave it for now, as I wouldn't want this to be a changed behavior.
No description provided.