[PATCH] Initial support for __sptr and __uptr

Aaron Ballman aaron at aaronballman.com
Wed May 22 16:26:20 PDT 2013


Thanks for the help!  Committed in r182535

~Aaron

On Wed, May 22, 2013 at 5:46 PM, Richard Smith <richard at metafoo.co.uk> wrote:
> +    if (!AT->isSugared())
> +      break;
> +
> +    Desugared = AT->desugar();
>
> Please call AttributedType::getEquivalentType() rather than desugar(), and
> don't call AT->isSugared() (it always returns true).
>
>
> +  QualType origType = Type;
> +  Type = S.Context.getAttributedType(TAK, origType, Type);
> +  return false;
>
> Should be "OrigType", but why not just pass Type in directly?
>
> With those two tweaks, LGTM
>
> On Wed, May 22, 2013 at 2:34 PM, Aaron Ballman <aaron at aaronballman.com>
> wrote:
>>
>> Here is the updated patch with your suggestions applied.
>>
>> Thanks!
>>
>> ~Aaron
>>
>> On Wed, May 22, 2013 at 4:45 PM, Aaron Ballman <aaron at aaronballman.com>
>> wrote:
>> > On Wed, May 22, 2013 at 4:39 PM, Richard Smith <richard at metafoo.co.uk>
>> > wrote:
>> >> On Wed, May 22, 2013 at 1:34 PM, Aaron Ballman <aaron at aaronballman.com>
>> >> wrote:
>> >>>
>> >>> On Wed, May 22, 2013 at 3:59 PM, Richard Smith <richard at metafoo.co.uk>
>> >>> wrote:
>> >>> > +  // Pointer type qualifiers can only operate on pointer types, but
>> >>> > not
>> >>> > +  // pointer-to-member types.
>> >>> > +  if (!Type->isPointerType() || Type->isMemberPointerType()) {
>> >>> >
>> >>> > You don't need the isMemberPointerType here.
>> >>>
>> >>> Removed.
>> >>>
>> >>> > This will still accept cases like:
>> >>> >
>> >>> > typedef int *P;
>> >>> > P __ptr32 myp;
>> >>> >
>> >>> > I would suggest checking isa<PointerType> on the type you get after
>> >>> > stripping off AttributedTypes.
>> >>>
>> >>> I had an explicit test case in to ensure that worked because I felt it
>> >>> was a reasonable use case.  Are you saying we should not allow it?
>> >>
>> >>
>> >> You said this was illegal in MSVC, so I don't see why we should allow
>> >> it.
>> >
>> > MSVC isn't particularly consistent with what it allows and disallows.
>> > ;-)  But I'll remove it just the same.
>> >
>> > Thanks!
>> >
>> > ~Aaron
>
>



More information about the cfe-commits mailing list