[PATCH] Initial support for __sptr and __uptr

Aaron Ballman aaron at aaronballman.com
Mon May 13 17:59:56 PDT 2013


On Mon, May 13, 2013 at 8:51 PM, Richard Smith <richard at metafoo.co.uk> wrote:
> You have ''__foo'' in some of your diagnostics. These should only use a
> single level of quotes.

The quotes were used for consistency with other diagnostics involving
attributes.  I'll look into it again though.

> Should these really be handled as declaration attributes? They look like
> they would more naturally be type attributes. Can you do this:
>
>   void * __sptr * __uptr p;

I think eventually they (along with __ptr32 and __ptr64) will have to
move over to ExtQuals (or somewhere near there) because they're really
type qualifers more than declaration attributes.  But I was modeling
after existing constructs.  And yes, you can do that, though it's
basically pointless because there's not a __ptrXX involved.  But you
are correct, this really highlights that __ptrXX and __sptr/__uptr
need to move to types instead of declarations.

Do you think ExtQuals would be an appropriate place, or do you have a
better suggestion?

> I don't think there's any need to check MicrosoftExt in SemaDeclAttr: these
> things are only keyword in MicrosoftExt mode.
>
> +    // You cannot have both __sptr and __uptr on the same declaration, nor
> can
> +    // you duplicate the attributes.
> +    bool HasUPtr = D->hasAttr<UPtrAttr>(), HasSPtr =
> D->hasAttr<SPtrAttr>();
> +    if ((HasUPtr && Kind == AttributeList::AT_SPtr) ||
> +        (HasSPtr && Kind == AttributeList::AT_UPtr)) {
> +      S.Diag(Attr.getLoc(), diag::err_attributes_are_not_compatible)
> +        << S.Context.Idents.get("'__sptr'").getName()
> +        << S.Context.Idents.get("'__uptr'").getName();
>
> This seems like a very strange way to convert a string literal into a
> StringRef.

It was, but I think this will have to be reworked considerably and so
likely won't survive to the next patch.

Thanks!

~Aaron



More information about the cfe-commits mailing list