[PATCH] Initial support for __sptr and __uptr

[Aaron Ballman]

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
>
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: sptr_uptr.patch
Type: application/octet-stream
Size: 17385 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130517/72c4314f/attachment.obj>