On Sun, Jun 17, 2012 at 6:47 PM, Richard Smith <[email protected]> wrote:
> On Sun, Jun 17, 2012 at 4:37 PM, Aaron Ballman <[email protected]> wrote:
>> On Sun, Jun 17, 2012 at 6:07 PM, Richard Smith <[email protected]> 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

_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to