Skip to content

[HLSL] Allow resource type attributes only on __hlsl_resource_t #110143

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Sep 26, 2024
Merged

[HLSL] Allow resource type attributes only on __hlsl_resource_t #110143

merged 1 commit into from
Sep 26, 2024

Conversation

hekota
Copy link
Member

@hekota hekota commented Sep 26, 2024

Resource type attributes should only ever be used on the intangible type __hlsl_resource_t.

@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 Sep 26, 2024
@hekota hekota changed the title [HLSL] Allow resource type attributes only on __hlsl_resource_t [HLSL] Allow resource type attributes only on __hlsl_resource_t Sep 26, 2024
@llvmbot
Copy link
Member

llvmbot commented Sep 26, 2024

@llvm/pr-subscribers-clang

Author: Helena Kotas (hekota)

Changes

Resource type attributes should only ever be used on the intangible type __hlsl_resource_t.

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

8 Files Affected:

  • (modified) clang/include/clang/Basic/DiagnosticSemaKinds.td (+1)
  • (modified) clang/include/clang/Sema/SemaHLSL.h (+1-1)
  • (modified) clang/lib/Sema/SemaHLSL.cpp (+8-2)
  • (modified) clang/lib/Sema/SemaType.cpp (+1-1)
  • (modified) clang/test/ParserHLSL/hlsl_contained_type_attr_error.hlsl (+4)
  • (modified) clang/test/ParserHLSL/hlsl_is_rov_attr_error.hlsl (+4)
  • (modified) clang/test/ParserHLSL/hlsl_raw_buffer_attr_error.hlsl (+4)
  • (modified) clang/test/ParserHLSL/hlsl_resource_class_attr_error.hlsl (+3)
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index e4e04bff8b5120..a9b2042fcff4e3 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -12394,6 +12394,7 @@ def err_hlsl_packoffset_alignment_mismatch : Error<"packoffset at 'y' not match
 def err_hlsl_pointers_unsupported : Error<
   "%select{pointers|references}0 are unsupported in HLSL">;
 def err_hlsl_missing_resource_class : Error<"HLSL resource needs to have [[hlsl::resource_class()]] attribute">;
+def err_hlsl_attribute_needs_intangible_type: Error<"attribute %0 can be used only on HLSL intangible type %1">;
 
 def err_hlsl_operator_unsupported : Error<
   "the '%select{&|*|->}0' operator is unsupported in HLSL">;
diff --git a/clang/include/clang/Sema/SemaHLSL.h b/clang/include/clang/Sema/SemaHLSL.h
index e088254c566d3e..311cd58bbcac2c 100644
--- a/clang/include/clang/Sema/SemaHLSL.h
+++ b/clang/include/clang/Sema/SemaHLSL.h
@@ -70,7 +70,7 @@ class SemaHLSL : public SemaBase {
   void handleShaderAttr(Decl *D, const ParsedAttr &AL);
   void handleResourceBindingAttr(Decl *D, const ParsedAttr &AL);
   void handleParamModifierAttr(Decl *D, const ParsedAttr &AL);
-  bool handleResourceTypeAttr(const ParsedAttr &AL);
+  bool handleResourceTypeAttr(QualType T, const ParsedAttr &AL);
 
   bool CheckBuiltinFunctionCall(unsigned BuiltinID, CallExpr *TheCall);
   QualType ProcessResourceTypeAttributes(QualType Wrapped);
diff --git a/clang/lib/Sema/SemaHLSL.cpp b/clang/lib/Sema/SemaHLSL.cpp
index ebe76185cbb2d5..1d8ccdda45573f 100644
--- a/clang/lib/Sema/SemaHLSL.cpp
+++ b/clang/lib/Sema/SemaHLSL.cpp
@@ -693,13 +693,19 @@ bool clang::CreateHLSLAttributedResourceType(
 // HLSL resource. The attributes are collected in HLSLResourcesTypeAttrs and at
 // the end of the declaration they are applied to the declaration type by
 // wrapping it in HLSLAttributedResourceType.
-bool SemaHLSL::handleResourceTypeAttr(const ParsedAttr &AL) {
-  Attr *A = nullptr;
+bool SemaHLSL::handleResourceTypeAttr(QualType T, const ParsedAttr &AL) {
+  // only allow resource type attributes on intangible types
+  if (!T->isHLSLResourceType()) {
+    Diag(AL.getLoc(), diag::err_hlsl_attribute_needs_intangible_type)
+        << AL << getASTContext().HLSLResourceTy;
+    return false;
+  }
 
   // validate number of arguments
   if (!AL.checkExactlyNumArgs(SemaRef, AL.getMinArgs()))
     return false;
 
+  Attr *A = nullptr;
   switch (AL.getKind()) {
   case ParsedAttr::AT_HLSLResourceClass: {
     if (!AL.isArgIdent(0)) {
diff --git a/clang/lib/Sema/SemaType.cpp b/clang/lib/Sema/SemaType.cpp
index 950bd6db0359d1..a7beb9d222c3b5 100644
--- a/clang/lib/Sema/SemaType.cpp
+++ b/clang/lib/Sema/SemaType.cpp
@@ -8860,7 +8860,7 @@ static void processTypeAttrs(TypeProcessingState &state, QualType &type,
       // decl-specifier-seq; do not collect attributes on declarations or those
       // that get to slide after declaration name.
       if (TAL == TAL_DeclSpec &&
-          state.getSema().HLSL().handleResourceTypeAttr(attr))
+          state.getSema().HLSL().handleResourceTypeAttr(type, attr))
         attr.setUsedAsTypeAttr();
       break;
     }
diff --git a/clang/test/ParserHLSL/hlsl_contained_type_attr_error.hlsl b/clang/test/ParserHLSL/hlsl_contained_type_attr_error.hlsl
index 1c37d72de86148..b2d492d95945c1 100644
--- a/clang/test/ParserHLSL/hlsl_contained_type_attr_error.hlsl
+++ b/clang/test/ParserHLSL/hlsl_contained_type_attr_error.hlsl
@@ -22,3 +22,7 @@ __hlsl_resource_t [[hlsl::resource_class(UAV)]] [[hlsl::contained_type(float)]]
 
 // expected-warning@+1{{attribute 'contained_type' is already applied with different arguments}}
 __hlsl_resource_t [[hlsl::resource_class(UAV)]] [[hlsl::contained_type(float)]] [[hlsl::contained_type(int)]] h8;
+
+// expected-error@+2{{attribute 'resource_class' can be used only on HLSL intangible type '__hlsl_resource_t'}}
+// expected-error@+1{{attribute 'contained_type' can be used only on HLSL intangible type '__hlsl_resource_t'}}
+float [[hlsl::resource_class(UAV)]] [[hlsl::contained_type(float)]] res5;
diff --git a/clang/test/ParserHLSL/hlsl_is_rov_attr_error.hlsl b/clang/test/ParserHLSL/hlsl_is_rov_attr_error.hlsl
index 15685bd1a3baa7..3b2c12e7a96c5c 100644
--- a/clang/test/ParserHLSL/hlsl_is_rov_attr_error.hlsl
+++ b/clang/test/ParserHLSL/hlsl_is_rov_attr_error.hlsl
@@ -14,3 +14,7 @@ __hlsl_resource_t [[hlsl::resource_class(UAV)]] [[hlsl::is_rov(gibberish)]] res3
 
 // expected-warning@+1{{attribute 'is_rov' is already applied}}
 __hlsl_resource_t [[hlsl::resource_class(UAV)]] [[hlsl::is_rov]] [[hlsl::is_rov]] res4;
+
+// expected-error@+2{{attribute 'resource_class' can be used only on HLSL intangible type '__hlsl_resource_t'}}
+// expected-error@+1{{attribute 'is_rov' can be used only on HLSL intangible type '__hlsl_resource_t'}}
+float [[hlsl::resource_class(UAV)]] [[hlsl::is_rov]] res5;
diff --git a/clang/test/ParserHLSL/hlsl_raw_buffer_attr_error.hlsl b/clang/test/ParserHLSL/hlsl_raw_buffer_attr_error.hlsl
index 83273426017ed0..77530cbf9e4d92 100644
--- a/clang/test/ParserHLSL/hlsl_raw_buffer_attr_error.hlsl
+++ b/clang/test/ParserHLSL/hlsl_raw_buffer_attr_error.hlsl
@@ -11,3 +11,7 @@ __hlsl_resource_t [[hlsl::resource_class(UAV)]] [[hlsl::raw_buffer(gibberish)]]
 
 // expected-warning@+1{{attribute 'raw_buffer' is already applied}}
 __hlsl_resource_t [[hlsl::resource_class(UAV)]] [[hlsl::raw_buffer]] [[hlsl::raw_buffer]] res4;
+
+// expected-error@+2{{attribute 'resource_class' can be used only on HLSL intangible type '__hlsl_resource_t'}}
+// expected-error@+1{{attribute 'raw_buffer' can be used only on HLSL intangible type '__hlsl_resource_t'}}
+float [[hlsl::resource_class(UAV)]] [[hlsl::raw_buffer]] res5;
diff --git a/clang/test/ParserHLSL/hlsl_resource_class_attr_error.hlsl b/clang/test/ParserHLSL/hlsl_resource_class_attr_error.hlsl
index 01ff1c007e2b57..63e39daff949b4 100644
--- a/clang/test/ParserHLSL/hlsl_resource_class_attr_error.hlsl
+++ b/clang/test/ParserHLSL/hlsl_resource_class_attr_error.hlsl
@@ -17,3 +17,6 @@ __hlsl_resource_t [[hlsl::resource_class(SRV)]] [[hlsl::resource_class(SRV)]] e4
 
 // expected-error@+1{{'resource_class' attribute takes one argument}}
 __hlsl_resource_t [[hlsl::resource_class(SRV, "aa")]] e5;
+
+// expected-error@+1{{attribute 'resource_class' can be used only on HLSL intangible type '__hlsl_resource_t'}}
+float [[hlsl::resource_class(UAV)]] e6;

@llvmbot
Copy link
Member

llvmbot commented Sep 26, 2024

@llvm/pr-subscribers-hlsl

Author: Helena Kotas (hekota)

Changes

Resource type attributes should only ever be used on the intangible type __hlsl_resource_t.

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

8 Files Affected:

  • (modified) clang/include/clang/Basic/DiagnosticSemaKinds.td (+1)
  • (modified) clang/include/clang/Sema/SemaHLSL.h (+1-1)
  • (modified) clang/lib/Sema/SemaHLSL.cpp (+8-2)
  • (modified) clang/lib/Sema/SemaType.cpp (+1-1)
  • (modified) clang/test/ParserHLSL/hlsl_contained_type_attr_error.hlsl (+4)
  • (modified) clang/test/ParserHLSL/hlsl_is_rov_attr_error.hlsl (+4)
  • (modified) clang/test/ParserHLSL/hlsl_raw_buffer_attr_error.hlsl (+4)
  • (modified) clang/test/ParserHLSL/hlsl_resource_class_attr_error.hlsl (+3)
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index e4e04bff8b5120..a9b2042fcff4e3 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -12394,6 +12394,7 @@ def err_hlsl_packoffset_alignment_mismatch : Error<"packoffset at 'y' not match
 def err_hlsl_pointers_unsupported : Error<
   "%select{pointers|references}0 are unsupported in HLSL">;
 def err_hlsl_missing_resource_class : Error<"HLSL resource needs to have [[hlsl::resource_class()]] attribute">;
+def err_hlsl_attribute_needs_intangible_type: Error<"attribute %0 can be used only on HLSL intangible type %1">;
 
 def err_hlsl_operator_unsupported : Error<
   "the '%select{&|*|->}0' operator is unsupported in HLSL">;
diff --git a/clang/include/clang/Sema/SemaHLSL.h b/clang/include/clang/Sema/SemaHLSL.h
index e088254c566d3e..311cd58bbcac2c 100644
--- a/clang/include/clang/Sema/SemaHLSL.h
+++ b/clang/include/clang/Sema/SemaHLSL.h
@@ -70,7 +70,7 @@ class SemaHLSL : public SemaBase {
   void handleShaderAttr(Decl *D, const ParsedAttr &AL);
   void handleResourceBindingAttr(Decl *D, const ParsedAttr &AL);
   void handleParamModifierAttr(Decl *D, const ParsedAttr &AL);
-  bool handleResourceTypeAttr(const ParsedAttr &AL);
+  bool handleResourceTypeAttr(QualType T, const ParsedAttr &AL);
 
   bool CheckBuiltinFunctionCall(unsigned BuiltinID, CallExpr *TheCall);
   QualType ProcessResourceTypeAttributes(QualType Wrapped);
diff --git a/clang/lib/Sema/SemaHLSL.cpp b/clang/lib/Sema/SemaHLSL.cpp
index ebe76185cbb2d5..1d8ccdda45573f 100644
--- a/clang/lib/Sema/SemaHLSL.cpp
+++ b/clang/lib/Sema/SemaHLSL.cpp
@@ -693,13 +693,19 @@ bool clang::CreateHLSLAttributedResourceType(
 // HLSL resource. The attributes are collected in HLSLResourcesTypeAttrs and at
 // the end of the declaration they are applied to the declaration type by
 // wrapping it in HLSLAttributedResourceType.
-bool SemaHLSL::handleResourceTypeAttr(const ParsedAttr &AL) {
-  Attr *A = nullptr;
+bool SemaHLSL::handleResourceTypeAttr(QualType T, const ParsedAttr &AL) {
+  // only allow resource type attributes on intangible types
+  if (!T->isHLSLResourceType()) {
+    Diag(AL.getLoc(), diag::err_hlsl_attribute_needs_intangible_type)
+        << AL << getASTContext().HLSLResourceTy;
+    return false;
+  }
 
   // validate number of arguments
   if (!AL.checkExactlyNumArgs(SemaRef, AL.getMinArgs()))
     return false;
 
+  Attr *A = nullptr;
   switch (AL.getKind()) {
   case ParsedAttr::AT_HLSLResourceClass: {
     if (!AL.isArgIdent(0)) {
diff --git a/clang/lib/Sema/SemaType.cpp b/clang/lib/Sema/SemaType.cpp
index 950bd6db0359d1..a7beb9d222c3b5 100644
--- a/clang/lib/Sema/SemaType.cpp
+++ b/clang/lib/Sema/SemaType.cpp
@@ -8860,7 +8860,7 @@ static void processTypeAttrs(TypeProcessingState &state, QualType &type,
       // decl-specifier-seq; do not collect attributes on declarations or those
       // that get to slide after declaration name.
       if (TAL == TAL_DeclSpec &&
-          state.getSema().HLSL().handleResourceTypeAttr(attr))
+          state.getSema().HLSL().handleResourceTypeAttr(type, attr))
         attr.setUsedAsTypeAttr();
       break;
     }
diff --git a/clang/test/ParserHLSL/hlsl_contained_type_attr_error.hlsl b/clang/test/ParserHLSL/hlsl_contained_type_attr_error.hlsl
index 1c37d72de86148..b2d492d95945c1 100644
--- a/clang/test/ParserHLSL/hlsl_contained_type_attr_error.hlsl
+++ b/clang/test/ParserHLSL/hlsl_contained_type_attr_error.hlsl
@@ -22,3 +22,7 @@ __hlsl_resource_t [[hlsl::resource_class(UAV)]] [[hlsl::contained_type(float)]]
 
 // expected-warning@+1{{attribute 'contained_type' is already applied with different arguments}}
 __hlsl_resource_t [[hlsl::resource_class(UAV)]] [[hlsl::contained_type(float)]] [[hlsl::contained_type(int)]] h8;
+
+// expected-error@+2{{attribute 'resource_class' can be used only on HLSL intangible type '__hlsl_resource_t'}}
+// expected-error@+1{{attribute 'contained_type' can be used only on HLSL intangible type '__hlsl_resource_t'}}
+float [[hlsl::resource_class(UAV)]] [[hlsl::contained_type(float)]] res5;
diff --git a/clang/test/ParserHLSL/hlsl_is_rov_attr_error.hlsl b/clang/test/ParserHLSL/hlsl_is_rov_attr_error.hlsl
index 15685bd1a3baa7..3b2c12e7a96c5c 100644
--- a/clang/test/ParserHLSL/hlsl_is_rov_attr_error.hlsl
+++ b/clang/test/ParserHLSL/hlsl_is_rov_attr_error.hlsl
@@ -14,3 +14,7 @@ __hlsl_resource_t [[hlsl::resource_class(UAV)]] [[hlsl::is_rov(gibberish)]] res3
 
 // expected-warning@+1{{attribute 'is_rov' is already applied}}
 __hlsl_resource_t [[hlsl::resource_class(UAV)]] [[hlsl::is_rov]] [[hlsl::is_rov]] res4;
+
+// expected-error@+2{{attribute 'resource_class' can be used only on HLSL intangible type '__hlsl_resource_t'}}
+// expected-error@+1{{attribute 'is_rov' can be used only on HLSL intangible type '__hlsl_resource_t'}}
+float [[hlsl::resource_class(UAV)]] [[hlsl::is_rov]] res5;
diff --git a/clang/test/ParserHLSL/hlsl_raw_buffer_attr_error.hlsl b/clang/test/ParserHLSL/hlsl_raw_buffer_attr_error.hlsl
index 83273426017ed0..77530cbf9e4d92 100644
--- a/clang/test/ParserHLSL/hlsl_raw_buffer_attr_error.hlsl
+++ b/clang/test/ParserHLSL/hlsl_raw_buffer_attr_error.hlsl
@@ -11,3 +11,7 @@ __hlsl_resource_t [[hlsl::resource_class(UAV)]] [[hlsl::raw_buffer(gibberish)]]
 
 // expected-warning@+1{{attribute 'raw_buffer' is already applied}}
 __hlsl_resource_t [[hlsl::resource_class(UAV)]] [[hlsl::raw_buffer]] [[hlsl::raw_buffer]] res4;
+
+// expected-error@+2{{attribute 'resource_class' can be used only on HLSL intangible type '__hlsl_resource_t'}}
+// expected-error@+1{{attribute 'raw_buffer' can be used only on HLSL intangible type '__hlsl_resource_t'}}
+float [[hlsl::resource_class(UAV)]] [[hlsl::raw_buffer]] res5;
diff --git a/clang/test/ParserHLSL/hlsl_resource_class_attr_error.hlsl b/clang/test/ParserHLSL/hlsl_resource_class_attr_error.hlsl
index 01ff1c007e2b57..63e39daff949b4 100644
--- a/clang/test/ParserHLSL/hlsl_resource_class_attr_error.hlsl
+++ b/clang/test/ParserHLSL/hlsl_resource_class_attr_error.hlsl
@@ -17,3 +17,6 @@ __hlsl_resource_t [[hlsl::resource_class(SRV)]] [[hlsl::resource_class(SRV)]] e4
 
 // expected-error@+1{{'resource_class' attribute takes one argument}}
 __hlsl_resource_t [[hlsl::resource_class(SRV, "aa")]] e5;
+
+// expected-error@+1{{attribute 'resource_class' can be used only on HLSL intangible type '__hlsl_resource_t'}}
+float [[hlsl::resource_class(UAV)]] e6;

damyanp
damyanp approved these changes Sep 26, 2024
View reviewed changes
Copy link
Contributor

@damyanp damyanp left a comment

Choose a reason for hiding this comment

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

LGTM

@hekota hekota merged commit 1d8fad9 into llvm:main Sep 26, 2024
12 checks passed
Sterling-Augustine pushed a commit to Sterling-Augustine/llvm-project that referenced this pull request Sep 27, 2024
@hekota @Sterling-Augustine
[HLSL] Allow resource type attributes only on __hlsl_resource_t (ll…
c2c6388 
…vm#110143)

Resource type attributes should only ever be used on the intangible type
`__hlsl_resource_t`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bogner bogner bogner approved these changes

@damyanp damyanp damyanp approved these changes

Assignees
No one assigned
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
HLSL Support
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@hekota @llvmbot @bogner @damyanp