rjmccall added inline comments.
================
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;
----------------
ahatanak wrote:
> rjmccall wrote:
> > 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.
> I found that there is a function named `hasDirectOwnershipQualifier` in
> SemaType.cpp which also detects explicit ownership qualifiers, so I'm using
> that instead of `isObjCOwnershipAttributedType`. The difference between
> `isObjCOwnershipAttributedType` is that it detects paren types like
> `I*(__strong x)` and doesn't look through typedefs (see the test case in
> non-trivial-c-union.h).
Okay. I don't know what the right rule here is, but I agree we should be using
the same rule in both places.
================
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);
----------------
ahatanak wrote:
> rjmccall wrote:
> > 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.
> We are ignoring unavailable fields here since they don't make the containing
> union non-trivial and we don't want the user to think that the unavailable
> field has to be a trivial field in order to fix a compile error. For example,
> without the check for `UnavailableAttr`, clang will diagnose the unavailable
> field `f0` when the following code is compiled:
>
> ```
> union U3 {
> id f0 __attribute__((unavailable)); // expected-note {{f0 has type
> '__strong id' that is non-trivial to default-initialize}}
> __weak id f1; // expected-note {{f1 has type '__weak id' that is
> non-trivial to default-initialize}}
> };
>
> void test(void) {
> union U3 b; // expected-error {{cannot default-initialize an object of type
> 'union U3' since it is a union that is non-trivial to default-initialize}}
> }
> ```
>
> In that case, does it matter whether the field is explicitly annotated
> unavailable in the source code or implicitly by the compiler?
Oh, I didn't realize we were already honoring the ordinary attribute in the
ABI, but from the code it looks we do. In that case, yeah, it makes sense to
ignore it here. Should we extract a function to ask whether a `FieldDecl`
should be ignored for triviality computations so that it's more
self-documenting that there's a consistent logic here?
Repository:
rC Clang
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D65256/new/
https://reviews.llvm.org/D65256
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
