-
Notifications
You must be signed in to change notification settings - Fork 15.2k
[CIR] Add diagnostic for NYI AST visitor handlers #151561
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
A couple of handlers that were missing from the CIRGenerator AST visitor allowed important features to be silently ignored during CIR generation. This change adds these handlers with diagnostics to report that they are not yet handled (except in the case where only debug information is missed).
|
@llvm/pr-subscribers-clang Author: Andy Kaylor (andykaylor) ChangesA couple of handlers that were missing from the CIRGenerator AST visitor allowed important features to be silently ignored during CIR generation. This change adds these handlers with diagnostics to report that they are not yet handled (except in the case where only debug information is missed). Full diff: https://github.com/llvm/llvm-project/pull/151561.diff 2 Files Affected:
diff --git a/clang/include/clang/CIR/CIRGenerator.h b/clang/include/clang/CIR/CIRGenerator.h
index dd48eec238fca..5ea11463ffa9f 100644
--- a/clang/include/clang/CIR/CIRGenerator.h
+++ b/clang/include/clang/CIR/CIRGenerator.h
@@ -79,7 +79,10 @@ class CIRGenerator : public clang::ASTConsumer {
void HandleTranslationUnit(clang::ASTContext &astContext) override;
void HandleInlineFunctionDefinition(clang::FunctionDecl *d) override;
void HandleTagDeclDefinition(clang::TagDecl *d) override;
+ void HandleTagDeclRequiredDefinition(const clang::TagDecl *D) override;
+ void HandleCXXStaticMemberVarInstantiation(clang::VarDecl *D) override;
void CompleteTentativeDefinition(clang::VarDecl *d) override;
+ void HandleVTable(clang::CXXRecordDecl *rd) override;
mlir::ModuleOp getModule() const;
mlir::MLIRContext &getMLIRContext() { return *mlirContext; };
diff --git a/clang/lib/CIR/CodeGen/CIRGenerator.cpp b/clang/lib/CIR/CodeGen/CIRGenerator.cpp
index 99d652841be27..b0357d9d3b7fa 100644
--- a/clang/lib/CIR/CodeGen/CIRGenerator.cpp
+++ b/clang/lib/CIR/CodeGen/CIRGenerator.cpp
@@ -152,9 +152,30 @@ void CIRGenerator::HandleTagDeclDefinition(TagDecl *d) {
cgm->errorNYI(d->getSourceRange(), "HandleTagDeclDefinition: OpenMP");
}
+void CIRGenerator::HandleTagDeclRequiredDefinition(const TagDecl *D) {
+ if (diags.hasErrorOccurred())
+ return;
+
+ assert(!cir::MissingFeatures::generateDebugInfo());
+}
+
+void CIRGenerator::HandleCXXStaticMemberVarInstantiation(VarDecl *D) {
+ if (diags.hasErrorOccurred())
+ return;
+
+ cgm->errorNYI(D->getSourceRange(), "HandleCXXStaticMemberVarInstantiation");
+}
+
void CIRGenerator::CompleteTentativeDefinition(VarDecl *d) {
if (diags.hasErrorOccurred())
return;
cgm->emitTentativeDefinition(d);
}
+
+void CIRGenerator::HandleVTable(CXXRecordDecl *rd) {
+ if (diags.hasErrorOccurred())
+ return;
+
+ cgm->errorNYI(rd->getSourceRange(), "HandleVTable");
+}
|
|
@llvm/pr-subscribers-clangir Author: Andy Kaylor (andykaylor) ChangesA couple of handlers that were missing from the CIRGenerator AST visitor allowed important features to be silently ignored during CIR generation. This change adds these handlers with diagnostics to report that they are not yet handled (except in the case where only debug information is missed). Full diff: https://github.com/llvm/llvm-project/pull/151561.diff 2 Files Affected:
diff --git a/clang/include/clang/CIR/CIRGenerator.h b/clang/include/clang/CIR/CIRGenerator.h
index dd48eec238fca..5ea11463ffa9f 100644
--- a/clang/include/clang/CIR/CIRGenerator.h
+++ b/clang/include/clang/CIR/CIRGenerator.h
@@ -79,7 +79,10 @@ class CIRGenerator : public clang::ASTConsumer {
void HandleTranslationUnit(clang::ASTContext &astContext) override;
void HandleInlineFunctionDefinition(clang::FunctionDecl *d) override;
void HandleTagDeclDefinition(clang::TagDecl *d) override;
+ void HandleTagDeclRequiredDefinition(const clang::TagDecl *D) override;
+ void HandleCXXStaticMemberVarInstantiation(clang::VarDecl *D) override;
void CompleteTentativeDefinition(clang::VarDecl *d) override;
+ void HandleVTable(clang::CXXRecordDecl *rd) override;
mlir::ModuleOp getModule() const;
mlir::MLIRContext &getMLIRContext() { return *mlirContext; };
diff --git a/clang/lib/CIR/CodeGen/CIRGenerator.cpp b/clang/lib/CIR/CodeGen/CIRGenerator.cpp
index 99d652841be27..b0357d9d3b7fa 100644
--- a/clang/lib/CIR/CodeGen/CIRGenerator.cpp
+++ b/clang/lib/CIR/CodeGen/CIRGenerator.cpp
@@ -152,9 +152,30 @@ void CIRGenerator::HandleTagDeclDefinition(TagDecl *d) {
cgm->errorNYI(d->getSourceRange(), "HandleTagDeclDefinition: OpenMP");
}
+void CIRGenerator::HandleTagDeclRequiredDefinition(const TagDecl *D) {
+ if (diags.hasErrorOccurred())
+ return;
+
+ assert(!cir::MissingFeatures::generateDebugInfo());
+}
+
+void CIRGenerator::HandleCXXStaticMemberVarInstantiation(VarDecl *D) {
+ if (diags.hasErrorOccurred())
+ return;
+
+ cgm->errorNYI(D->getSourceRange(), "HandleCXXStaticMemberVarInstantiation");
+}
+
void CIRGenerator::CompleteTentativeDefinition(VarDecl *d) {
if (diags.hasErrorOccurred())
return;
cgm->emitTentativeDefinition(d);
}
+
+void CIRGenerator::HandleVTable(CXXRecordDecl *rd) {
+ if (diags.hasErrorOccurred())
+ return;
+
+ cgm->errorNYI(rd->getSourceRange(), "HandleVTable");
+}
|
erichkeane
left a comment
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.
Should we get a test for the TagDeclRequired case?
This turned out to be a very good question. Forty-five of the existing tests should go through that case, but I was missing a connection in the chain of visitors. so when I added an assert there to make sure it was being covered, it wasn't. With my latest update here it is. However, since that handler doesn't do anything, there is no way to test it apart from just having it covered, which it is. |
erichkeane
left a comment
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.
Huh, neat!
xlauko
left a comment
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.
lgtm
A couple of handlers that were missing from the CIRGenerator AST visitor allowed important features to be silently ignored during CIR generation. This change adds these handlers with diagnostics to report that they are not yet handled (except in the case where only debug information is missed).