Ping ~Aaron
On Fri, May 17, 2013 at 11:22 AM, Aaron Ballman <[email protected]> wrote: > 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 <[email protected]> wrote: >> On Thu, May 16, 2013 at 7:12 AM, Aaron Ballman <[email protected]> >> wrote: >>> >>> On Thu, May 16, 2013 at 12:46 AM, Richard Smith <[email protected]> >>> 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 >> >> _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
