[cfe-commits] [PATCH] Improved __declspec support

[Michael Spencer]

On Mon, Jun 18, 2012 at 4:43 PM, Aaron Ballman <aaron at aaronballman.com> wrote:
> On Mon, Jun 18, 2012 at 6:17 PM, Richard Smith <richard at metafoo.co.uk> wrote:
>>> +  /// Returns true if the attribute is a pure __declspec or a synthesized
>>> +  /// declspec representing a type specification (like __w64 or __ptr32).
>>> +  bool isDeclspecAttribute() const { return SyntaxUsed == AS_Declspec ||
>>> +                                            SyntaxUsed == AS_MSTypespec; }
>>
>> Given that these attributes aren't spelled as __declspec, I'd prefer
>> that we return false for MSTypespec attributes here (and change the
>> comment on AS_MSTypespec above to match). I think this makes things
>> simpler, and doesn't seem to matter for the rest of the patch (you
>> remove the one existing use of this function, and none of your added
>> calls need this behavior).
>
> This one kind of bugged me too, but the reason I did it was for
> historical purposes.  Those have always been considered __declspec
> previously, so this patch was retaining compatibility with the
> previous behavior.
>
> I think I would prefer to make that change as a separate patch just to
> keep the distinction more clear.  I do agree that it would be better
> for those type specifiers to not be spelled as declspecs, but I have
> no idea the scope of that sort of change.

Note that __forceinline is currently represented as a GNU attribute,
which is obviously wrong. It would be nice for that to be fixed at the
same time, as it's a similar keyword type.

- Michael Spencer

>>> +  } else if (Ident->getName() == "property") {
>> [...]
>>> +    return true;
>>
>> How about recovering from this, with something like:
>>
>>  BalancedDelimiterTracker T(*this, tok::l_paren);
>>  if (!T.consumeOpen())
>>    T.skipToEnd();
>>  return;
>>
>> ... then remove the return value from the function. This would allow
>> us to deal with things like
>>  __declspec(property(...) some_other_attribute)
>
> My next patch (already mostly written) adds true parsing support for
> property, so that's a WIP.  But until that patch lands, this is a good
> idea.
>
> ~Aaron
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits