[cfe-commits] r162793 - in /cfe/trunk: include/clang/Basic/DiagnosticGroups.td include/clang/Basic/DiagnosticParseKinds.td lib/Sema/DeclSpec.cpp test/Misc/warning-flags.c test/Parser/cxx-decl.cpp test/Parser/cxx0x-decl.cpp

[Aaron Ballman]

On Wed, Aug 29, 2012 at 1:58 AM, Chandler Carruth <chandlerc at google.com> wrote:
> On Tue, Aug 28, 2012 at 10:51 PM, Richard Smith <richard at metafoo.co.uk>
> wrote:
>>
>>
>> On 28 Aug 2012 21:58, "Aaron Ballman" <aaron at aaronballman.com> wrote:
>> >
>> > Author: aaronballman
>> > Date: Tue Aug 28 15:55:40 2012
>> > New Revision: 162793
>> >
>> > URL: http://llvm.org/viewvc/llvm-project?rev=162793&view=rev
>> > Log:
>> > Splitting the duplicated decl spec extension warning into two: one is an
>> > ExtWarn and the other a vanilla warning.  This addresses PR13705, where
>> > const char const * wouldn't warn unless -pedantic was specified under the
>> > right conditions.
>> >
>> > Modified:
>> >     cfe/trunk/include/clang/Basic/DiagnosticGroups.td
>> >     cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td
>> >     cfe/trunk/lib/Sema/DeclSpec.cpp
>> >     cfe/trunk/test/Misc/warning-flags.c
>> >     cfe/trunk/test/Parser/cxx-decl.cpp
>> >     cfe/trunk/test/Parser/cxx0x-decl.cpp
>> >
>> > Modified: cfe/trunk/include/clang/Basic/DiagnosticGroups.td
>> > URL:
>> > http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticGroups.td?rev=162793&r1=162792&r2=162793&view=diff
>> >
>> > ==============================================================================
>> > --- cfe/trunk/include/clang/Basic/DiagnosticGroups.td (original)
>> > +++ cfe/trunk/include/clang/Basic/DiagnosticGroups.td Tue Aug 28
>> > 15:55:40 2012
>> > @@ -197,6 +197,7 @@
>> >  def StrncatSize : DiagGroup<"strncat-size">;
>> >  def TautologicalCompare : DiagGroup<"tautological-compare">;
>> >  def HeaderHygiene : DiagGroup<"header-hygiene">;
>> > +def DuplicateDeclSpecifiers : DiagGroup<"duplicate-decl-specifiers">;
>>
>> It seems inconsistent to use a plural here and a singular in the warning
>> text.
>>
>> >
>> >  // Preprocessor warnings.
>> >  def : DiagGroup<"builtin-macro-redefined">;
>> >
>> > Modified: cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td
>> > URL:
>> > http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td?rev=162793&r1=162792&r2=162793&view=diff
>> >
>> > ==============================================================================
>> > --- cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td (original)
>> > +++ cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td Tue Aug 28
>> > 15:55:40 2012
>> > @@ -43,7 +43,10 @@
>> >    "extra ';' after member function definition">,
>> >    InGroup<ExtraSemi>, DefaultIgnore;
>> >
>> > -def ext_duplicate_declspec : Extension<"duplicate '%0' declaration
>> > specifier">;
>> > +def ext_duplicate_declspec : ExtWarn<"duplicate '%0' declaration
>> > specifier">,
>> > +  InGroup<DuplicateDeclSpecifiers>;
>> > +def warn_duplicate_declspec : Warning<"duplicate '%0' declaration
>> > specifier">,
>> > +  InGroup<DuplicateDeclSpecifiers>;
>> >  def ext_plain_complex : ExtWarn<
>> >    "plain '_Complex' requires a type specifier; assuming '_Complex
>> > double'">;
>> >  def ext_integer_complex : Extension<
>> >
>> > Modified: cfe/trunk/lib/Sema/DeclSpec.cpp
>> > URL:
>> > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/DeclSpec.cpp?rev=162793&r1=162792&r2=162793&view=diff
>> >
>> > ==============================================================================
>> > --- cfe/trunk/lib/Sema/DeclSpec.cpp (original)
>> > +++ cfe/trunk/lib/Sema/DeclSpec.cpp Tue Aug 28 15:55:40 2012
>> > @@ -325,10 +325,14 @@
>> >
>> >  template <class T> static bool BadSpecifier(T TNew, T TPrev,
>> >                                              const char *&PrevSpec,
>> > -                                            unsigned &DiagID) {
>> > +                                            unsigned &DiagID,
>> > +                                            bool IsExtension = true) {
>> >    PrevSpec = DeclSpec::getSpecifierName(TPrev);
>> > -  DiagID = (TNew == TPrev ? diag::ext_duplicate_declspec
>> > -            : diag::err_invalid_decl_spec_combination);
>> > +  if (TNew != TPrev)
>> > +    DiagID = diag::err_invalid_decl_spec_combination;
>> > +  else
>> > +    DiagID = IsExtension ? diag::ext_duplicate_declspec :
>> > +                           diag::warn_duplicate_declspec;
>> >    return true;
>> >  }
>> >
>> > @@ -673,9 +677,15 @@
>> >                             unsigned &DiagID, const LangOptions &Lang,
>> >                             bool IsTypeSpec) {
>> >    // Duplicates are permitted in C99, and are permitted in C++11 unless
>> > the
>> > -  // cv-qualifier appears as a type-specifier.
>> > -  if ((TypeQualifiers & T) && !Lang.C99 && (!Lang.CPlusPlus0x ||
>> > IsTypeSpec))
>> > -    return BadSpecifier(T, T, PrevSpec, DiagID);
>> > +  // cv-qualifier appears as a type-specifier.  However, since this is
>> > likely
>> > +  // not what the user intended, we will always warn.  We do not need
>> > to set the
>> > +  // qualifier's location since we already have it.
>> > +  if (TypeQualifiers & T) {
>> > +    bool IsExtension = false;
>> > +    if (Lang.C99 || (Lang.CPlusPlus0x && !IsTypeSpec))
>> > +      IsExtension = true;
>>
>> Isn't this backwards? The warning is not an extension in C99 because the
>> code is well-formed there.
>
>
> Doh, sorry! I totally missed that in review...

I'll correct and commit, good catch!

~Aaron