[PATCH] D65256: [Sema][ObjC] Mark C union fields that have non-trivial ObjC ownership qualifications as unavailable if the union is declared in a system header

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.
Herald added a subscriber: ributzka.

Okay, now that I understand the source-compatibility issues a little better, I think this approach is probably okay.  If it causes trouble, we can consider special-casing these headers to treat the member as `__unsafe_unretained` implicitly — the special case isn't great, but it's better than the potential unsoundness.



================
Comment at: include/clang/AST/ASTContext.h:2060
+  /// attr::ObjCOwnership implies an ownership qualifier was explicitly
+  /// specified rather than being added implicitly by the compiler.
+  bool isObjCOwnershipAttributedType(QualType Ty) const;
----------------
How about something like: "Return true if the type has been explicitly qualified with ObjC ownership.  A type may be implicitly qualified with ownership under ObjC ARC, and in some cases the compiler treats these differently".

Could you look for other places where we look for an explicit qualifier?  I'm pretty sure I remember that happening once or twice.


================
Comment at: lib/Sema/SemaDecl.cpp:11176
     for (const FieldDecl *FD : RD->fields())
-      asDerived().visit(FD->getType(), FD, InNonTrivialUnion);
+      if (!FD->hasAttr<UnavailableAttr>())
+        asDerived().visit(FD->getType(), FD, InNonTrivialUnion);
----------------
Can we make these exceptions only apply to the attributes we synthesize rather than arbitrary uses of `__attribute__((unavailable))`?  These aren't really good semantics in general.

You can do the check in a well-named function like `isSuppressedNonTrivialMember`, which would be a good place for a comment explaining what's going on here and why this seemed like the most reasonable solution.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D65256/new/

https://reviews.llvm.org/D65256