Skip to content

[HLSL] Allow EmptyDecl in cbuffer/tbuffer #128250

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

Merged
merged 2 commits into from
Feb 26, 2025

Conversation

llvm-beanz
Copy link
Collaborator

We do handle EmptyDecls in codegen already as of #124886, but we were blocking them in Sema. EmptyDecls tend to be caused by extra semicolons which are not illegal.

Fixes #128238

We do handle EmptyDecls in codegen already as of llvm#124886, but we were
blocking them in Sema. EmptyDecls tend to be caused by extra semicolons
which are not illegal.

Fixes llvm#128238
@llvm-beanz llvm-beanz requested a review from hekota February 21, 2025 23:49
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" HLSL HLSL Language Support labels Feb 21, 2025
@llvmbot
Copy link
Member

llvmbot commented Feb 21, 2025

@llvm/pr-subscribers-hlsl

Author: Chris B (llvm-beanz)

Changes

We do handle EmptyDecls in codegen already as of #124886, but we were blocking them in Sema. EmptyDecls tend to be caused by extra semicolons which are not illegal.

Fixes #128238


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

2 Files Affected:

  • (modified) clang/lib/Parse/ParseHLSL.cpp (+1-1)
  • (modified) clang/test/SemaHLSL/cb_error.hlsl (+12)
diff --git a/clang/lib/Parse/ParseHLSL.cpp b/clang/lib/Parse/ParseHLSL.cpp
index 443bf2b9ec626..612e1ab0084ff 100644
--- a/clang/lib/Parse/ParseHLSL.cpp
+++ b/clang/lib/Parse/ParseHLSL.cpp
@@ -30,7 +30,7 @@ static bool validateDeclsInsideHLSLBuffer(Parser::DeclGroupPtrTy DG,
   // Only allow function, variable, record decls inside HLSLBuffer.
   for (DeclGroupRef::iterator I = Decls.begin(), E = Decls.end(); I != E; ++I) {
     Decl *D = *I;
-    if (isa<CXXRecordDecl, RecordDecl, FunctionDecl, VarDecl>(D))
+    if (isa<CXXRecordDecl, RecordDecl, FunctionDecl, VarDecl, EmptyDecl>(D))
       continue;
 
     // FIXME: support nested HLSLBuffer and namespace inside HLSLBuffer.
diff --git a/clang/test/SemaHLSL/cb_error.hlsl b/clang/test/SemaHLSL/cb_error.hlsl
index 133adeeb2068b..95c917a9bb9ee 100644
--- a/clang/test/SemaHLSL/cb_error.hlsl
+++ b/clang/test/SemaHLSL/cb_error.hlsl
@@ -47,3 +47,15 @@ tbuffer B {
   // expected-error@+1 {{unknown type name 'flaot'}}
   flaot f;
 }
+
+// None of these should produce an error!
+cbuffer EmptyCBuffer {}
+
+cbuffer EmptyDeclCBuffer {
+  ;
+}
+
+cbuffer EmptyDecl2CBuffer {
+  ;
+  int X;
+}

@llvmbot
Copy link
Member

llvmbot commented Feb 21, 2025

@llvm/pr-subscribers-clang

Author: Chris B (llvm-beanz)

Changes

We do handle EmptyDecls in codegen already as of #124886, but we were blocking them in Sema. EmptyDecls tend to be caused by extra semicolons which are not illegal.

Fixes #128238


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

2 Files Affected:

  • (modified) clang/lib/Parse/ParseHLSL.cpp (+1-1)
  • (modified) clang/test/SemaHLSL/cb_error.hlsl (+12)
diff --git a/clang/lib/Parse/ParseHLSL.cpp b/clang/lib/Parse/ParseHLSL.cpp
index 443bf2b9ec626..612e1ab0084ff 100644
--- a/clang/lib/Parse/ParseHLSL.cpp
+++ b/clang/lib/Parse/ParseHLSL.cpp
@@ -30,7 +30,7 @@ static bool validateDeclsInsideHLSLBuffer(Parser::DeclGroupPtrTy DG,
   // Only allow function, variable, record decls inside HLSLBuffer.
   for (DeclGroupRef::iterator I = Decls.begin(), E = Decls.end(); I != E; ++I) {
     Decl *D = *I;
-    if (isa<CXXRecordDecl, RecordDecl, FunctionDecl, VarDecl>(D))
+    if (isa<CXXRecordDecl, RecordDecl, FunctionDecl, VarDecl, EmptyDecl>(D))
       continue;
 
     // FIXME: support nested HLSLBuffer and namespace inside HLSLBuffer.
diff --git a/clang/test/SemaHLSL/cb_error.hlsl b/clang/test/SemaHLSL/cb_error.hlsl
index 133adeeb2068b..95c917a9bb9ee 100644
--- a/clang/test/SemaHLSL/cb_error.hlsl
+++ b/clang/test/SemaHLSL/cb_error.hlsl
@@ -47,3 +47,15 @@ tbuffer B {
   // expected-error@+1 {{unknown type name 'flaot'}}
   flaot f;
 }
+
+// None of these should produce an error!
+cbuffer EmptyCBuffer {}
+
+cbuffer EmptyDeclCBuffer {
+  ;
+}
+
+cbuffer EmptyDecl2CBuffer {
+  ;
+  int X;
+}

Copy link
Member

@hekota hekota left a comment

Choose a reason for hiding this comment

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

LGTM!

@llvm-beanz llvm-beanz merged commit 0f6240c into llvm:main Feb 26, 2025
12 checks passed
@damyanp damyanp moved this to Closed in HLSL Support Apr 25, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category HLSL HLSL Language Support
Projects
Status: Closed
Development

Successfully merging this pull request may close these issues.

[HLSL] Allow EmptyDecl in cbuffer
5 participants