Skip to content

Commit 61372fc

Browse files
authored
[HLSL] Warn on duplicate is_rov attribute; remove unnecessary parentheses (#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`.
1 parent f4e2d7b commit 61372fc

File tree

6 files changed

+18
-10
lines changed

6 files changed

+18
-10
lines changed

clang/lib/AST/TypePrinter.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2077,7 +2077,7 @@ void TypePrinter::printHLSLAttributedResourceAfter(
20772077
<< HLSLResourceClassAttr::ConvertResourceClassToStr(Attrs.ResourceClass)
20782078
<< ")]]";
20792079
if (Attrs.IsROV)
2080-
OS << " [[hlsl::is_rov()]]";
2080+
OS << " [[hlsl::is_rov]]";
20812081
}
20822082

20832083
void TypePrinter::printObjCInterfaceBefore(const ObjCInterfaceType *T,

clang/lib/Sema/SemaHLSL.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -592,6 +592,10 @@ bool clang::CreateHLSLAttributedResourceType(Sema &S, QualType Wrapped,
592592
break;
593593
}
594594
case attr::HLSLROV:
595+
if (ResAttrs.IsROV) {
596+
S.Diag(A->getLocation(), diag::warn_duplicate_attribute_exact) << A;
597+
return false;
598+
}
595599
ResAttrs.IsROV = true;
596600
break;
597601
default:

clang/lib/Sema/SemaType.cpp

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8844,7 +8844,11 @@ static void processTypeAttrs(TypeProcessingState &state, QualType &type,
88448844
}
88458845
case ParsedAttr::AT_HLSLResourceClass:
88468846
case ParsedAttr::AT_HLSLROV: {
8847-
if (state.getSema().HLSL().handleResourceTypeAttr(attr))
8847+
// Only collect HLSL resource type attributes that are in
8848+
// decl-specifier-seq; do not collect attributes on declarations or those
8849+
// that get to slide after declaration name.
8850+
if (TAL == TAL_DeclSpec &&
8851+
state.getSema().HLSL().handleResourceTypeAttr(attr))
88488852
attr.setUsedAsTypeAttr();
88498853
break;
88508854
}
Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,16 @@
11
// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-compute -x hlsl -ast-dump -o - %s | FileCheck %s
22

33
// CHECK: CXXRecordDecl 0x{{[0-9a-f]+}} {{.*}} struct MyBuffer definition
4-
// 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'
4+
// 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'
55
struct MyBuffer {
66
__hlsl_resource_t [[hlsl::resource_class(UAV)]] [[hlsl::is_rov]] h;
77
};
88

9-
// 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'
9+
// 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'
1010
__hlsl_resource_t [[hlsl::is_rov]] [[hlsl::resource_class(SRV)]] res;
1111

1212
// CHECK: FunctionDecl 0x{{[0-9a-f]+}} <line:14:1, line:16:1> line:14:6 f 'void ()
13-
// 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'
13+
// 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'
1414
void f() {
1515
__hlsl_resource_t [[hlsl::resource_class(Sampler)]] [[hlsl::is_rov]] r;
1616
}
Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,16 @@
11
// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-compute -x hlsl -o - %s -verify
22

33
// expected-error@+1{{'is_rov' attribute cannot be applied to a declaration}}
4-
[[hlsl::is_rov()]] __hlsl_resource_t res0;
4+
[[hlsl::is_rov]] __hlsl_resource_t res0;
55

66
// expected-error@+1{{HLSL resource needs to have [[hlsl::resource_class()]] attribute}}
7-
__hlsl_resource_t [[hlsl::is_rov()]] res1;
7+
__hlsl_resource_t [[hlsl::is_rov]] res1;
88

99
// expected-error@+1{{'is_rov' attribute takes no arguments}}
1010
__hlsl_resource_t [[hlsl::resource_class(UAV)]] [[hlsl::is_rov(3)]] res2;
1111

1212
// expected-error@+1{{use of undeclared identifier 'gibberish'}}
1313
__hlsl_resource_t [[hlsl::resource_class(UAV)]] [[hlsl::is_rov(gibberish)]] res3;
1414

15-
// duplicate attribute with the same meaning - no error
16-
__hlsl_resource_t [[hlsl::resource_class(UAV)]] [[hlsl::is_rov()]] [[hlsl::is_rov()]] res4;
15+
// expected-warning@+1{{attribute 'is_rov' is already applied}}
16+
__hlsl_resource_t [[hlsl::resource_class(UAV)]] [[hlsl::is_rov]] [[hlsl::is_rov]] res4;

clang/test/ParserHLSL/hlsl_resource_handle_attrs.hlsl

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,6 @@ RWBuffer<float> Buffer1;
1111
// CHECK: -TemplateArgument type 'vector<float, 4>'
1212
// CHECK: `-ExtVectorType 0x{{[0-9a-f]+}} 'vector<float, 4>' 4
1313
// CHECK: `-BuiltinType 0x{{[0-9a-f]+}} 'float'
14-
// 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>'
14+
// 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>'
1515
// CHECK: -HLSLResourceAttr 0x{{[0-9a-f]+}} <<invalid sloc>> Implicit TypedBuffer
1616
RasterizerOrderedBuffer<vector<float, 4> > BufferArray3[4];

0 commit comments

Comments
 (0)