[PATCH] Initial support for __sptr and __uptr

Richard Smith richard at metafoo.co.uk
Thu May 16 12:54:19 PDT 2013


On Thu, May 16, 2013 at 7:12 AM, Aaron Ballman <aaron at aaronballman.com>wrote:

> On Thu, May 16, 2013 at 12:46 AM, Richard Smith <richard at metafoo.co.uk>
> wrote:
> > +++ lib/Sema/SemaDeclAttr.cpp (working copy)
> > @@ -4225,7 +4225,7 @@
> >      QualType BufferTy = getFunctionOrMethodArgType(D, ArgumentIdx);
> >      if (!BufferTy->isPointerType()) {
> >        S.Diag(Attr.getLoc(), diag::err_attribute_pointers_only)
> > -        << AttrName;
> > +        << Attr.getName();
> >
> > Did you mean to include this change?
>
> I did -- I changed the error in the td file so that it didn't include
> the quotes; by using the attribute identifier itself, the quotes are
> automatically inserted.  This keeps the error consistent between
> usages, but has no functional difference in terms of the output.
>
> >
> > +++ lib/Sema/SemaType.cpp (working copy)
> > @@ -110,6 +110,13 @@
> >      case AttributeList::AT_PnaclCall: \
> >      case AttributeList::AT_IntelOclBicc \
> >
> > +// Microsoft-specific type qualifiers.
> > +#define MS_TYPE_ATTRS_CASELIST  \
> > +    case AttributeList::AT_Ptr32: \
> > +    case AttributeList::AT_Ptr64: \
> > +    case AttributeList::AT_SPtr: \
> > +    case AttributeList::AT_UPtr \
> > +
> >
> > Please remove the trailing \ from the end of this macro (and the previous
> > existing one!)
>
> Can do!
>
> >
> > +  // Pointer type qualifiers can only operate on pointer types, but not
> > +  // pointer-to-member types.
> > +  if (!Type->isPointerType() || Type->isMemberPointerType()) {
> >
> > Member pointer types are not pointer types, so you can drop the || ...
> part
> > here.
> >
> > Also, what if there is type sugar before the pointer type? For instance:
> >
> > typedef int *T;
> > T __ptr32 __sptr P; // ok?
>
> This is illegal in VS (type qualifier must be after *)
>
> > ... or ...
> >
> > int *(__ptr32 __sptr p); // ok?
>
> As is this (same error)
>
> > +  // You cannot have both __sptr and __uptr on the same declaration, nor
> > can
> > +  // you duplicate the attributes.
> > +  const AttributedType *AT = dyn_cast<AttributedType>(Type);
> >
> > What if there is another AttributedType in between? For instance:
> >
> >   int * __sptr __ptr32 __sptr p; // presumably should be rejected,
> > presumably won't be
>
> This should be rejected according to the documentation (though it is
> accepted by CL, but not Intellisense; I've already reported this via
> Connect).


OK, in case it wasn't clear, It looks to me like your patch will accept all
three of these cases.


> > You could address both of these by looping over AttributedTypes until you
> > hit a PointerType, or by looking at the form of the pointer by inspecting
> > the TypeProcessingState.
> >
> >
> > Please add tests for these appearing at the start of the declaration, and
> > after the identifier.
>
> Will do.  Thanks!
>
> ~Aaron
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130516/831452be/attachment.html>


More information about the cfe-commits mailing list