[PATCH] D114483: [SYCL] Add support for sycl_special_class attribute

[Mariya Podchishchaeva via Phabricator via cfe-commits]

Fznamznon added inline comments.


================
Comment at: clang/lib/Sema/SemaDecl.cpp:9118
+          NewFD->getKind() == Decl::Kind::CXXMethod &&
+          NewFD->getName() == "__init" && D.isFunctionDefinition()) {
+        if (auto *def = Parent->getDefinition())
----------------
In our downstream we have not only `__init`, but `__init_esimd` as well there. I wonder how this case should be handled?


================
Comment at: clang/lib/Sema/SemaDecl.cpp:16675-16683
+  if (auto *RD = dyn_cast<CXXRecordDecl>(Tag)) {
     FieldCollector->FinishClass();
+    if (RD->hasAttr<SYCLSpecialClassAttr>()) {
+      auto *def = RD->getDefinition();
+      if (def && !def->hasInitMethod())
+        Diag(RD->getLocation(),
+             diag::err_sycl_special_type_missing_init_method);
----------------
Code style correction.


================
Comment at: clang/lib/Sema/SemaDeclAttr.cpp:7762
+  // The 'sycl_special_class' attribute applies only to records.
+  const auto *RD = cast<CXXRecordDecl>(D);
+  assert(RD && "Record type is expected");
----------------
I wonder if this check is enforced by this line in Attr.td:
```
let Subjects = SubjectList<[CXXRecord]>;
```
I would assume that it is.



================
Comment at: clang/test/SemaSYCL/special-class-attribute.cpp:21
+
+class [[clang::sycl_special_class]] struct1 {
+  void __init(){};
----------------
Did you mean `struct` keyword here?


================
Comment at: clang/test/SemaSYCL/special-class-attribute.cpp:31
+};
+class special5 {};
+
----------------
Is this one needed?


================
Comment at: clang/test/SemaSYCL/special-class-attribute.cpp:43
+class [[clang::sycl_special_class]] class8 { // expected-error {{types with 'sycl_special_class' attribute must have an '__init' method defined}}
+  void __init();
+};
----------------
Will detection of `__init` method work correctly if it is defined outside of the class?
Maybe we should also mention that it should be not be defined in a separate library.


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

https://reviews.llvm.org/D114483