[Sema] Diagnose addr space mismatch while constructing objects

If we construct an object in some arbitrary non-default addr space
it should fail unless either:
- There is an implicit conversion from the address space to default
/generic address space.
- There is a matching ctor qualified with an address space that is
either exactly matching or convertible to the address space of an
object.

Differential Revision: https://reviews.llvm.org/D62156

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@363944 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td
index 9cb2a769f4f8688522d49a82f08db14824e8c68a..dff40055bbc4796729a1f60c28543de623f4a7d8 100644 (file)
--- a/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/include/clang/Basic/DiagnosticSemaKinds.td
@@ -3683,6 +3683,9 @@ def note_ovl_candidate_inherited_constructor_slice : Note<
 def note_ovl_candidate_illegal_constructor : Note<
     "candidate %select{constructor|template}0 ignored: "
     "instantiation %select{takes|would take}0 its own class type by value">;
+def note_ovl_candidate_illegal_constructor_adrspace_mismatch : Note<
+    "candidate constructor ignored: cannot be used to construct an object "
+    "in address space %0">;
 def note_ovl_candidate_bad_deduction : Note<
     "candidate template ignored: failed template argument deduction">;
 def note_ovl_candidate_incomplete_deduction : Note<"candidate template ignored: "

diff --git a/include/clang/Sema/Overload.h b/include/clang/Sema/Overload.h
index cacb592b18437cf621b7c72e119c3fe495a4e624..96aadeac2ba389a5a042a0cd6eeb5c00b0a0558b 100644 (file)
--- a/include/clang/Sema/Overload.h
+++ b/include/clang/Sema/Overload.h
@@ -723,6 +723,11 @@ class Sema;
     /// This candidate was not viable because it is a non-default multiversioned
     /// function.
     ovl_non_default_multiversion_function,
+
+    /// This constructor/conversion candidate fail due to an address space
+    /// mismatch between the object being constructed and the overload
+    /// candidate.
+    ovl_fail_object_addrspace_mismatch
   };
 
   /// A list of implicit conversion sequences for the arguments of an
@@ -878,6 +883,9 @@ class Sema;
     unsigned NumInlineBytesUsed = 0;
     llvm::AlignedCharArray<alignof(void *), NumInlineBytes> InlineSpace;
 
+    // Address space of the object being constructed.
+    LangAS DestAS = LangAS::Default;
+
     /// If we have space, allocates from inline storage. Otherwise, allocates
     /// from the slab allocator.
     /// FIXME: It would probably be nice to have a SmallBumpPtrAllocator
@@ -983,6 +991,17 @@ class Sema;
                         ArrayRef<OverloadCandidate *> Cands,
                         StringRef Opc = "",
                         SourceLocation OpLoc = SourceLocation());
+
+    LangAS getDestAS() { return DestAS; }
+
+    void setDestAS(LangAS AS) {
+      assert((Kind == CSK_InitByConstructor ||
+              Kind == CSK_InitByUserDefinedConversion) &&
+             "can't set the destination address space when not constructing an "
+             "object");
+      DestAS = AS;
+    }
+
   };
 
   bool isBetterOverloadCandidate(Sema &S,

diff --git a/lib/Sema/SemaDeclCXX.cpp b/lib/Sema/SemaDeclCXX.cpp
index 6686b855c3b1e35d1a0e5ddde95624eb086a068d..6dda693c6ad0c74738645e2205b78dc4ae645e03 100644 (file)
--- a/lib/Sema/SemaDeclCXX.cpp
+++ b/lib/Sema/SemaDeclCXX.cpp
@@ -8232,12 +8232,19 @@ QualType Sema::CheckConstructorDeclarator(Declarator &D, QualType R,
 
   DeclaratorChunk::FunctionTypeInfo &FTI = D.getFunctionTypeInfo();
   if (FTI.hasMethodTypeQualifiers()) {
+    bool DiagOccured = false;
     FTI.MethodQualifiers->forEachQualifier(
         [&](DeclSpec::TQ TypeQual, StringRef QualName, SourceLocation SL) {
+          // This diagnostic should be emitted on any qualifier except an addr
+          // space qualifier. However, forEachQualifier currently doesn't visit
+          // addr space qualifiers, so there's no way to write this condition
+          // right now; we just diagnose on everything.
           Diag(SL, diag::err_invalid_qualified_constructor)
               << QualName << SourceRange(SL);
+          DiagOccured = true;
         });
-    D.setInvalidType();
+    if (DiagOccured)
+      D.setInvalidType();
   }
 
   // C++0x [class.ctor]p4:

diff --git a/lib/Sema/SemaInit.cpp b/lib/Sema/SemaInit.cpp
index d5ef5eddf3e8d568fb596adb61b32c102eec0c36..16c396890a90ca8f9481de38b298185883539764 100644 (file)
--- a/lib/Sema/SemaInit.cpp
+++ b/lib/Sema/SemaInit.cpp
@@ -3733,6 +3733,7 @@ ResolveConstructorOverload(Sema &S, SourceLocation DeclLoc,
                            bool OnlyListConstructors, bool IsListInit,
                            bool SecondStepOfCopyInit = false) {
   CandidateSet.clear(OverloadCandidateSet::CSK_InitByConstructor);
+  CandidateSet.setDestAS(DestType.getQualifiers().getAddressSpace());
 
   for (NamedDecl *D : Ctors) {
     auto Info = getConstructorInfo(D);
@@ -4985,6 +4986,7 @@ static void TryUserDefinedConversion(Sema &S,
   // structure, so that it will persist if we fail.
   OverloadCandidateSet &CandidateSet = Sequence.getFailedCandidateSet();
   CandidateSet.clear(OverloadCandidateSet::CSK_InitByUserDefinedConversion);
+  CandidateSet.setDestAS(DestType.getQualifiers().getAddressSpace());
 
   // Determine whether we are allowed to call explicit constructors or
   // explicit conversion operators.

diff --git a/lib/Sema/SemaOverload.cpp b/lib/Sema/SemaOverload.cpp
index 70bb757607d29b231a8d7522a79d94430d9e792c..11595fac2cb6bd8757c8dc7f4027b8a9dd03e558 100644 (file)
--- a/lib/Sema/SemaOverload.cpp
+++ b/lib/Sema/SemaOverload.cpp
@@ -6101,6 +6101,15 @@ void Sema::AddOverloadCandidate(
         return;
       }
     }
+
+    // Check that the constructor is capable of constructing an object in the
+    // destination address space.
+    if (!Qualifiers::isAddressSpaceSupersetOf(
+            Constructor->getMethodQualifiers().getAddressSpace(),
+            CandidateSet.getDestAS())) {
+      Candidate.Viable = false;
+      Candidate.FailureKind = ovl_fail_object_addrspace_mismatch;
+    }
   }
 
   unsigned NumParams = Proto->getNumParams();
@@ -10392,9 +10401,12 @@ static void DiagnoseOpenCLExtensionDisabled(Sema &S, OverloadCandidate *Cand) {
 /// It would be great to be able to express per-candidate problems
 /// more richly for those diagnostic clients that cared, but we'd
 /// still have to be just as careful with the default diagnostics.
+/// \param CtorDestAS Addr space of object being constructed (for ctor
+/// candidates only).
 static void NoteFunctionCandidate(Sema &S, OverloadCandidate *Cand,
                                   unsigned NumArgs,
-                                  bool TakingCandidateAddress) {
+                                  bool TakingCandidateAddress,
+                                  LangAS CtorDestAS = LangAS::Default) {
   FunctionDecl *Fn = Cand->Function;
 
   // Note deleted candidates, but only if they're viable.
@@ -10432,6 +10444,16 @@ static void NoteFunctionCandidate(Sema &S, OverloadCandidate *Cand,
     return;
   }
 
+  case ovl_fail_object_addrspace_mismatch: {
+    Qualifiers QualsForPrinting;
+    QualsForPrinting.setAddressSpace(CtorDestAS);
+    S.Diag(Fn->getLocation(),
+           diag::note_ovl_candidate_illegal_constructor_adrspace_mismatch)
+        << QualsForPrinting;
+    MaybeEmitInheritedConstructorNote(S, Cand->FoundDecl);
+    return;
+  }
+
   case ovl_fail_trivial_conversion:
   case ovl_fail_bad_final_conversion:
   case ovl_fail_final_conversion_not_exact:
@@ -10862,7 +10884,7 @@ void OverloadCandidateSet::NoteCandidates(Sema &S, ArrayRef<Expr *> Args,
 
     if (Cand->Function)
       NoteFunctionCandidate(S, Cand, Args.size(),
-                            /*TakingCandidateAddress=*/false);
+                            /*TakingCandidateAddress=*/false, DestAS);
     else if (Cand->IsSurrogate)
       NoteSurrogateCandidate(S, Cand);
     else {

diff --git a/test/CodeGenCXX/address-space-of-this.cpp b/test/CodeGenCXX/address-space-of-this.cpp
index 3a1e53ced0e4e6d5677074243e48f637c8336b95..cfa3da09596fed31443c1c2f3adb9a3b476077d4 100644 (file)
--- a/test/CodeGenCXX/address-space-of-this.cpp
+++ b/test/CodeGenCXX/address-space-of-this.cpp
@@ -1,8 +1,11 @@
 // RUN: %clang_cc1 %s -std=c++14 -triple=spir -emit-llvm -o - | FileCheck %s
 // RUN: %clang_cc1 %s -std=c++17 -triple=spir -emit-llvm -o - | FileCheck %s
+// XFAIL: *
 
+// FIXME: We can't compile address space method qualifiers yet.
+// Therefore there is no way to check correctness of this code.
 struct MyType {
-  MyType(int i) : i(i) {}
+  MyType(int i) __attribute__((address_space(10))) : i(i) {}
   int i;
 };
 //CHECK: call void @_ZN6MyTypeC1Ei(%struct.MyType* addrspacecast (%struct.MyType addrspace(10)* @m to %struct.MyType*), i32 123)

diff --git a/test/CodeGenOpenCLCXX/addrspace-ctor.cl b/test/CodeGenOpenCLCXX/addrspace-ctor.cl
new file mode 100644 (file)
index 0000000..42c2e6e
--- /dev/null
+++ b/test/CodeGenOpenCLCXX/addrspace-ctor.cl
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 %s -triple spir-unknown-unknown -cl-std=c++ -emit-llvm -O0 -o - | FileCheck %s
+
+struct MyType {
+  MyType(int i) : i(i) {}
+  MyType(int i) __constant : i(i) {}
+  int i;
+};
+
+//CHECK: call void @_ZNU3AS26MyTypeC1Ei(%struct.MyType addrspace(2)* @const1, i32 1)
+__constant MyType const1 = 1;
+//CHECK: call void @_ZNU3AS26MyTypeC1Ei(%struct.MyType addrspace(2)* @const2, i32 2)
+__constant MyType const2(2);
+//CHECK: call void @_ZNU3AS46MyTypeC1Ei(%struct.MyType addrspace(4)* addrspacecast (%struct.MyType addrspace(1)* @glob to %struct.MyType addrspace(4)*), i32 1)
+MyType glob(1);

diff --git a/test/SemaCXX/address-space-ctor.cpp b/test/SemaCXX/address-space-ctor.cpp
new file mode 100644 (file)
index 0000000..b872b5a
--- /dev/null
+++ b/test/SemaCXX/address-space-ctor.cpp
@@ -0,0 +1,18 @@
+// RUN: %clang_cc1 %s -std=c++14 -triple=spir -verify -fsyntax-only
+// RUN: %clang_cc1 %s -std=c++17 -triple=spir -verify -fsyntax-only
+
+struct MyType {
+  MyType(int i) : i(i) {}
+  int i;
+};
+
+//expected-note@-5{{candidate constructor (the implicit copy constructor) not viable: no known conversion from 'int' to 'const MyType &' for 1st argument}}
+//expected-note@-6{{candidate constructor (the implicit move constructor) not viable: no known conversion from 'int' to 'MyType &&' for 1st argument}}
+//expected-note@-6{{candidate constructor ignored: cannot be used to construct an object in address space '__attribute__((address_space(10)))'}}
+//expected-note@-8{{candidate constructor ignored: cannot be used to construct an object in address space '__attribute__((address_space(10)))'}}
+//expected-note@-9{{candidate constructor ignored: cannot be used to construct an object in address space '__attribute__((address_space(10)))'}}
+//expected-note@-9{{candidate constructor ignored: cannot be used to construct an object in address space '__attribute__((address_space(10)))'}}
+
+// FIXME: We can't implicitly convert between address spaces yet.
+MyType __attribute__((address_space(10))) m1 = 123; //expected-error{{no viable conversion from 'int' to '__attribute__((address_space(10))) MyType'}}
+MyType __attribute__((address_space(10))) m2(123);  //expected-error{{no matching constructor for initialization of '__attribute__((address_space(10))) MyType'}}