aaron.ballman accepted this revision. aaron.ballman added a comment. LGTM with some testing requests.
================ Comment at: clang/test/SemaSYCL/kernel-attribute.cpp:4-5 + +__attribute((sycl_kernel)) void foo() { +} ---------------- aaron.ballman wrote: > Missing some tests: > * test that both attributes can be applied to whatever subjects they > appertain to > * test that neither attribute can be applied to an incorrect subject > * test that the attributes do not accept arguments > * test that the attribute is ignored when SYCL is not enabled > > Are there situations where the attribute does not make sense, such as member > functions, virtual functions, etc? If so, those are good test cases (and > diagnostics) to add as well. Still missing a test that the attribute is ignored when SYCL is not enabled. ================ Comment at: clang/test/SemaSYCL/kernel-attribute.cpp:6 + +__attribute__((sycl_kernel(1))) void foo(); // expected-warning {{'sycl_kernel' attribute only applies to function templates}} +[[clang::sycl_kernel(1)]] void foo2(); // expected-warning {{'sycl_kernel' attribute only applies to function templates}} ---------------- This test should be on a templated function (we already demonstrated it only applies to templated functions, so the check for the argument is not what is failing). ================ Comment at: clang/test/SemaSYCL/kernel-attribute.cpp:7 +__attribute__((sycl_kernel(1))) void foo(); // expected-warning {{'sycl_kernel' attribute only applies to function templates}} +[[clang::sycl_kernel(1)]] void foo2(); // expected-warning {{'sycl_kernel' attribute only applies to function templates}} + ---------------- Same here. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60455/new/ https://reviews.llvm.org/D60455 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits