On Thu, May 16, 2013 at 7:12 AM, Aaron Ballman <span dir="ltr"><<a href="mailto:aaron@aaronballman.com" target="_blank">aaron@aaronballman.com</a>></span> wrote:<br><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="im">On Thu, May 16, 2013 at 12:46 AM, Richard Smith <<a href="mailto:richard@metafoo.co.uk">richard@metafoo.co.uk</a>> 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>
</div>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>
<div class="im"><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 previous<br>
> existing one!)<br>
<br>
</div>Can do!<br>
<div class="im"><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 || ... 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>
</div>This is illegal in VS (type qualifier must be after *)<br>
<div class="im"><br>
> ... or ...<br>
><br>
> int *(__ptr32 __sptr p); // ok?<br>
<br>
</div>As is this (same error)<br>
<div class="im"><br>
> +  // You cannot have both __sptr and __uptr on the same declaration, 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>
</div>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).</blockquote><div><br></div><div>OK, in case it wasn't clear, It looks to me like your patch will accept all three of these cases.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="im">
> You could address both of these by looping over AttributedTypes until you<br>
> hit a PointerType, or by looking at the form of the pointer by inspecting<br>
> the TypeProcessingState.<br>
><br>
><br>
> Please add tests for these appearing at the start of the declaration, and<br>
> after the identifier.<br>
<br>
</div>Will do.  Thanks!<br>
<span class="HOEnZb"><font color="#888888"><br>
~Aaron<br>
</font></span></blockquote></div><br>