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>.

llvm-svn: 131065

5 files changed, 62 insertions, 5 deletions

diff --git a/clang/include/clang/AST/Type.h b/clang/include/clang/AST/Type.h
index 814d12b2f01..9fc776d0510 100644
--- a/clang/include/clang/AST/Type.h
+++ b/clang/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/clang/lib/Sema/SemaOverload.cpp b/clang/lib/Sema/SemaOverload.cpp
index 64f04de87aa..602ecd66027 100644
--- a/clang/lib/Sema/SemaOverload.cpp
+++ b/clang/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/clang/test/SemaObjC/gc-attributes.m b/clang/test/SemaObjC/gc-attributes.m
new file mode 100644
index 00000000000..2f5432842e7
--- /dev/null
+++ b/clang/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/clang/test/SemaObjCXX/gc-attributes.mm b/clang/test/SemaObjCXX/gc-attributes.mm
new file mode 100644
index 00000000000..70a93b2e33d
--- /dev/null
+++ b/clang/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/clang/test/SemaObjCXX/overload-gc.mm b/clang/test/SemaObjCXX/overload-gc.mm
index d2ddd40efb4..5488ea56315 100644
--- a/clang/test/SemaObjCXX/overload-gc.mm
+++ b/clang/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