arphaman added inline comments.
================
Comment at: lib/Sema/SemaDecl.cpp:10184
+ (!getLangOpts().ObjCAutoRefCount && getLangOpts().ObjCWeak &&
+ VDecl->getType().getObjCLifetime() != Qualifiers::OCL_Weak)) &&
!Diags.isIgnored(diag::warn_arc_repeated_use_of_weak,
----------------
jordan_rose wrote:
> bkelley wrote:
> > jordan_rose wrote:
> > > This condition's getting complicated, and it shows up in a few places.
> > > Would it make sense to factor it out?
> > What do you think about adding a member function like
> > `hasMRCNonTrivialWeakObjCLifetime(const ASTContext &Context)` to QualType
> > to factor out lines 10183-10184? We could use that in D31003, D31004, here,
> > and D31007.
> I'm fine with it myself but I don't work on Clang very much anymore. Maybe
> someone else can say whether it's actually a good idea.
>
> (By the way, the conventional abbreviation for this mode is "MRR" for "Manual
> Retain/Release", even though it's "ARC" and "Automated Reference Counting".)
Do you want to extract the out the entire
```
(!getLangOpts().ObjCAutoRefCount && getLangOpts().ObjCWeak &&
VDecl->getType().getObjCLifetime() != Qualifiers::OCL_Weak)
```
?
It looks like the others patches use only `getLangOpts().ObjCWeak &&
Type.getObjCLifetime() != Qualifiers::OCL_Weak` so the use of
`hasMRCNonTrivialWeakObjCLifetime` won't be equivalent to the original code in
the other patches if you extract all code from lines 10183-10184.
================
Comment at: test/SemaObjC/arc-repeated-weak.mm:468
+// With -fobjc-weak, the cast below is allowed.
+#if __has_feature(objc_arc)
// This used to crash in the constructor of WeakObjectProfileTy when a
----------------
NIT: You can keep the code in `foo1` active and just use the `#if` on the
`expected-error`, like:
```
void foo1() {
INTFPtrTy tmp = (INTFPtrTy)e1;
#if __has_feature(objc_arc)
// expected-error@-2 {{cast of 'E' to 'INTFPtrTy' (aka 'INTF *') is disallowed
with ARC}}
#endif
}
```
https://reviews.llvm.org/D31005
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
