[cfe-commits] [PATCH] Improved __declspec support

[Aaron Ballman]

On Sun, Jun 17, 2012 at 6:47 PM, Richard Smith <richard at metafoo.co.uk> wrote:
> On Sun, Jun 17, 2012 at 4:37 PM, Aaron Ballman <aaron at aaronballman.com> wrote:
>> On Sun, Jun 17, 2012 at 6:07 PM, Richard Smith <richard at metafoo.co.uk> wrote:
>>> Mostly just nitpicking:
>>>
>>>> Index: include/clang/Basic/DiagnosticParseKinds.td
>>>> ===================================================================
>>>> --- include/clang/Basic/DiagnosticParseKinds.td       (revision 158272)
>>>> +++ include/clang/Basic/DiagnosticParseKinds.td       (working copy)
>>>> @@ -481,6 +481,9 @@
>>>>    "introducing an attribute">;
>>>>  def err_alignas_pack_exp_unsupported : Error<
>>>>    "pack expansions in alignment specifiers are not supported yet">;
>>>> +def err_unknown_ms_declspec : Error<"unrecognized __declspec attribute %0">;
>>>
>>> This should probably be a Warning, and InGroup<UnknownAttributes>. It
>>> would also seems sensible to mirror the wording of
>>> warn_unknown_attribute_ignored: "unknown __declspec attribute %0
>>> ignored".
>>
>> The reason I made it an error was because it's an error in VS as well.
>>  Since it pertains to __declspec, which is already MS-specific, I
>> figured the behavior should match.  Given that reasoning, do you still
>> think we should prefer a warning over an error?
>
> Yes. Code using new Visual Studio 2013 (or whatever) __declspec
> attributes shouldn't have to wait for a new Clang release in order to
> ignore the attributes (for this reason I also don't think we *need* a
> different diagnostic for "we've heard of this attribute but don't
> support it" versus "we've not heard of this attribute" -- but I don't
> object to that distinction either).

That's a fair point -- I remove the distinction and keep it as a warning.

~Aaron