[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

[Chandler Carruth]

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...
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120828/b69c879c/attachment.html>