[clang] [SYCL] Basic diagnostics for the sycl_kernel_entry_point attribute. (PR #120327)

Aaron Ballman via cfe-commits cfe-commits at lists.llvm.org
Wed Jan 8 11:13:34 PST 2025


================
@@ -15978,6 +15988,24 @@ Decl *Sema::ActOnFinishFunctionBody(Decl *dcl, Stmt *Body,
       CheckCoroutineWrapper(FD);
   }
 
+  // Diagnose invalid SYCL kernel entry point function declarations.
+  if (FD && !FD->isInvalidDecl() && !FD->isTemplated() &&
+      FD->hasAttr<SYCLKernelEntryPointAttr>()) {
+    if (FD->isDeleted()) {
+      Diag(FD->getAttr<SYCLKernelEntryPointAttr>()->getLocation(),
+           diag::err_sycl_entry_point_invalid)
+          << /*deleted function*/ 2;
+    } else if (FD->isDefaulted()) {
+      Diag(FD->getAttr<SYCLKernelEntryPointAttr>()->getLocation(),
+           diag::err_sycl_entry_point_invalid)
+          << /*defaulted function*/ 3;
+    } else if (FSI->isCoroutine()) {
+      Diag(FD->getAttr<SYCLKernelEntryPointAttr>()->getLocation(),
+           diag::err_sycl_entry_point_invalid)
+          << /*coroutine*/ 7;
----------------
AaronBallman wrote:

It's a judgement call either way, IMO. Personally, I dislike using one-shot enumerations because they're overkill for what they're used for. The in-line comment saying "this value corresponds to that selection" is plenty to keep the code readable. However, I *do* like using enumerations when the enum is used for selecting the diagnostic *and* some other purpose. e.g.,
```
AwesomeSauce Result = CheckSomeCondition(Whatever);
if (Result != AwesomeSauce::Good)
  Diag(Loc, diag::dont_do_that) << Result;
else
  Diag(Loc, diag::congrats) << AwesomeSauce::Fantastic;
```
In this specific case, I lean towards not needing the enumeration because then we have to figure out where that lives (Are we making `Sema` even more complicated because the diagnostics are in multiple files? Are we adding a new header? Something else?), and that will lead to inconsistency as we add new diagnostics because some folks will add unscoped enums, others will use scoped enums, and the enums will likely live in various different places in the source.

That said, coming up with some tablegen syntax that allows someone to associate enumerations with the diagnostic definition itself would be a pretty interesting idea to explore because then we'd get something that could be consistently applied and doesn't require as much maintenance and review effort. e.g.,
```
def err_sycl_entry_point_invalid : Error<
  "'sycl_kernel_entry_point' attribute cannot be applied to a"
  " %enum_select{"
  "%enum{NonStaticMemberFunc}non-static member function|"
  "%enum{VariadicFunc}variadic function|"
  "%enum{DeletedFunc}deleted function|"
  "%enum{DefaultedFunc}defaulted function|...<you get the idea>}0">;
```
that code then be used like:
```
Diag(Loc, diag::err_sycl_entry_point_invalid) << diag::err_sycl_entry_point_invalid::NonStaticMemberFunction;
```
(Though I think we may want to find some better place for the enum to live so that uses in calls to `Diag()` are more succinct. Obviously there's a lot of design work left to be done with the idea.)

https://github.com/llvm/llvm-project/pull/120327


More information about the cfe-commits mailing list