[PATCH] D134067: [HLSL] Enable availability attribute

Chris Bieneman via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Sep 16 13:25:15 PDT 2022


beanz added inline comments.


================
Comment at: clang/test/SemaHLSL/AvailabilityMarkup.hlsl:15
+void fn() {
+    // expected-warning at +2 {{'fn6_0' is only available on HLSL ShaderModel 6.0 or newer}}
+    // expected-note at +1 {{enclose 'fn6_0' in a __builtin_available check to silence this warning}}
----------------
erichkeane wrote:
> So minor thing, and one that I am going to start pushing for more, is better organization of 'expected-diagnostics'.  I think they should better reflect the order and 'cause' of the diagnostic, particularly with notes.  So something like:
> 
> ``` 
>   // expected-warning at +3 {{... only available ...}}
>   // expected-note@#FN60_LOC {{ marked as being introduced...}}
>   // expected-note at +1{{enclose...}}
> ```
> 
> Then on line 5, add the comment: `// FN60`
> 
> I have this preference because it makes it more clear to the reader where the notes come from.  I realize this is a new direction, and perhaps pushing my ability to request, but I'd still appreciate it happening here (I WILL be pushing for it on template reviews, where I think it is even MORE beneficial, thanks to 'instantiation of' diagnostics, and I have the authority to demand it :) ).
I was completely unaware of the ability to create labels like that. That's awesome. I'll update the tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134067



More information about the cfe-commits mailing list