beanz marked an inline comment as done.
beanz added a comment.

Thank you! Will update shortly.



================
Comment at: clang/include/clang/Basic/AttributeCommonInfo.h:47-48
 
     // Note TableGen depends on the order above.  Do not add or change the 
order
     // without adding related code to TableGen/ClangAttrEmitter.cpp.
     /// Context-sensitive version of a keyword attribute.
----------------
aaron.ballman wrote:
> Should this comment now move down a bit?
I'm struggling a bit with what that comment is referring to, and I feel like 
I'm missing something obvious. If you change the order of any of the cases 
`AS_ContextSensitiveKeyword` or earlier in the list, things start breaking. 
Specifically putting `AS_HLSLSemantic` above `AS_ContextSensitiveKeyword` 
breaks parsing context sensitive keywords...

I'm clearly missing something...

That said, I can add a new case after `AS_ContextSensitiveKeyword` before 
`AS_HLSLSemantic` and it all still works fine.

With my current understanding, I'm not sure moving the comment makes sense, but 
I could expand the comment. Thoughts?


================
Comment at: clang/test/SemaHLSL/Semantics/semantic_parsing.hlsl:4-7
+void Entry(int GI : ) { }
+
+// expected-error@+1 {{unknown HLSL semantic 'SV_IWantAPony'}}
+void Pony(int GI : SV_IWantAPony) { }
----------------
aaron.ballman wrote:
> This isn't really a Sema test because it's testing parsing behavior -- 
> thoughts on adding `ParserHLSL` as a test directory (or do you expect so few 
> parsing changes for this that it'd be a waste)?
Adding a ParserHLSL directory makes sense. I think we'll have enough cases to 
justify it :).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122699

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to