[PATCH] Initial support for __sptr and __uptr

[Aaron Ballman]

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).

> 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