[PATCH] D130033: [HLSL] Add resource binding attribute for HLSL.

[Xiang Li via Phabricator via cfe-commits]

python3kgae added inline comments.


================
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))
----------------
aaron.ballman wrote:
> I don't see any tests covering this code path. How do you get such a redeclaration in HLSL?
It should not.
I'll remove this code.


================
Comment at: clang/test/SemaHLSL/resource_binding_attr_error.hlsl:15
+// expected-error at +1 {{invalid space specifier 's2' used; expected 'space' followed by an integer, like space1}}
+cbuffer a : register(b0, s2) {
+
----------------
aaron.ballman wrote:
> Isn't this a re-definition of `a` which would cause an error?
The name for cbuffer is not used.
And no error should be reported for re-definition.

Discussed with the team, have to keep it like this for back-compat reason.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130033/new/

https://reviews.llvm.org/D130033