[PATCH] D31006: [Objective-C] Fix "weak-unavailable" warning with -fobjc-weak

2017-03-27 Thread Brian T. Kelley via Phabricator via cfe-commits 
bkelley added a comment.

Thank you @rjmccall for the approval. I don't have commit access; would someone 
be willing to commit this path for me please? Thanks!


https://reviews.llvm.org/D31006



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D31006: [Objective-C] Fix "weak-unavailable" warning with -fobjc-weak

2017-03-21 Thread John McCall via Phabricator via cfe-commits 
rjmccall accepted this revision.
rjmccall added a comment.
This revision is now accepted and ready to land.

LGTM.




Comment at: lib/Sema/SemaCast.cpp:125
+  assert(Self.getLangOpts().ObjCAutoRefCount ||
+ Self.getLangOpts().ObjCWeak);
 

bkelley wrote:
> rjmccall wrote:
> > Unlike the other patches, we do clearly need to be checking the language 
> > options in places like this.  Still, it's a shame to repeat the same 
> > condition in a million places.
> > 
> > I think the right thing to do here is to add a helper method to LangOpts:
> > 
> >   /// Returns true if any types in the program might have non-trivial 
> > lifetime qualifiers.
> >   bool allowsNonTrivialObjCLifetimeQualifiers() const {
> > return ObjCAutoRefCount || ObjCWeak;
> >   }
> Thanks for the suggestion. I was hesitant to add a method to LangOpts since 
> it has so few derived state functions, but it certainly makes everything else 
> cleaner.
There's probably two main reasons for that:

  - Often, when there's a feature that cuts across different language 
configurations, there's also a specific language option for it.
  - People are too reticent about adding derived state functions to LangOpts. :)


https://reviews.llvm.org/D31006



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D31006: [Objective-C] Fix "weak-unavailable" warning with -fobjc-weak

2017-03-17 Thread Brian T. Kelley via Phabricator via cfe-commits 
bkelley marked an inline comment as done.
bkelley added inline comments.



Comment at: lib/Sema/SemaCast.cpp:125
+  assert(Self.getLangOpts().ObjCAutoRefCount ||
+ Self.getLangOpts().ObjCWeak);
 

rjmccall wrote:
> Unlike the other patches, we do clearly need to be checking the language 
> options in places like this.  Still, it's a shame to repeat the same 
> condition in a million places.
> 
> I think the right thing to do here is to add a helper method to LangOpts:
> 
>   /// Returns true if any types in the program might have non-trivial 
> lifetime qualifiers.
>   bool allowsNonTrivialObjCLifetimeQualifiers() const {
> return ObjCAutoRefCount || ObjCWeak;
>   }
Thanks for the suggestion. I was hesitant to add a method to LangOpts since it 
has so few derived state functions, but it certainly makes everything else 
cleaner.


https://reviews.llvm.org/D31006



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D31006: [Objective-C] Fix "weak-unavailable" warning with -fobjc-weak

2017-03-17 Thread Brian T. Kelley via Phabricator via cfe-commits 
bkelley updated this revision to Diff 92219.
bkelley marked an inline comment as done.
bkelley added a comment.

Updated with feedback from @rjmccall


https://reviews.llvm.org/D31006

Files:
  include/clang/Basic/LangOptions.h
  include/clang/Sema/Sema.h
  lib/Sema/SemaCast.cpp
  lib/Sema/SemaExpr.cpp
  lib/Sema/SemaExprCXX.cpp
  lib/Sema/SemaExprObjC.cpp
  lib/Sema/SemaPseudoObject.cpp
  test/SemaObjC/arc-unavailable-for-weakref.m
  test/SemaObjCXX/arc-unavailable-for-weakref.mm

Index: test/SemaObjCXX/arc-unavailable-for-weakref.mm
===
--- test/SemaObjCXX/arc-unavailable-for-weakref.mm
+++ test/SemaObjCXX/arc-unavailable-for-weakref.mm
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 -x objective-c++ -triple x86_64-apple-darwin11 -fobjc-runtime-has-weak -fsyntax-only -fobjc-arc -verify %s
+// RUN: %clang_cc1 -x objective-c++ -triple x86_64-apple-darwin11 -fobjc-runtime-has-weak -fsyntax-only -fobjc-weak -verify %s
 // rdar://9693477
 
 __attribute__((objc_arc_weak_reference_unavailable))
Index: test/SemaObjC/arc-unavailable-for-weakref.m
===
--- test/SemaObjC/arc-unavailable-for-weakref.m
+++ test/SemaObjC/arc-unavailable-for-weakref.m
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 -triple x86_64-apple-darwin11 -fobjc-runtime-has-weak -fsyntax-only -fobjc-arc -verify -Wno-objc-root-class %s
+// RUN: %clang_cc1 -triple x86_64-apple-darwin11 -fobjc-runtime-has-weak -fsyntax-only -fobjc-weak -verify -Wno-objc-root-class %s
 // rdar://9693477
 
 __attribute__((objc_arc_weak_reference_unavailable))
Index: lib/Sema/SemaPseudoObject.cpp
===
--- lib/Sema/SemaPseudoObject.cpp
+++ lib/Sema/SemaPseudoObject.cpp
@@ -1127,8 +1127,8 @@
   if (!Getter)
 return;
   QualType T = Getter->parameters()[0]->getType();
-  S.CheckObjCARCConversion(Key->getSourceRange(), 
- T, Key, Sema::CCK_ImplicitConversion);
+  S.CheckObjCConversion(Key->getSourceRange(), T, Key,
+Sema::CCK_ImplicitConversion);
 }
 
 bool ObjCSubscriptOpBuilder::findAtIndexGetter() {
Index: lib/Sema/SemaExprObjC.cpp
===
--- lib/Sema/SemaExprObjC.cpp
+++ lib/Sema/SemaExprObjC.cpp
@@ -4108,11 +4108,10 @@
 }
 
 Sema::ARCConversionResult
-Sema::CheckObjCARCConversion(SourceRange castRange, QualType castType,
- Expr *, CheckedConversionKind CCK,
- bool Diagnose,
- bool DiagnoseCFAudited,
- BinaryOperatorKind Opc) {
+Sema::CheckObjCConversion(SourceRange castRange, QualType castType,
+  Expr *, CheckedConversionKind CCK,
+  bool Diagnose, bool DiagnoseCFAudited,
+  BinaryOperatorKind Opc) {
   QualType castExprType = castExpr->getType();
 
   // For the purposes of the classification, we assume reference types
@@ -4152,7 +4151,12 @@
 }
 return ACR_okay;
   }
-  
+
+  // The life-time qualifier cast check above is all we need for ObjCWeak.
+  // ObjCAutoRefCount has more restrictions on what is legal.
+  if (!getLangOpts().ObjCAutoRefCount)
+return ACR_okay;
+
   if (isAnyCLike(exprACTC) && isAnyCLike(castACTC)) return ACR_okay;
 
   // Allow all of these types to be cast to integer types (but not
Index: lib/Sema/SemaExprCXX.cpp
===
--- lib/Sema/SemaExprCXX.cpp
+++ lib/Sema/SemaExprCXX.cpp
@@ -3744,10 +3744,9 @@
   if (From->getType()->isObjCObjectPointerType() &&
   ToType->isObjCObjectPointerType())
 EmitRelatedResultTypeNote(From);
-}
-else if (getLangOpts().ObjCAutoRefCount &&
- !CheckObjCARCUnavailableWeakConversion(ToType,
-From->getType())) {
+} else if (getLangOpts().allowsNonTrivialObjCLifetimeQualifiers() &&
+   !CheckObjCARCUnavailableWeakConversion(ToType,
+  From->getType())) {
   if (Action == AA_Initializing)
 Diag(From->getLocStart(),
  diag::err_arc_weak_unavailable_assign);
@@ -3770,8 +3769,8 @@
   (void) PrepareCastToObjCObjectPointer(E);
   From = E.get();
 }
-if (getLangOpts().ObjCAutoRefCount)
-  CheckObjCARCConversion(SourceRange(), ToType, From, CCK);
+if (getLangOpts().allowsNonTrivialObjCLifetimeQualifiers())
+  CheckObjCConversion(SourceRange(), ToType, From, CCK);
 From = ImpCastExprToType(From, ToType, Kind, VK_RValue, , CCK)
  .get();
 break;
Index: lib/Sema/SemaExpr.cpp
===
--- lib/Sema/SemaExpr.cpp
+++ lib/Sema/SemaExpr.cpp
@@ -7700,7 +7700,7 @@
   Kind = CK_BitCast;
   Sema::AssignConvertType

[PATCH] D31006: [Objective-C] Fix "weak-unavailable" warning with -fobjc-weak

2017-03-16 Thread John McCall via Phabricator via cfe-commits 
rjmccall added inline comments.



Comment at: lib/Sema/SemaCast.cpp:125
+  assert(Self.getLangOpts().ObjCAutoRefCount ||
+ Self.getLangOpts().ObjCWeak);
 

Unlike the other patches, we do clearly need to be checking the language 
options in places like this.  Still, it's a shame to repeat the same condition 
in a million places.

I think the right thing to do here is to add a helper method to LangOpts:

  /// Returns true if any types in the program might have non-trivial lifetime 
qualifiers.
  bool allowsNonTrivialObjCLifetimeQualifiers() const {
return ObjCAutoRefCount || ObjCWeak;
  }


https://reviews.llvm.org/D31006



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D31006: [Objective-C] Fix "weak-unavailable" warning with -fobjc-weak

2017-03-15 Thread Brian T. Kelley via Phabricator via cfe-commits 
bkelley created this revision.

clang should produce the same errors Objective-C classes that cannot be 
assigned to weak pointers under both -fobjc-arc and -fobjc-weak. Check for 
ObjCWeak along with ObjCAutoRefCount when analyzing pointer conversions. Add an 
-fobjc-weak pass to the existing arc-unavailable-for-weakref test cases to 
verify the behavior is the same.


https://reviews.llvm.org/D31006

Files:
  include/clang/Sema/Sema.h
  lib/Sema/SemaCast.cpp
  lib/Sema/SemaExpr.cpp
  lib/Sema/SemaExprCXX.cpp
  lib/Sema/SemaExprObjC.cpp
  lib/Sema/SemaPseudoObject.cpp
  test/SemaObjC/arc-unavailable-for-weakref.m
  test/SemaObjCXX/arc-unavailable-for-weakref.mm

Index: test/SemaObjCXX/arc-unavailable-for-weakref.mm
===
--- test/SemaObjCXX/arc-unavailable-for-weakref.mm
+++ test/SemaObjCXX/arc-unavailable-for-weakref.mm
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 -x objective-c++ -triple x86_64-apple-darwin11 -fobjc-runtime-has-weak -fsyntax-only -fobjc-arc -verify %s
+// RUN: %clang_cc1 -x objective-c++ -triple x86_64-apple-darwin11 -fobjc-runtime-has-weak -fsyntax-only -fobjc-weak -verify %s
 // rdar://9693477
 
 __attribute__((objc_arc_weak_reference_unavailable))
Index: test/SemaObjC/arc-unavailable-for-weakref.m
===
--- test/SemaObjC/arc-unavailable-for-weakref.m
+++ test/SemaObjC/arc-unavailable-for-weakref.m
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 -triple x86_64-apple-darwin11 -fobjc-runtime-has-weak -fsyntax-only -fobjc-arc -verify -Wno-objc-root-class %s
+// RUN: %clang_cc1 -triple x86_64-apple-darwin11 -fobjc-runtime-has-weak -fsyntax-only -fobjc-weak -verify -Wno-objc-root-class %s
 // rdar://9693477
 
 __attribute__((objc_arc_weak_reference_unavailable))
Index: lib/Sema/SemaPseudoObject.cpp
===
--- lib/Sema/SemaPseudoObject.cpp
+++ lib/Sema/SemaPseudoObject.cpp
@@ -1128,8 +1128,8 @@
   if (!Getter)
 return;
   QualType T = Getter->parameters()[0]->getType();
-  S.CheckObjCARCConversion(Key->getSourceRange(), 
- T, Key, Sema::CCK_ImplicitConversion);
+  S.CheckObjCConversion(Key->getSourceRange(), T, Key,
+Sema::CCK_ImplicitConversion);
 }
 
 bool ObjCSubscriptOpBuilder::findAtIndexGetter() {
Index: lib/Sema/SemaExprObjC.cpp
===
--- lib/Sema/SemaExprObjC.cpp
+++ lib/Sema/SemaExprObjC.cpp
@@ -4108,11 +4108,10 @@
 }
 
 Sema::ARCConversionResult
-Sema::CheckObjCARCConversion(SourceRange castRange, QualType castType,
- Expr *, CheckedConversionKind CCK,
- bool Diagnose,
- bool DiagnoseCFAudited,
- BinaryOperatorKind Opc) {
+Sema::CheckObjCConversion(SourceRange castRange, QualType castType,
+  Expr *, CheckedConversionKind CCK,
+  bool Diagnose, bool DiagnoseCFAudited,
+  BinaryOperatorKind Opc) {
   QualType castExprType = castExpr->getType();
 
   // For the purposes of the classification, we assume reference types
@@ -4152,7 +4151,12 @@
 }
 return ACR_okay;
   }
-  
+
+  // The life-time qualifier cast check above is all we need for ObjCWeak.
+  // ObjCAutoRefCount has more restrictions on what is legal.
+  if (!getLangOpts().ObjCAutoRefCount)
+return ACR_okay;
+
   if (isAnyCLike(exprACTC) && isAnyCLike(castACTC)) return ACR_okay;
 
   // Allow all of these types to be cast to integer types (but not
Index: lib/Sema/SemaExprCXX.cpp
===
--- lib/Sema/SemaExprCXX.cpp
+++ lib/Sema/SemaExprCXX.cpp
@@ -3744,10 +3744,9 @@
   if (From->getType()->isObjCObjectPointerType() &&
   ToType->isObjCObjectPointerType())
 EmitRelatedResultTypeNote(From);
-}
-else if (getLangOpts().ObjCAutoRefCount &&
- !CheckObjCARCUnavailableWeakConversion(ToType,
-From->getType())) {
+} else if ((getLangOpts().ObjCAutoRefCount || getLangOpts().ObjCWeak) &&
+   !CheckObjCARCUnavailableWeakConversion(ToType,
+  From->getType())) {
   if (Action == AA_Initializing)
 Diag(From->getLocStart(),
  diag::err_arc_weak_unavailable_assign);
@@ -3770,8 +3769,8 @@
   (void) PrepareCastToObjCObjectPointer(E);
   From = E.get();
 }
-if (getLangOpts().ObjCAutoRefCount)
-  CheckObjCARCConversion(SourceRange(), ToType, From, CCK);
+if (getLangOpts().ObjCAutoRefCount || getLangOpts().ObjCWeak)
+  CheckObjCConversion(SourceRange(), ToType, From, CCK);
 From = ImpCastExprToType(From, ToType, Kind, VK_RValue, , CCK)
  .get();
 break;
Index: lib/Sema/SemaExpr.cpp