[PATCH] D134067: [HLSL] Enable availability attribute

[Erich Keane via Phabricator via cfe-commits]

erichkeane accepted this revision.
erichkeane added a comment.
This revision is now accepted and ready to land.

Besides the request for re-organizing the the test files, LGTM.



================
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}}
----------------
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 :) ).


================
Comment at: clang/test/SemaHLSL/WaveBuiltinAvailability.hlsl:10
+
+// expected-note@* {{'WaveActiveCountBits' has been marked as being introduced in HLSL ShaderModel 6.0 here, but the deployment target is HLSL ShaderModel 5.0}}
----------------
Same here, though perhaps less-so.  You can use `@hlsl_intrinsics.h:14` I believe to specify the line as well (which would be nice!) but keeping the diagnostics in 'emitted order' is more important to me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134067