================
Comment at: lib/Sema/SemaDeclAttr.cpp:1130
@@ -1129,2 +1129,3 @@
+bool Sema::isValidPointerAttrType(QualType T) {
T = T.getNonReferenceType();
----------------
hfinkel wrote:
> rsmith wrote:
> > hfinkel wrote:
> > > rsmith wrote:
> > > > Is this really appropriate for your attribute? (It makes almost no
> > > > sense for `__attribute__((nonnull))` either, but I couldn't find anyone
> > > > at Apple to tell me what the relevant radar says, so I have no idea why
> > > > we have this "feature".)
> > > Nice :-) -- Honestly, I don't have a strong opinion regarding whether or
> > > not we strip references, but I like the idea of consistency between the
> > > two attributes. The set of returned pointers about which I can assert
> > > something about the alignment should be the same as that about which I
> > > can assert a nonnull property.
> > >
> > > That having been said, it looks like the CodeGen for returns_nonnull that
> > > adds the nonnull IR attribute (and the associated UBSan code) does not
> > > handle references-to-pointers correctly. I'll fix that as a follow-up too.
> > It seems really weird for either of these to look through references to the
> > referenced pointer. And it's ambiguous: does the attribute apply to the
> > reference or to the pointer that the reference refers to? (For the nonnull
> > attribute, GCC and Clang make different choices here, at least in the cases
> > where you can persuade GCC not to reject the attribute!)
> Indeed, unlike nonnull, this does become ambiguous. Perhaps it is best to
> prohibit it?
>
> How do Clang and GCC make different choices for nonnull; is that a bug?
> Because a reference cannot be null, it would seem that it should apply to the
> pointer being bound.
Here's what I can piece together:
* GCC does not intend to support `nonnull` on reference parameters. It rejects
`nonnull(N)` where parameter `N` is a reference, *but* if you apply `nonnull`
with no `N`, it applies to both pointer and reference parameters, and happens
to have the semantics of checking for "null references".
* Clang made up its own thing in response to some non-publicly-visible radar
issue, where it treats the `nonnull` attribute on references-to-pointers as
applying to the pointer within the reference. This makes little sense to me,
but there it is. We naturally can't optimize on the basis of `nonnull` on a
reference-to-pointer like we can on a real pointer parameter/return value (we
have no IR representation for that).
It seems our options are to either not support `assume_aligned` on references
(which would be a shame, because it's natural and useful there just as it is
for pointers), or make `nonnull` and `assume_aligned` inconsistent on
pointers-to-references, or remove our (dubious, IMO) extension of the GNU
`nonnull` semantics. I'm not happy with the third option since I don't know
what the motivation for the extension is nor how many people might be relying
on it, but either of the first two seem fine to me. I don't think the
connection between `nonnull` and `assume_aligned` is strong enough that the
difference would be jarring for people.
http://reviews.llvm.org/D4601
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
