Add additional test cases and address review comments

Author: schittir

clang/include/clang/Basic/DiagnosticSemaKinds.td

@@ -12882,11 +12882,10 @@ def err_sycl_special_type_num_init_method : Error<
 // SYCL external attribute diagnostics
 def err_sycl_attribute_invalid_linkage : Error<
   "'sycl_external' can only be applied to functions with external linkage">;
-def err_sycl_attribute_avoid_main : Error<
+def err_sycl_attribute_invalid_main : Error<
   "'sycl_external' cannot be applied to the 'main' function">;
-def err_sycl_attribute_avoid_deleted_function
-    : Error<"'sycl_external' cannot be applied to an explicitly deleted "
-            "function">;
+def err_sycl_attribute_invalid_deleted_function : Error<
+  "'sycl_external' cannot be applied to an explicitly deleted function">;
 
 // SYCL kernel entry point diagnostics
 def err_sycl_entry_point_invalid : Error<

clang/lib/Sema/SemaDecl.cpp

@@ -4085,10 +4085,10 @@ bool Sema::MergeFunctionDecl(FunctionDecl *New, NamedDecl *&OldD, Scope *S,
     }
 
     // SYCL 2020 section 5.10.1, "SYCL functions and member functions linkage":
-    //   When a function is declared with sycl_external, that attribute must be
+    //   When a function is declared with SYCL_EXTERNAL, that macro must be
     //   used on the first declaration of that function in the translation unit.
     //   Redeclarations of the function in the same translation unit may
-    //   optionally use sycl_external, but this is not required.
+    //   optionally use SYCL_EXTERNAL, but this is not required.
     if (LangOpts.SYCLIsDevice) {
       const SYCLExternalAttr *SEA = New->getAttr<SYCLExternalAttr>();
       if (SEA && !Old->hasAttr<SYCLExternalAttr>()) {
@@ -12459,7 +12459,7 @@ void Sema::CheckMain(FunctionDecl *FD, const DeclSpec &DS) {
 
   if (getLangOpts().SYCLIsDevice) {
     if (FD->hasAttr<SYCLExternalAttr>()) {
-      Diag(FD->getLocation(), diag::err_sycl_attribute_avoid_main);
+      Diag(FD->getLocation(), diag::err_sycl_attribute_invalid_main);
       FD->setInvalidDecl();
       return;
     }

clang/lib/Sema/SemaSYCL.cpp

@@ -257,7 +257,7 @@ void SemaSYCL::CheckSYCLExternalFunctionDecl(FunctionDecl *FD) {
     }
     if (FD->isDeletedAsWritten()) {
       Diag(SEAttr->getLocation(),
-           diag::err_sycl_attribute_avoid_deleted_function);
+           diag::err_sycl_attribute_invalid_deleted_function);
       return;
     }
   }

clang/test/SemaSYCL/sycl-external-attr-grammar.cpp

@@ -1,19 +1,16 @@
 // RUN: %clang_cc1 -fsycl-is-device -fsyntax-only -verify %s
 
-// expected-error@+1{{'clang::sycl_external' attribute only applies to functions}}
-[[clang::sycl_external]] int a;
+// expected-error@+1{{'clang::sycl_external' attribute takes no arguments}}
+[[clang::sycl_external(3)]] void bar() {}
 
+// FIXME: this case should be diagnosed too
+[[clang::sycl_external()]] void bad1();
 
-// expected-error@+2{{'clang::sycl_external' attribute only applies to functions}}
-struct s {
-[[clang::sycl_external]] int b;
-};
+// expected-error@+1{{expected expression}}
+[[clang::sycl_external(,)]] void bad2();
 
 // expected-error@+1{{'clang::sycl_external' attribute takes no arguments}}
-[[clang::sycl_external(3)]] void bar() {}
-
-// FIXME: The first declaration of a function is required to have the attribute.
-// The attribute may be optionally present on subsequent declarations
-int foo(int c);
+[[clang::sycl_external(3)]] void bad3();
 
-[[clang::sycl_external]] void foo();
+// expected-error@+1{{expected expression}}
+[[clang::sycl_external(4,)]] void bad4();

clang/test/SemaSYCL/sycl-external-attr-ignored.cpp

@@ -1,12 +1,15 @@
 // RUN: %clang_cc1 -fsyntax-only -verify %s
 
+// These tests validate that the sycl_external attribute is ignored when SYCL
+// support is not enabled.
+
 // expected-warning@+1{{'clang::sycl_external' attribute ignored}}
 [[clang::sycl_external]] void bar() {}
 
 // expected-warning@+1{{'clang::sycl_external' attribute ignored}}
 [[clang::sycl_external]] int a;
 
 // expected-warning@+2{{'clang::sycl_external' attribute ignored}}
-namespace not_sycl {
-[[clang::sycl_external]] void foo() {}
-}
+template<typename T>
+[[clang::sycl_external]] void ft(T) {}
+template void ft(int);

clang/test/SemaSYCL/sycl-external-attr.cpp

@@ -1,45 +1,73 @@
-// RUN: %clang_cc1 -fsycl-is-device -fsyntax-only -verify %s
+// RUN: %clang_cc1 -fsycl-is-device -std=c++17 -fsyntax-only -verify -DCPP17 %s
 // RUN: %clang_cc1 -fsycl-is-device -std=c++20 -fsyntax-only -verify -DCPP20 %s
-// Semantic tests for sycl_external attribute
 
-[[clang::sycl_external]] // expected-error {{'sycl_external' can only be applied to functions with external linkage}}
+// Semantic tests for the sycl_external attribute.
+
+// expected-error@+1{{'sycl_external' can only be applied to functions with external linkage}}
+[[clang::sycl_external]]
 static void func1() {}
 
+// expected-error@+2{{'sycl_external' can only be applied to functions with external linkage}}
 namespace {
-  [[clang::sycl_external]] // expected-error {{'sycl_external' can only be applied to functions with external linkage}}
+  [[clang::sycl_external]]
   void func2() {}
 
   struct UnnX {};
 }
 
-[[clang::sycl_external]] // expected-error {{'sycl_external' can only be applied to functions with external linkage}}
-  void func4(UnnX) {}
+// expected-error@+2{{'sycl_external' can only be applied to functions with external linkage}}
+namespace { struct S4 {}; }
+[[clang::sycl_external]] void func4(S4) {}
+
+// FIXME: This case is currently being diagnosed as an error because clang implements
+// default inheritance of attribute and explicit instantiation declaration names the
+// symbol that causes the instantiated specialization to have internal linkage.
+// expected-error@+3{{'sycl_external' can only be applied to functions with external linkage}}
+namespace { struct S6 {}; }
+template<typename>
+[[clang::sycl_external]] void func6() {}
+template void func6<S6>();
+// expected-note@-1{{in instantiation of function template specialization 'func6<(anonymous namespace)::S6>' requested here}}
+
+// expected-error@+3 2{{'sycl_external' can only be applied to functions with external linkage}}
+namespace { struct S7 {}; }
+template<typename>
+[[clang::sycl_external]] void func7();
+template<> void func7<S7>() {}
+// expected-note@-1{{in instantiation of function template specialization 'func7<(anonymous namespace)::S7>' requested here}}
+
+namespace { struct S8 {}; }
+template<typename>
+void func8();
+template<> [[clang::sycl_external]] void func8<S8>() {}
+// expected-error@-1{{'clang::sycl_external' attribute does not appear on the first declaration}}
+// expected-note@-2{{previous declaration is here}}
 
 // The first declaration of a SYCL external function is required to have this attribute.
-int foo(); // expected-note {{previous declaration is here}}
-
-[[clang::sycl_external]] int foo(); // expected-error {{'clang::sycl_external' attribute does not appear on the first declaration}}
+// expected-note@+1{{previous declaration is here}}
+int foo();
+// expected-error@+1{{'clang::sycl_external' attribute does not appear on the first declaration}}
+[[clang::sycl_external]] int foo();
 
 // Subsequent declrations of a SYCL external function may optionally specify this attribute.
 [[clang::sycl_external]] int boo();
-
 [[clang::sycl_external]] int boo(); // OK
-
 int boo(); // OK
 
 class C {
   [[clang::sycl_external]] void member();
 };
 
-[[clang::sycl_external]] int main() // expected-error {{'sycl_external' cannot be applied to the 'main' function}}
+// expected-error@+1{{'sycl_external' cannot be applied to the 'main' function}}
+[[clang::sycl_external]] int main()
 {
   return 0;
 }
 
+// expected-error@+2{{'sycl_external' cannot be applied to an explicitly deleted function}}
 class D {
-  [[clang::sycl_external]] void del() = delete; // expected-error {{'sycl_external' cannot be applied to an explicitly deleted function}}
+  [[clang::sycl_external]] void del() = delete;
 };
-
 struct NonCopyable {
   ~NonCopyable() = delete;
   [[clang::sycl_external]] NonCopyable(const NonCopyable&) = default;
@@ -49,7 +77,8 @@ class A {
   [[clang::sycl_external]]
   A() {}
 
-  [[clang::sycl_external]] void func3() {}
+  [[clang::sycl_external]] void mf() {}
+  [[clang::sycl_external]] static void smf();
 };
 
 class B {
@@ -59,12 +88,18 @@ class B {
   [[clang::sycl_external]] virtual void bar() = 0;
 };
 
-[[clang::sycl_external]] int *func0() { return nullptr; }
-
-[[clang::sycl_external]] void func2(int *) {}
-
 [[clang::sycl_external]] constexpr int square(int x);
 
-#ifdef CPP20
+// Devices that do not support the generic address space shall not specify
+// a raw pointer or reference type as the return type or as a parameter type.
+[[clang::sycl_external]] int *fun0();
+[[clang::sycl_external]] int &fun1();
+[[clang::sycl_external]] int &&fun2();
+[[clang::sycl_external]] void fun3(int *);
+[[clang::sycl_external]] void fun4(int &);
+[[clang::sycl_external]] void fun5(int &&);
+
+#if CPP20
 [[clang::sycl_external]] consteval int func();
 #endif
+