<div>+ // Pointer type qualifiers can only operate on pointer types, but not</div><div><div>+ // pointer-to-member types.</div></div><div>+ if (!Type->isPointerType() || Type->isMemberPointerType()) {</div><div><br>
</div><div>You don't need the isMemberPointerType here.</div><div><br></div><div>This will still accept cases like:</div><div><br></div><div>typedef int *P;</div><div>P __ptr32 myp;</div><div><br></div><div>I would suggest checking isa<PointerType> on the type you get after stripping off AttributedTypes.</div>
<br><div class="gmail_quote">On Sun, May 19, 2013 at 11:51 AM, Aaron Ballman <span dir="ltr"><<a href="mailto:aaron@aaronballman.com" target="_blank">aaron@aaronballman.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Ping<br>
<span class="HOEnZb"><font color="#888888"><br>
~Aaron<br>
</font></span><div class="HOEnZb"><div class="h5"><br>
On Fri, May 17, 2013 at 11:22 AM, Aaron Ballman <<a href="mailto:aaron@aaronballman.com">aaron@aaronballman.com</a>> wrote:<br>
> This revised patch is based on our previous conversations and should<br>
> be all set. We discussed whether we should move further with semantic<br>
> and code gen support, but decided against it for now (not enough bang<br>
> for the buck), but this patch at least moves us in a more correct<br>
> direction from what was previously supported.<br>
><br>
> Thanks!<br>
><br>
> ~Aaron<br>
><br>
> On Thu, May 16, 2013 at 3:54 PM, Richard Smith <<a href="mailto:richard@metafoo.co.uk">richard@metafoo.co.uk</a>> wrote:<br>
>> On Thu, May 16, 2013 at 7:12 AM, Aaron Ballman <<a href="mailto:aaron@aaronballman.com">aaron@aaronballman.com</a>><br>
>> wrote:<br>
>>><br>
>>> On Thu, May 16, 2013 at 12:46 AM, Richard Smith <<a href="mailto:richard@metafoo.co.uk">richard@metafoo.co.uk</a>><br>
>>> wrote:<br>
>>> > +++ lib/Sema/SemaDeclAttr.cpp (working copy)<br>
>>> > @@ -4225,7 +4225,7 @@<br>
>>> > QualType BufferTy = getFunctionOrMethodArgType(D, ArgumentIdx);<br>
>>> > if (!BufferTy->isPointerType()) {<br>
>>> > S.Diag(Attr.getLoc(), diag::err_attribute_pointers_only)<br>
>>> > - << AttrName;<br>
>>> > + << Attr.getName();<br>
>>> ><br>
>>> > Did you mean to include this change?<br>
>>><br>
>>> I did -- I changed the error in the td file so that it didn't include<br>
>>> the quotes; by using the attribute identifier itself, the quotes are<br>
>>> automatically inserted. This keeps the error consistent between<br>
>>> usages, but has no functional difference in terms of the output.<br>
>>><br>
>>> ><br>
>>> > +++ lib/Sema/SemaType.cpp (working copy)<br>
>>> > @@ -110,6 +110,13 @@<br>
>>> > case AttributeList::AT_PnaclCall: \<br>
>>> > case AttributeList::AT_IntelOclBicc \<br>
>>> ><br>
>>> > +// Microsoft-specific type qualifiers.<br>
>>> > +#define MS_TYPE_ATTRS_CASELIST \<br>
>>> > + case AttributeList::AT_Ptr32: \<br>
>>> > + case AttributeList::AT_Ptr64: \<br>
>>> > + case AttributeList::AT_SPtr: \<br>
>>> > + case AttributeList::AT_UPtr \<br>
>>> > +<br>
>>> ><br>
>>> > Please remove the trailing \ from the end of this macro (and the<br>
>>> > previous<br>
>>> > existing one!)<br>
>>><br>
>>> Can do!<br>
>>><br>
>>> ><br>
>>> > + // Pointer type qualifiers can only operate on pointer types, but not<br>
>>> > + // pointer-to-member types.<br>
>>> > + if (!Type->isPointerType() || Type->isMemberPointerType()) {<br>
>>> ><br>
>>> > Member pointer types are not pointer types, so you can drop the || ...<br>
>>> > part<br>
>>> > here.<br>
>>> ><br>
>>> > Also, what if there is type sugar before the pointer type? For instance:<br>
>>> ><br>
>>> > typedef int *T;<br>
>>> > T __ptr32 __sptr P; // ok?<br>
>>><br>
>>> This is illegal in VS (type qualifier must be after *)<br>
>>><br>
>>> > ... or ...<br>
>>> ><br>
>>> > int *(__ptr32 __sptr p); // ok?<br>
>>><br>
>>> As is this (same error)<br>
>>><br>
>>> > + // You cannot have both __sptr and __uptr on the same declaration,<br>
>>> > nor<br>
>>> > can<br>
>>> > + // you duplicate the attributes.<br>
>>> > + const AttributedType *AT = dyn_cast<AttributedType>(Type);<br>
>>> ><br>
>>> > What if there is another AttributedType in between? For instance:<br>
>>> ><br>
>>> > int * __sptr __ptr32 __sptr p; // presumably should be rejected,<br>
>>> > presumably won't be<br>
>>><br>
>>> This should be rejected according to the documentation (though it is<br>
>>> accepted by CL, but not Intellisense; I've already reported this via<br>
>>> Connect).<br>
>><br>
>><br>
>> OK, in case it wasn't clear, It looks to me like your patch will accept all<br>
>> three of these cases.<br>
>><br>
>>><br>
>>> > You could address both of these by looping over AttributedTypes until<br>
>>> > you<br>
>>> > hit a PointerType, or by looking at the form of the pointer by<br>
>>> > inspecting<br>
>>> > the TypeProcessingState.<br>
>>> ><br>
>>> ><br>
>>> > Please add tests for these appearing at the start of the declaration,<br>
>>> > and<br>
>>> > after the identifier.<br>
>>><br>
>>> Will do. Thanks!<br>
>>><br>
>>> ~Aaron<br>
>><br>
>><br>
</div></div></blockquote></div><br>