[PATCH] D31007: [Objective-C] Miscellaneous -fobjc-weak Fixes

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/D31007



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


[PATCH] D31007: [Objective-C] Miscellaneous -fobjc-weak Fixes

2017-03-24 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, thanks!


https://reviews.llvm.org/D31007



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


[PATCH] D31007: [Objective-C] Miscellaneous -fobjc-weak Fixes

2017-03-24 Thread Brian T. Kelley via Phabricator via cfe-commits
bkelley added inline comments.



Comment at: lib/Sema/SemaInit.cpp:6681
   // full-expression's cleanups.
-  if ((S.getLangOpts().ObjCAutoRefCount &&
-   MTE->getType()->isObjCLifetimeType()) ||
+  if (MTE->getType()->isNonTrivialObjCLifetimeType() ||
   (MTE->getStorageDuration() == SD_Automatic &&

rjmccall wrote:
> Oh, I see what's happening here.  The special case for ARC is actually wholly 
> unnecessary.  The code's been written as if MTE->getType() for an ARC 
> temporary will just have type "id".  In fact, it'll have a lifetime-qualified 
> type, dependent on the type of the reference which has been bound to it.  So 
> we need to mark that there's a temporary cleanup if the type has non-trivial 
> lifetime, i.e. if MTE->getType().hasNonTrivialObjCLifetime(); but this is 
> already considered as part of MTE->getType().isDestructedType(), and the 
> storage-duration condition there is correct as well.  So really we should 
> just delete this entire clause.
> 
> That should eliminate the need for Type::isNonTrivialObjCLifetimeType().  In 
> general, we don't want methods like that to ever exist: lifetime is 
> determined by qualifiers, and querying a Type instead of a QualType has the 
> potential to implicitly remove qualifiers.
Thanks for looking in to this and for the explanation. I've removed the check 
and the changes to Type.


https://reviews.llvm.org/D31007



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


[PATCH] D31007: [Objective-C] Miscellaneous -fobjc-weak Fixes

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

Updated with latest feedback from @rjmccall


https://reviews.llvm.org/D31007

Files:
  lib/Sema/SemaCast.cpp
  lib/Sema/SemaDecl.cpp
  lib/Sema/SemaDeclCXX.cpp
  lib/Sema/SemaInit.cpp
  test/SemaObjCXX/objc-weak.mm

Index: test/SemaObjCXX/objc-weak.mm
===
--- /dev/null
+++ test/SemaObjCXX/objc-weak.mm
@@ -0,0 +1,28 @@
+// RUN: %clang_cc1 -fsyntax-only -fobjc-runtime-has-weak -fobjc-weak -fblocks -Wno-objc-root-class -std=c++98 -Wno-c++0x-extensions -verify %s
+
+@interface AnObject
+@property(weak) id value;
+@end
+
+__attribute__((objc_arc_weak_reference_unavailable))
+@interface NOWEAK : AnObject // expected-note 2 {{class is declared here}}
+@end
+
+struct S {
+  __weak id a; // expected-note {{because type 'S' has a member with __weak ownership}}
+};
+
+union U {
+  __weak id a; // expected-error {{ARC forbids Objective-C objects in union}}
+  S b; // expected-error {{union member 'b' has a non-trivial copy constructor}}
+};
+
+void testCast(AnObject *o) {
+  __weak id a = reinterpret_cast<__weak NOWEAK *>(o); // expected-error {{class is incompatible with __weak references}} \
+  // expected-error {{explicit ownership qualifier on cast result has no effect}} \
+  // expected-error {{assignment of a weak-unavailable object to a __weak object}}
+
+  __weak id b = static_cast<__weak NOWEAK *>(o); // expected-error {{class is incompatible with __weak references}} \
+ // expected-error {{explicit ownership qualifier on cast result has no effect}} \
+ // expected-error {{assignment of a weak-unavailable object to a __weak object}}
+}
Index: lib/Sema/SemaInit.cpp
===
--- lib/Sema/SemaInit.cpp
+++ lib/Sema/SemaInit.cpp
@@ -6674,14 +6674,10 @@
   /*IsInitializerList=*/false,
   ExtendingEntity->getDecl());
 
-  // If we're binding to an Objective-C object that has lifetime, we
-  // need cleanups. Likewise if we're extending this temporary to automatic
-  // storage duration -- we need to register its cleanup during the
-  // full-expression's cleanups.
-  if ((S.getLangOpts().ObjCAutoRefCount &&
-   MTE->getType()->isObjCLifetimeType()) ||
-  (MTE->getStorageDuration() == SD_Automatic &&
-   MTE->getType().isDestructedType()))
+  // If we're extending this temporary to automatic storage duration -- we
+  // need to register its cleanup during the full-expression's cleanups.
+  if (MTE->getStorageDuration() == SD_Automatic &&
+  MTE->getType().isDestructedType())
 S.Cleanup.setExprNeedsCleanups(true);
 
   CurInit = MTE;
Index: lib/Sema/SemaDeclCXX.cpp
===
--- lib/Sema/SemaDeclCXX.cpp
+++ lib/Sema/SemaDeclCXX.cpp
@@ -7145,8 +7145,7 @@
 //   [...] nontrivally ownership-qualified types are [...] not trivially
 //   default constructible, copy constructible, move constructible, copy
 //   assignable, move assignable, or destructible [...]
-if (S.getLangOpts().ObjCAutoRefCount &&
-FieldType.hasNonTrivialObjCLifetime()) {
+if (FieldType.hasNonTrivialObjCLifetime()) {
   if (Diagnose)
 S.Diag(FI->getLocation(), diag::note_nontrivial_objc_ownership)
   << RD << FieldType.getObjCLifetime();
Index: lib/Sema/SemaDecl.cpp
===
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -14492,7 +14492,7 @@
   // Verify that all the fields are okay.
   SmallVector RecFields;
 
-  bool ARCErrReported = false;
+  bool ObjCFieldLifetimeErrReported = false;
   for (ArrayRef::iterator i = Fields.begin(), end = Fields.end();
i != end; ++i) {
 FieldDecl *FD = cast(*i);
@@ -14627,16 +14627,16 @@
 << FixItHint::CreateInsertion(FD->getLocation(), "*");
   QualType T = Context.getObjCObjectPointerType(FD->getType());
   FD->setType(T);
-} else if (getLangOpts().ObjCAutoRefCount && Record && !ARCErrReported &&
+} else if (getLangOpts().allowsNonTrivialObjCLifetimeQualifiers() &&
+   Record && !ObjCFieldLifetimeErrReported &&
(!getLangOpts().CPlusPlus || Record->isUnion())) {
-  // It's an error in ARC if a field has lifetime.
+  // It's an error in ARC or Weak if a field has lifetime.
   // We don't want to report this in a system header, though,
   // so we just make the field unavailable.
   // FIXME: that's really not sufficient; we need to make the type
   // itself invalid to, 

[PATCH] D31007: [Objective-C] Miscellaneous -fobjc-weak Fixes

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



Comment at: lib/Sema/SemaInit.cpp:6681
   // full-expression's cleanups.
-  if ((S.getLangOpts().ObjCAutoRefCount &&
-   MTE->getType()->isObjCLifetimeType()) ||
+  if (MTE->getType()->isNonTrivialObjCLifetimeType() ||
   (MTE->getStorageDuration() == SD_Automatic &&

Oh, I see what's happening here.  The special case for ARC is actually wholly 
unnecessary.  The code's been written as if MTE->getType() for an ARC temporary 
will just have type "id".  In fact, it'll have a lifetime-qualified type, 
dependent on the type of the reference which has been bound to it.  So we need 
to mark that there's a temporary cleanup if the type has non-trivial lifetime, 
i.e. if MTE->getType().hasNonTrivialObjCLifetime(); but this is already 
considered as part of MTE->getType().isDestructedType(), and the 
storage-duration condition there is correct as well.  So really we should just 
delete this entire clause.

That should eliminate the need for Type::isNonTrivialObjCLifetimeType().  In 
general, we don't want methods like that to ever exist: lifetime is determined 
by qualifiers, and querying a Type instead of a QualType has the potential to 
implicitly remove qualifiers.


https://reviews.llvm.org/D31007



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


[PATCH] D31007: [Objective-C] Miscellaneous -fobjc-weak Fixes

2017-03-23 Thread Brian T. Kelley via Phabricator via cfe-commits
bkelley added inline comments.



Comment at: lib/AST/Type.cpp:3773
+/// lifetime semantics.
+bool Type::isNonTrivialObjCLifetimeType() const {
+  return CanonicalType.hasNonTrivialObjCLifetime();

rjmccall wrote:
> Is this method not identical in behavior to hasNonTrivialObjCLifetime()?
Yes, but I didn't see how to extract that information, which is on the internal 
QualType. But after examining the usage of this function in SemaInit.cpp:6681, 
I think we need something similar to the isObjCLifetimeType() implementation 
above.


https://reviews.llvm.org/D31007



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


[PATCH] D31007: [Objective-C] Miscellaneous -fobjc-weak Fixes

2017-03-23 Thread Brian T. Kelley via Phabricator via cfe-commits
bkelley updated this revision to Diff 92892.

https://reviews.llvm.org/D31007

Files:
  include/clang/AST/Type.h
  lib/AST/Type.cpp
  lib/Sema/SemaCast.cpp
  lib/Sema/SemaDecl.cpp
  lib/Sema/SemaDeclCXX.cpp
  lib/Sema/SemaInit.cpp
  test/SemaObjCXX/objc-weak.mm

Index: test/SemaObjCXX/objc-weak.mm
===
--- /dev/null
+++ test/SemaObjCXX/objc-weak.mm
@@ -0,0 +1,28 @@
+// RUN: %clang_cc1 -fsyntax-only -fobjc-runtime-has-weak -fobjc-weak -fblocks -Wno-objc-root-class -std=c++98 -Wno-c++0x-extensions -verify %s
+
+@interface AnObject
+@property(weak) id value;
+@end
+
+__attribute__((objc_arc_weak_reference_unavailable))
+@interface NOWEAK : AnObject // expected-note 2 {{class is declared here}}
+@end
+
+struct S {
+  __weak id a; // expected-note {{because type 'S' has a member with __weak ownership}}
+};
+
+union U {
+  __weak id a; // expected-error {{ARC forbids Objective-C objects in union}}
+  S b; // expected-error {{union member 'b' has a non-trivial copy constructor}}
+};
+
+void testCast(AnObject *o) {
+  __weak id a = reinterpret_cast<__weak NOWEAK *>(o); // expected-error {{class is incompatible with __weak references}} \
+  // expected-error {{explicit ownership qualifier on cast result has no effect}} \
+  // expected-error {{assignment of a weak-unavailable object to a __weak object}}
+
+  __weak id b = static_cast<__weak NOWEAK *>(o); // expected-error {{class is incompatible with __weak references}} \
+ // expected-error {{explicit ownership qualifier on cast result has no effect}} \
+ // expected-error {{assignment of a weak-unavailable object to a __weak object}}
+}
Index: lib/Sema/SemaInit.cpp
===
--- lib/Sema/SemaInit.cpp
+++ lib/Sema/SemaInit.cpp
@@ -6678,8 +6678,7 @@
   // need cleanups. Likewise if we're extending this temporary to automatic
   // storage duration -- we need to register its cleanup during the
   // full-expression's cleanups.
-  if ((S.getLangOpts().ObjCAutoRefCount &&
-   MTE->getType()->isObjCLifetimeType()) ||
+  if (MTE->getType()->isNonTrivialObjCLifetimeType() ||
   (MTE->getStorageDuration() == SD_Automatic &&
MTE->getType().isDestructedType()))
 S.Cleanup.setExprNeedsCleanups(true);
Index: lib/Sema/SemaDeclCXX.cpp
===
--- lib/Sema/SemaDeclCXX.cpp
+++ lib/Sema/SemaDeclCXX.cpp
@@ -7145,8 +7145,7 @@
 //   [...] nontrivally ownership-qualified types are [...] not trivially
 //   default constructible, copy constructible, move constructible, copy
 //   assignable, move assignable, or destructible [...]
-if (S.getLangOpts().ObjCAutoRefCount &&
-FieldType.hasNonTrivialObjCLifetime()) {
+if (FieldType.hasNonTrivialObjCLifetime()) {
   if (Diagnose)
 S.Diag(FI->getLocation(), diag::note_nontrivial_objc_ownership)
   << RD << FieldType.getObjCLifetime();
Index: lib/Sema/SemaDecl.cpp
===
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -14492,7 +14492,7 @@
   // Verify that all the fields are okay.
   SmallVector RecFields;
 
-  bool ARCErrReported = false;
+  bool ObjCFieldLifetimeErrReported = false;
   for (ArrayRef::iterator i = Fields.begin(), end = Fields.end();
i != end; ++i) {
 FieldDecl *FD = cast(*i);
@@ -14627,16 +14627,16 @@
 << FixItHint::CreateInsertion(FD->getLocation(), "*");
   QualType T = Context.getObjCObjectPointerType(FD->getType());
   FD->setType(T);
-} else if (getLangOpts().ObjCAutoRefCount && Record && !ARCErrReported &&
+} else if (getLangOpts().allowsNonTrivialObjCLifetimeQualifiers() &&
+   Record && !ObjCFieldLifetimeErrReported &&
(!getLangOpts().CPlusPlus || Record->isUnion())) {
-  // It's an error in ARC if a field has lifetime.
+  // It's an error in ARC or Weak if a field has lifetime.
   // We don't want to report this in a system header, though,
   // so we just make the field unavailable.
   // FIXME: that's really not sufficient; we need to make the type
   // itself invalid to, say, initialize or copy.
   QualType T = FD->getType();
-  Qualifiers::ObjCLifetime lifetime = T.getObjCLifetime();
-  if (lifetime && lifetime != Qualifiers::OCL_ExplicitNone) {
+  if (T.hasNonTrivialObjCLifetime()) {
 SourceLocation loc = FD->getLocation();
 if (getSourceManager().isInSystemHeader(loc)) {
   if (!FD->hasAttr()) {
@@ -14647,7 +14647,7 @@
   Diag(FD->getLocation(), diag::err_arc_objc_object_in_tag)
 << 

[PATCH] D31007: [Objective-C] Miscellaneous -fobjc-weak Fixes

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



Comment at: lib/AST/Type.cpp:3773
+/// lifetime semantics.
+bool Type::isNonTrivialObjCLifetimeType() const {
+  return CanonicalType.hasNonTrivialObjCLifetime();

Is this method not identical in behavior to hasNonTrivialObjCLifetime()?


https://reviews.llvm.org/D31007



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


[PATCH] D31007: [Objective-C] Miscellaneous -fobjc-weak Fixes

2017-03-17 Thread Brian T. Kelley via Phabricator via cfe-commits
bkelley updated this revision to Diff 92220.
bkelley added a comment.

Integrated feedback from @rjmccall


https://reviews.llvm.org/D31007

Files:
  include/clang/AST/Type.h
  lib/AST/Type.cpp
  lib/Sema/SemaCast.cpp
  lib/Sema/SemaDecl.cpp
  lib/Sema/SemaDeclCXX.cpp
  lib/Sema/SemaInit.cpp
  test/SemaObjCXX/objc-weak.mm

Index: test/SemaObjCXX/objc-weak.mm
===
--- /dev/null
+++ test/SemaObjCXX/objc-weak.mm
@@ -0,0 +1,28 @@
+// RUN: %clang_cc1 -fsyntax-only -fobjc-runtime-has-weak -fobjc-weak -fblocks -Wno-objc-root-class -std=c++98 -Wno-c++0x-extensions -verify %s
+
+@interface AnObject
+@property(weak) id value;
+@end
+
+__attribute__((objc_arc_weak_reference_unavailable))
+@interface NOWEAK : AnObject // expected-note 2 {{class is declared here}}
+@end
+
+struct S {
+  __weak id a; // expected-note {{because type 'S' has a member with __weak ownership}}
+};
+
+union U {
+  __weak id a; // expected-error {{ARC forbids Objective-C objects in union}}
+  S b; // expected-error {{union member 'b' has a non-trivial copy constructor}}
+};
+
+void testCast(AnObject *o) {
+  __weak id a = reinterpret_cast<__weak NOWEAK *>(o); // expected-error {{class is incompatible with __weak references}} \
+  // expected-error {{explicit ownership qualifier on cast result has no effect}} \
+  // expected-error {{assignment of a weak-unavailable object to a __weak object}}
+
+  __weak id b = static_cast<__weak NOWEAK *>(o); // expected-error {{class is incompatible with __weak references}} \
+ // expected-error {{explicit ownership qualifier on cast result has no effect}} \
+ // expected-error {{assignment of a weak-unavailable object to a __weak object}}
+}
Index: lib/Sema/SemaInit.cpp
===
--- lib/Sema/SemaInit.cpp
+++ lib/Sema/SemaInit.cpp
@@ -6678,8 +6678,7 @@
   // need cleanups. Likewise if we're extending this temporary to automatic
   // storage duration -- we need to register its cleanup during the
   // full-expression's cleanups.
-  if ((S.getLangOpts().ObjCAutoRefCount &&
-   MTE->getType()->isObjCLifetimeType()) ||
+  if (MTE->getType()->isNonTrivialObjCLifetimeType() ||
   (MTE->getStorageDuration() == SD_Automatic &&
MTE->getType().isDestructedType()))
 S.Cleanup.setExprNeedsCleanups(true);
Index: lib/Sema/SemaDeclCXX.cpp
===
--- lib/Sema/SemaDeclCXX.cpp
+++ lib/Sema/SemaDeclCXX.cpp
@@ -7145,8 +7145,7 @@
 //   [...] nontrivally ownership-qualified types are [...] not trivially
 //   default constructible, copy constructible, move constructible, copy
 //   assignable, move assignable, or destructible [...]
-if (S.getLangOpts().ObjCAutoRefCount &&
-FieldType.hasNonTrivialObjCLifetime()) {
+if (FieldType.hasNonTrivialObjCLifetime()) {
   if (Diagnose)
 S.Diag(FI->getLocation(), diag::note_nontrivial_objc_ownership)
   << RD << FieldType.getObjCLifetime();
Index: lib/Sema/SemaDecl.cpp
===
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -14492,7 +14492,7 @@
   // Verify that all the fields are okay.
   SmallVector RecFields;
 
-  bool ARCErrReported = false;
+  bool ObjCFieldLifetimeErrReported = false;
   for (ArrayRef::iterator i = Fields.begin(), end = Fields.end();
i != end; ++i) {
 FieldDecl *FD = cast(*i);
@@ -14627,16 +14627,16 @@
 << FixItHint::CreateInsertion(FD->getLocation(), "*");
   QualType T = Context.getObjCObjectPointerType(FD->getType());
   FD->setType(T);
-} else if (getLangOpts().ObjCAutoRefCount && Record && !ARCErrReported &&
+} else if (getLangOpts().allowsNonTrivialObjCLifetimeQualifiers() &&
+   Record && !ObjCFieldLifetimeErrReported &&
(!getLangOpts().CPlusPlus || Record->isUnion())) {
-  // It's an error in ARC if a field has lifetime.
+  // It's an error in ARC or Weak if a field has lifetime.
   // We don't want to report this in a system header, though,
   // so we just make the field unavailable.
   // FIXME: that's really not sufficient; we need to make the type
   // itself invalid to, say, initialize or copy.
   QualType T = FD->getType();
-  Qualifiers::ObjCLifetime lifetime = T.getObjCLifetime();
-  if (lifetime && lifetime != Qualifiers::OCL_ExplicitNone) {
+  if (T.hasNonTrivialObjCLifetime()) {
 SourceLocation loc = FD->getLocation();
 if (getSourceManager().isInSystemHeader(loc)) {
   if (!FD->hasAttr()) {
@@ -14647,7 +14647,7 @@
   Diag(FD->getLocation(), 

[PATCH] D31007: [Objective-C] Miscellaneous -fobjc-weak Fixes

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

Thanks for all the feedback! I think things are looking a lot better now.


https://reviews.llvm.org/D31007



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


[PATCH] D31007: [Objective-C] Miscellaneous -fobjc-weak Fixes

2017-03-16 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment.

Same thing about parts of this patch — please try to just check 
T.hasNonTrivialObjCLifetime() when reasonable.

Thank you for all this, by the way. :)


https://reviews.llvm.org/D31007



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


[PATCH] D31007: [Objective-C] Miscellaneous -fobjc-weak Fixes

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

After examining the remaining uses of LangOptions.ObjCAutoRefCount, found a 
some additional places to also check for ObjCWeak not covered by previous test 
cases. Added a test file to verify all the code paths that were changed.


https://reviews.llvm.org/D31007

Files:
  lib/Sema/SemaCast.cpp
  lib/Sema/SemaDecl.cpp
  lib/Sema/SemaDeclCXX.cpp
  lib/Sema/SemaInit.cpp
  test/SemaObjCXX/objc-weak.mm

Index: test/SemaObjCXX/objc-weak.mm
===
--- /dev/null
+++ test/SemaObjCXX/objc-weak.mm
@@ -0,0 +1,28 @@
+// RUN: %clang_cc1 -fsyntax-only -fobjc-runtime-has-weak -fobjc-weak -fblocks -Wno-objc-root-class -std=c++98 -Wno-c++0x-extensions -verify %s
+
+@interface AnObject
+@property(weak) id value;
+@end
+
+__attribute__((objc_arc_weak_reference_unavailable))
+@interface NOWEAK : AnObject // expected-note 2 {{class is declared here}}
+@end
+
+struct S {
+  __weak id a; // expected-note {{because type 'S' has a member with __weak ownership}}
+};
+
+union U {
+  __weak id a; // expected-error {{ARC forbids Objective-C objects in union}}
+  S b; // expected-error {{union member 'b' has a non-trivial copy constructor}}
+};
+
+void testCast(AnObject *o) {
+  __weak id a = reinterpret_cast<__weak NOWEAK *>(o); // expected-error {{class is incompatible with __weak references}} \
+  // expected-error {{explicit ownership qualifier on cast result has no effect}} \
+  // expected-error {{assignment of a weak-unavailable object to a __weak object}}
+
+  __weak id b = static_cast<__weak NOWEAK *>(o); // expected-error {{class is incompatible with __weak references}} \
+ // expected-error {{explicit ownership qualifier on cast result has no effect}} \
+ // expected-error {{assignment of a weak-unavailable object to a __weak object}}
+}
Index: lib/Sema/SemaInit.cpp
===
--- lib/Sema/SemaInit.cpp
+++ lib/Sema/SemaInit.cpp
@@ -6680,6 +6680,8 @@
   // full-expression's cleanups.
   if ((S.getLangOpts().ObjCAutoRefCount &&
MTE->getType()->isObjCLifetimeType()) ||
+  (S.getLangOpts().ObjCWeak &&
+   MTE->getType().getObjCLifetime() == Qualifiers::OCL_Weak) ||
   (MTE->getStorageDuration() == SD_Automatic &&
MTE->getType().isDestructedType()))
 S.Cleanup.setExprNeedsCleanups(true);
Index: lib/Sema/SemaDeclCXX.cpp
===
--- lib/Sema/SemaDeclCXX.cpp
+++ lib/Sema/SemaDeclCXX.cpp
@@ -7150,8 +7150,10 @@
 //   [...] nontrivally ownership-qualified types are [...] not trivially
 //   default constructible, copy constructible, move constructible, copy
 //   assignable, move assignable, or destructible [...]
-if (S.getLangOpts().ObjCAutoRefCount &&
-FieldType.hasNonTrivialObjCLifetime()) {
+if ((S.getLangOpts().ObjCAutoRefCount &&
+ FieldType.hasNonTrivialObjCLifetime()) ||
+(S.getLangOpts().ObjCWeak &&
+ FieldType.getObjCLifetime() == Qualifiers::OCL_Weak)) {
   if (Diagnose)
 S.Diag(FI->getLocation(), diag::note_nontrivial_objc_ownership)
   << RD << FieldType.getObjCLifetime();
Index: lib/Sema/SemaDecl.cpp
===
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -14493,7 +14493,7 @@
   // Verify that all the fields are okay.
   SmallVector RecFields;
 
-  bool ARCErrReported = false;
+  bool ObjCFieldLifetimeErrReported = false;
   for (ArrayRef::iterator i = Fields.begin(), end = Fields.end();
i != end; ++i) {
 FieldDecl *FD = cast(*i);
@@ -14628,16 +14628,20 @@
 << FixItHint::CreateInsertion(FD->getLocation(), "*");
   QualType T = Context.getObjCObjectPointerType(FD->getType());
   FD->setType(T);
-} else if (getLangOpts().ObjCAutoRefCount && Record && !ARCErrReported &&
+} else if ((getLangOpts().ObjCAutoRefCount || getLangOpts().ObjCWeak) &&
+   Record && !ObjCFieldLifetimeErrReported &&
(!getLangOpts().CPlusPlus || Record->isUnion())) {
-  // It's an error in ARC if a field has lifetime.
+  // It's an error in ARC or Weak if a field has lifetime.
   // We don't want to report this in a system header, though,
   // so we just make the field unavailable.
   // FIXME: that's really not sufficient; we need to make the type
   // itself invalid to, say, initialize or copy.
   QualType T = FD->getType();
   Qualifiers::ObjCLifetime lifetime = T.getObjCLifetime();
-  if (lifetime && lifetime != Qualifiers::OCL_ExplicitNone) {
+  if (lifetime &&
+  ((getLangOpts().ObjCAutoRefCount &&
+