[clang] [HLSL] Warn on duplicate is_rov attribute; remove unnecessary parentheses (PR #107973)
Helena Kotas via cfe-commits
cfe-commits at lists.llvm.org
Mon Sep 9 22:21:28 PDT 2024
https://github.com/hekota created https://github.com/llvm/llvm-project/pull/107973
We should issue a warning whenever a duplicate resource type attribute is found. Currently we do that only for `resource_class`. This PR fixes that by checking for duplicate `is_rov` attributes as well.
Also removes unnecessary parenthesis on `is_rov`.
>From 1f63b9e4a0969f95c6efa8fa42a48307dea8fe28 Mon Sep 17 00:00:00 2001
From: Helena Kotas <hekotas at microsoft.com>
Date: Mon, 9 Sep 2024 22:14:57 -0700
Subject: [PATCH] [HLSL] Prohibit duplicate is_rov attributes; remove
unnecessary parentheses
---
clang/lib/AST/TypePrinter.cpp | 2 +-
clang/lib/Sema/SemaHLSL.cpp | 4 ++++
clang/lib/Sema/SemaType.cpp | 6 +++++-
clang/test/ParserHLSL/hlsl_is_rov_attr.hlsl | 6 +++---
clang/test/ParserHLSL/hlsl_is_rov_attr_error.hlsl | 8 ++++----
clang/test/ParserHLSL/hlsl_resource_handle_attrs.hlsl | 2 +-
6 files changed, 18 insertions(+), 10 deletions(-)
diff --git a/clang/lib/AST/TypePrinter.cpp b/clang/lib/AST/TypePrinter.cpp
index add6a5d10d61f7..be627a6242eb40 100644
--- a/clang/lib/AST/TypePrinter.cpp
+++ b/clang/lib/AST/TypePrinter.cpp
@@ -2077,7 +2077,7 @@ void TypePrinter::printHLSLAttributedResourceAfter(
<< HLSLResourceClassAttr::ConvertResourceClassToStr(Attrs.ResourceClass)
<< ")]]";
if (Attrs.IsROV)
- OS << " [[hlsl::is_rov()]]";
+ OS << " [[hlsl::is_rov]]";
}
void TypePrinter::printObjCInterfaceBefore(const ObjCInterfaceType *T,
diff --git a/clang/lib/Sema/SemaHLSL.cpp b/clang/lib/Sema/SemaHLSL.cpp
index 3b91303ac8cb8a..eb1b584b805c16 100644
--- a/clang/lib/Sema/SemaHLSL.cpp
+++ b/clang/lib/Sema/SemaHLSL.cpp
@@ -591,6 +591,10 @@ bool clang::CreateHLSLAttributedResourceType(Sema &S, QualType Wrapped,
break;
}
case attr::HLSLROV:
+ if (ResAttrs.IsROV) {
+ S.Diag(A->getLocation(), diag::warn_duplicate_attribute_exact) << A;
+ return false;
+ }
ResAttrs.IsROV = true;
break;
default:
diff --git a/clang/lib/Sema/SemaType.cpp b/clang/lib/Sema/SemaType.cpp
index 520dce870b7b78..e627fee51b66b8 100644
--- a/clang/lib/Sema/SemaType.cpp
+++ b/clang/lib/Sema/SemaType.cpp
@@ -8844,7 +8844,11 @@ static void processTypeAttrs(TypeProcessingState &state, QualType &type,
}
case ParsedAttr::AT_HLSLResourceClass:
case ParsedAttr::AT_HLSLROV: {
- if (state.getSema().HLSL().handleResourceTypeAttr(attr))
+ // Only collect HLSL resource type attributes that are in
+ // 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))
attr.setUsedAsTypeAttr();
break;
}
diff --git a/clang/test/ParserHLSL/hlsl_is_rov_attr.hlsl b/clang/test/ParserHLSL/hlsl_is_rov_attr.hlsl
index 24c85c6ccf7d74..cf21ec4d380dbf 100644
--- a/clang/test/ParserHLSL/hlsl_is_rov_attr.hlsl
+++ b/clang/test/ParserHLSL/hlsl_is_rov_attr.hlsl
@@ -1,16 +1,16 @@
// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-compute -x hlsl -ast-dump -o - %s | FileCheck %s
// CHECK: CXXRecordDecl 0x{{[0-9a-f]+}} {{.*}} struct MyBuffer definition
-// CHECK: FieldDecl 0x{{[0-9a-f]+}} <line:6:3, col:68> col:68 h '__hlsl_resource_t {{\[\[}}hlsl::resource_class(UAV)]] {{\[\[}}hlsl::is_rov()]]':'__hlsl_resource_t'
+// CHECK: FieldDecl 0x{{[0-9a-f]+}} <line:6:3, col:68> col:68 h '__hlsl_resource_t {{\[\[}}hlsl::resource_class(UAV)]] {{\[\[}}hlsl::is_rov]]':'__hlsl_resource_t'
struct MyBuffer {
__hlsl_resource_t [[hlsl::resource_class(UAV)]] [[hlsl::is_rov]] h;
};
-// CHECK: VarDecl 0x{{[0-9a-f]+}} <line:10:1, col:66> col:66 res '__hlsl_resource_t {{\[\[}}hlsl::resource_class(SRV)]] {{\[\[}}hlsl::is_rov()]]':'__hlsl_resource_t'
+// CHECK: VarDecl 0x{{[0-9a-f]+}} <line:10:1, col:66> col:66 res '__hlsl_resource_t {{\[\[}}hlsl::resource_class(SRV)]] {{\[\[}}hlsl::is_rov]]':'__hlsl_resource_t'
__hlsl_resource_t [[hlsl::is_rov]] [[hlsl::resource_class(SRV)]] res;
// CHECK: FunctionDecl 0x{{[0-9a-f]+}} <line:14:1, line:16:1> line:14:6 f 'void ()
-// CHECK: VarDecl 0x{{[0-9a-f]+}} <col:3, col:72> col:72 r '__hlsl_resource_t {{\[\[}}hlsl::resource_class(Sampler)]] {{\[\[}}hlsl::is_rov()]]':'__hlsl_resource_t'
+// CHECK: VarDecl 0x{{[0-9a-f]+}} <col:3, col:72> col:72 r '__hlsl_resource_t {{\[\[}}hlsl::resource_class(Sampler)]] {{\[\[}}hlsl::is_rov]]':'__hlsl_resource_t'
void f() {
__hlsl_resource_t [[hlsl::resource_class(Sampler)]] [[hlsl::is_rov]] r;
}
diff --git a/clang/test/ParserHLSL/hlsl_is_rov_attr_error.hlsl b/clang/test/ParserHLSL/hlsl_is_rov_attr_error.hlsl
index 68b2d9ecb190a8..15685bd1a3baa7 100644
--- a/clang/test/ParserHLSL/hlsl_is_rov_attr_error.hlsl
+++ b/clang/test/ParserHLSL/hlsl_is_rov_attr_error.hlsl
@@ -1,10 +1,10 @@
// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-compute -x hlsl -o - %s -verify
// expected-error at +1{{'is_rov' attribute cannot be applied to a declaration}}
-[[hlsl::is_rov()]] __hlsl_resource_t res0;
+[[hlsl::is_rov]] __hlsl_resource_t res0;
// expected-error at +1{{HLSL resource needs to have [[hlsl::resource_class()]] attribute}}
-__hlsl_resource_t [[hlsl::is_rov()]] res1;
+__hlsl_resource_t [[hlsl::is_rov]] res1;
// expected-error at +1{{'is_rov' attribute takes no arguments}}
__hlsl_resource_t [[hlsl::resource_class(UAV)]] [[hlsl::is_rov(3)]] res2;
@@ -12,5 +12,5 @@ __hlsl_resource_t [[hlsl::resource_class(UAV)]] [[hlsl::is_rov(3)]] res2;
// expected-error at +1{{use of undeclared identifier 'gibberish'}}
__hlsl_resource_t [[hlsl::resource_class(UAV)]] [[hlsl::is_rov(gibberish)]] res3;
-// duplicate attribute with the same meaning - no error
-__hlsl_resource_t [[hlsl::resource_class(UAV)]] [[hlsl::is_rov()]] [[hlsl::is_rov()]] res4;
+// expected-warning at +1{{attribute 'is_rov' is already applied}}
+__hlsl_resource_t [[hlsl::resource_class(UAV)]] [[hlsl::is_rov]] [[hlsl::is_rov]] res4;
diff --git a/clang/test/ParserHLSL/hlsl_resource_handle_attrs.hlsl b/clang/test/ParserHLSL/hlsl_resource_handle_attrs.hlsl
index 6324a11fc8a2df..7c3830a291970c 100644
--- a/clang/test/ParserHLSL/hlsl_resource_handle_attrs.hlsl
+++ b/clang/test/ParserHLSL/hlsl_resource_handle_attrs.hlsl
@@ -11,6 +11,6 @@ RWBuffer<float> Buffer1;
// CHECK: -TemplateArgument type 'vector<float, 4>'
// CHECK: `-ExtVectorType 0x{{[0-9a-f]+}} 'vector<float, 4>' 4
// CHECK: `-BuiltinType 0x{{[0-9a-f]+}} 'float'
-// CHECK: -FieldDecl 0x{{[0-9a-f]+}} <<invalid sloc>> <invalid sloc> implicit referenced h 'vector<float *, 4> {{\[\[}}hlsl::resource_class(UAV)]] {{\[\[}}hlsl::is_rov()]]':'vector<float *, 4>'
+// CHECK: -FieldDecl 0x{{[0-9a-f]+}} <<invalid sloc>> <invalid sloc> implicit referenced h 'vector<float *, 4> {{\[\[}}hlsl::resource_class(UAV)]] {{\[\[}}hlsl::is_rov]]':'vector<float *, 4>'
// CHECK: -HLSLResourceAttr 0x{{[0-9a-f]+}} <<invalid sloc>> Implicit TypedBuffer
RasterizerOrderedBuffer<vector<float, 4> > BufferArray3[4];
More information about the cfe-commits
mailing list