Page MenuHomePhabricator

[Objective-C] Fix "weak-unavailable" warning with -fobjc-weak
ClosedPublic

Authored by bkelley on Mar 15 2017, 3:39 PM.

Details

Summary

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.

Diff Detail

Event Timeline

rjmccall added inline comments.Mar 16 2017, 9:53 AM
lib/Sema/SemaCast.cpp
125

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;
}
bkelley marked an inline comment as done.Mar 17 2017, 4:03 PM
bkelley added inline comments.
lib/Sema/SemaCast.cpp
125

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.

bkelley updated this revision to Diff 92219.Mar 17 2017, 4:04 PM
bkelley marked an inline comment as done.

Updated with feedback from @rjmccall

rjmccall accepted this revision.Mar 20 2017, 11:05 PM

LGTM.

lib/Sema/SemaCast.cpp
125

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. :)
This revision is now accepted and ready to land.Mar 20 2017, 11:05 PM

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

bkelley closed this revision.Mar 29 2017, 11:21 AM