On Wed, Aug 29, 2012 at 1:58 AM, Chandler Carruth <[email protected]> wrote: > On Tue, Aug 28, 2012 at 10:51 PM, Richard Smith <[email protected]> > wrote: >> >> >> On 28 Aug 2012 21:58, "Aaron Ballman" <[email protected]> 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 _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
