Skip to content

Conversation

@andykaylor
Copy link
Contributor

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).

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).
@llvmbot llvmbot added clang Clang issues not falling into any other category ClangIR Anything related to the ClangIR project labels Jul 31, 2025
@llvmbot
Copy link
Member

llvmbot commented Jul 31, 2025

@llvm/pr-subscribers-clang

Author: Andy Kaylor (andykaylor)

Changes

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).


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

2 Files Affected:

  • (modified) clang/include/clang/CIR/CIRGenerator.h (+3)
  • (modified) clang/lib/CIR/CodeGen/CIRGenerator.cpp (+21)
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");
+}

@llvmbot
Copy link
Member

llvmbot commented Jul 31, 2025

@llvm/pr-subscribers-clangir

Author: Andy Kaylor (andykaylor)

Changes

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).


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

2 Files Affected:

  • (modified) clang/include/clang/CIR/CIRGenerator.h (+3)
  • (modified) clang/lib/CIR/CodeGen/CIRGenerator.cpp (+21)
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");
+}

Copy link
Collaborator

@erichkeane erichkeane left a 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?

@andykaylor
Copy link
Contributor Author

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.

Copy link
Collaborator

@erichkeane erichkeane left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Huh, neat!

Copy link
Contributor

@xlauko xlauko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@andykaylor andykaylor merged commit 96d1571 into llvm:main Aug 1, 2025
9 checks passed
@andykaylor andykaylor deleted the nyi-ast-visitors branch August 1, 2025 16:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

clang Clang issues not falling into any other category ClangIR Anything related to the ClangIR project

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants