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@+2 {{'fn6_0' is only available on HLSL ShaderModel 6.0
or newer}}
+ // expected-note@+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@+3 {{... only available ...}}
// expected-note@#FN60_LOC {{ marked as being introduced...}}
// expected-note@+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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
