llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang @llvm/pr-subscribers-hlsl Author: Helena Kotas (hekota) <details> <summary>Changes</summary> 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`. --- Full diff: https://github.com/llvm/llvm-project/pull/107973.diff 6 Files Affected: - (modified) clang/lib/AST/TypePrinter.cpp (+1-1) - (modified) clang/lib/Sema/SemaHLSL.cpp (+4) - (modified) clang/lib/Sema/SemaType.cpp (+5-1) - (modified) clang/test/ParserHLSL/hlsl_is_rov_attr.hlsl (+3-3) - (modified) clang/test/ParserHLSL/hlsl_is_rov_attr_error.hlsl (+4-4) - (modified) clang/test/ParserHLSL/hlsl_resource_handle_attrs.hlsl (+1-1) ``````````diff 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@+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@+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@+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@+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@+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]; `````````` </details> https://github.com/llvm/llvm-project/pull/107973 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
