[PATCH] Initial support for __sptr and __uptr

[Aaron Ballman]

Ping

~Aaron

On Fri, May 17, 2013 at 11:22 AM, Aaron Ballman <aaron at aaronballman.com> wrote:
> This revised patch is based on our previous conversations and should
> be all set.  We discussed whether we should move further with semantic
> and code gen support, but decided against it for now (not enough bang
> for the buck), but this patch at least moves us in a more correct
> direction from what was previously supported.
>
> Thanks!
>
> ~Aaron
>
> On Thu, May 16, 2013 at 3:54 PM, Richard Smith <richard at metafoo.co.uk> wrote:
>> 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
>>
>>