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).

-- 
Richard

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

Reply via email to