aaron.ballman added inline comments.
================ Comment at: clang/include/clang/Basic/AttrDocs.td:6557-6558 +Here're resource binding examples with and without space: +``RWBuffer<float> Uav : register(u3, space1)`` +``Buffer<float> Buf : register(t1)`` +The full documentation is available here: https://docs.microsoft.com/en-us/windows/win32/direct3d12/resource-binding-in-hlsl ---------------- Two things: * The example doesn't use valid RST, it should look more like what's done on AttrDocs:649 (using `.. code-block:: c++`) * I think the example is missing semicolons at the end of the declarations? ================ Comment at: clang/lib/Parse/ParseHLSL.cpp:112-113 + } + if (!Tok.is(tok::identifier)) { + Diag(Tok.getLocation(), diag::err_expected) << tok::identifier; + SkipUntil(tok::r_paren, StopAtSemi); // skip through ) ---------------- python3kgae wrote: > aaron.ballman wrote: > > Any reason not to use `ExpectAndConsume(tok::identifier, > > diag::err_expected)` here instead? > Need to save the identifier string and loc for later use. > And the consume is done in ParseIdentifierLoc. Ah, that's a good reason, thanks! ================ Comment at: clang/lib/Sema/SemaDecl.cpp:2893-2894 + else if (const auto *RB = dyn_cast<HLSLResourceBindingAttr>(Attr)) + NewAttr = + S.mergeHLSLResourceBindingAttr(D, *RB, RB->getSlot(), RB->getSpace()); else if (Attr->shouldInheritEvenIfAlreadyPresent() || !DeclHasAttr(D, Attr)) ---------------- I don't see any tests covering this code path. How do you get such a redeclaration in HLSL? ================ Comment at: clang/test/SemaHLSL/resource_binding_attr_error.hlsl:15 +// expected-error@+1 {{invalid space specifier 's2' used; expected 'space' followed by an integer, like space1}} +cbuffer a : register(b0, s2) { + ---------------- Isn't this a re-definition of `a` which would cause an error? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D130033/new/ https://reviews.llvm.org/D130033 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
