[PATCH] Initial support for __sptr and __uptr

Richard Smith richard at metafoo.co.uk
Wed May 22 14:46:02 PDT 2013


+    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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130522/846b5a42/attachment.html>


More information about the cfe-commits mailing list