Relax the conversion rules for Objective-C GC qualifiers a

bit by allowing __weak and __strong to be added/dropped as part of
implicit conversions (qualification conversions in C++). A little
history: GCC lets one add/remove/change GC qualifiers just about
anywhere, implicitly. Clang did roughly the same before, but we
recently normalized the semantics of qualifiers across the board to
get a semantics that we could reason about (yay). Unfortunately, this
tightened the screws a bit too much for GC qualifiers, where it's
common to add/remove these qualifiers at will.

Overall, we're still in better shape than we were before: we don't
permit directly changing the GC qualifier (e.g., __weak -> __strong),
so type safety is improved. More importantly, we're internally
consistent in our handling of qualifiers, and the logic that allows
adding/removing GC qualifiers (but not adding/removing address
spaces!) only touches two obvious places.

Fixes <rdar://problem/9402499>.

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

diff --git a/include/clang/AST/Type.h b/include/clang/AST/Type.h
index 814d12b2f01f7d9955ec1fa2cab9341f0c8d9e48..9fc776d051094d173e02ff55900983bbec9b71ff 100644 (file)
--- a/include/clang/AST/Type.h
+++ b/include/clang/AST/Type.h
@@ -294,9 +294,15 @@ public:
   /// Generally this answers the question of whether an object with the other
   /// qualifiers can be safely used as an object with these qualifiers.
   bool compatiblyIncludes(Qualifiers other) const {
-    // Non-CVR qualifiers must match exactly.  CVR qualifiers may subset.
-    return ((Mask & ~CVRMask) == (other.Mask & ~CVRMask)) &&
-           (((Mask & CVRMask) | (other.Mask & CVRMask)) == (Mask & CVRMask));
+    return 
+      // Address spaces must match exactly.
+      getAddressSpace() == other.getAddressSpace() &&
+      // ObjC GC qualifiers can match, be added, or be removed, but can't be
+      // changed.
+      (getObjCGCAttr() == other.getObjCGCAttr() ||
+       !hasObjCGCAttr() || !other.hasObjCGCAttr()) &&
+      // CVR qualifiers may subset.
+      (((Mask & CVRMask) | (other.Mask & CVRMask)) == (Mask & CVRMask));
   }
 
   /// \brief Determine whether this set of qualifiers is a strict superset of

diff --git a/lib/Sema/SemaOverload.cpp b/lib/Sema/SemaOverload.cpp
index 64f04de87aabbfacf730708c82041950ba00a061..602ecd66027db773839617f2d1e9702b9459623b 100644 (file)
--- a/lib/Sema/SemaOverload.cpp
+++ b/lib/Sema/SemaOverload.cpp
@@ -2197,6 +2197,13 @@ Sema::IsQualificationConversion(QualType FromType, QualType ToType,
     Qualifiers FromQuals = FromType.getQualifiers();
     Qualifiers ToQuals = ToType.getQualifiers();
     
+    // Allow addition/removal of GC attributes but not changing GC attributes.
+    if (FromQuals.getObjCGCAttr() != ToQuals.getObjCGCAttr() &&
+        (!FromQuals.hasObjCGCAttr() || !ToQuals.hasObjCGCAttr())) {
+      FromQuals.removeObjCGCAttr();
+      ToQuals.removeObjCGCAttr();
+    }
+    
     //   -- for every j > 0, if const is in cv 1,j then const is in cv
     //      2,j, and similarly for volatile.
     if (!CStyle && !ToQuals.compatiblyIncludes(FromQuals))

diff --git a/test/SemaObjC/gc-attributes.m b/test/SemaObjC/gc-attributes.m
new file mode 100644 (file)
index 0000000..2f54328
--- /dev/null
+++ b/test/SemaObjC/gc-attributes.m
@@ -0,0 +1,22 @@
+// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -fobjc-gc -fsyntax-only -verify %s
+
+@interface A
+@end
+
+void f0(__strong A**); // expected-note{{passing argument to parameter here}}
+
+void test_f0() {
+  A *a;
+  static __weak A *a2;
+  f0(&a);
+  f0(&a2); // expected-warning{{passing 'A *__weak *' to parameter of type 'A *__strong *' discards qualifiers}} 
+}
+
+void f1(__weak A**); // expected-note{{passing argument to parameter here}}
+
+void test_f1() {
+  A *a;
+  __strong A *a2;
+  f1(&a);
+  f1(&a2); // expected-warning{{passing 'A *__strong *' to parameter of type 'A *__weak *' discards qualifiers}} 
+}

diff --git a/test/SemaObjCXX/gc-attributes.mm b/test/SemaObjCXX/gc-attributes.mm
new file mode 100644 (file)
index 0000000..70a93b2
--- /dev/null
+++ b/test/SemaObjCXX/gc-attributes.mm
@@ -0,0 +1,22 @@
+// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -fobjc-gc -fsyntax-only -verify %s
+
+@interface A
+@end
+
+void f0(__strong A**); // expected-note{{candidate function not viable: 1st argument ('A *__weak *') has __weak lifetime, but parameter has __strong lifetime}}
+
+void test_f0() {
+  A *a;
+  static __weak A *a2;
+  f0(&a);
+  f0(&a2); // expected-error{{no matching function}}
+}
+
+void f1(__weak A**); // expected-note{{candidate function not viable: 1st argument ('A *__strong *') has __strong lifetime, but parameter has __weak lifetime}}
+
+void test_f1() {
+  A *a;
+  __strong A *a2;
+  f1(&a);
+  f1(&a2); // expected-error{{no matching function}}
+}

diff --git a/test/SemaObjCXX/overload-gc.mm b/test/SemaObjCXX/overload-gc.mm
index d2ddd40efb4cc3d68b6737bc357270b482c1b2d1..5488ea56315c0fa03a0c18441dae174c34fccea0 100644 (file)
--- a/test/SemaObjCXX/overload-gc.mm
+++ b/test/SemaObjCXX/overload-gc.mm
@@ -1,9 +1,9 @@
 // RUN: %clang_cc1 -fsyntax-only -triple i386-apple-darwin9 -fobjc-gc -verify %s
 
-void f0(__weak id *); // expected-note{{candidate function not viable: 1st argument ('id *') has no lifetime, but parameter has __weak lifetime}}
+void f0(__weak id *);
 
 void test_f0(id *x) {
-  f0(x); // expected-error{{no matching function for call to 'f0'}}
+  f0(x);
 }
 
 @interface A