[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]

Should be fixed in r162838

~Aaron

On Wed, Aug 29, 2012 at 8:15 AM, Aaron Ballman <aaron at aaronballman.com> wrote:
> 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