From 2f7dc46a5868db54f5a96e3a7308cad6970c0e4b Mon Sep 17 00:00:00 2001 From: Richard Trieu Date: Wed, 16 May 2012 19:04:59 +0000 Subject: [PATCH] Move the warnings for extra semi-colons under -Wextra-semi. Also, added a warning for an extra semi-colon after function definitions. Added logic so that a block of semi-colons on a line will only get one warning instead of a warning for each semi-colon. llvm-svn: 156934 --- clang/include/clang/Basic/DiagnosticParseKinds.td | 13 ++++----- clang/include/clang/Parse/Parser.h | 11 ++++++++ clang/lib/Parse/ParseDecl.cpp | 6 ++--- clang/lib/Parse/ParseDeclCXX.cpp | 17 +++++------- clang/lib/Parse/ParseObjc.cpp | 4 +-- clang/lib/Parse/Parser.cpp | 33 +++++++++++++++++++---- clang/test/Misc/warning-flags.c | 5 +--- clang/test/Parser/cxx-class.cpp | 4 +-- clang/test/Parser/cxx-extra-semi.cpp | 25 +++++++++++++++++ 9 files changed, 83 insertions(+), 35 deletions(-) create mode 100644 clang/test/Parser/cxx-extra-semi.cpp diff --git a/clang/include/clang/Basic/DiagnosticParseKinds.td b/clang/include/clang/Basic/DiagnosticParseKinds.td index aef20af..ad8a915 100644 --- a/clang/include/clang/Basic/DiagnosticParseKinds.td +++ b/clang/include/clang/Basic/DiagnosticParseKinds.td @@ -21,15 +21,16 @@ def warn_file_asm_volatile : Warning< let CategoryName = "Parse Issue" in { def ext_empty_source_file : Extension<"ISO C forbids an empty source file">; -def ext_top_level_semi : Extension< - "extra ';' outside of a function">; def warn_cxx98_compat_top_level_semi : Warning< "extra ';' outside of a function is incompatible with C++98">, InGroup, DefaultIgnore; -def ext_extra_struct_semi : Extension< - "extra ';' inside a %0">; -def ext_extra_ivar_semi : Extension< - "extra ';' inside instance variable list">; +def ext_extra_semi : Extension< + "extra ';' %select{" + "outside of a function|" + "inside a %1|" + "inside instance variable list|" + "after function definition}0">, + InGroup>; def ext_duplicate_declspec : Extension<"duplicate '%0' declaration specifier">; def ext_plain_complex : ExtWarn< diff --git a/clang/include/clang/Parse/Parser.h b/clang/include/clang/Parse/Parser.h index 0ea592f..bd5592c 100644 --- a/clang/include/clang/Parse/Parser.h +++ b/clang/include/clang/Parse/Parser.h @@ -654,6 +654,17 @@ private: /// to the semicolon, consumes that extra token. bool ExpectAndConsumeSemi(unsigned DiagID); + /// \brief The kind of extra semi diagnostic to emit. + enum ExtraSemiKind { + OutsideFunction = 0, + InsideStruct = 1, + InstanceVariableList = 2, + AfterDefinition = 3 + }; + + /// \brief Consume any extra semi-colons until the end of the line. + void ConsumeExtraSemi(ExtraSemiKind Kind, const char* DiagMsg = ""); + //===--------------------------------------------------------------------===// // Scope manipulation diff --git a/clang/lib/Parse/ParseDecl.cpp b/clang/lib/Parse/ParseDecl.cpp index de28c3a..ba40c9b 100644 --- a/clang/lib/Parse/ParseDecl.cpp +++ b/clang/lib/Parse/ParseDecl.cpp @@ -2701,10 +2701,8 @@ void Parser::ParseStructUnionBody(SourceLocation RecordLoc, // Check for extraneous top-level semicolon. if (Tok.is(tok::semi)) { - Diag(Tok, diag::ext_extra_struct_semi) - << DeclSpec::getSpecifierName((DeclSpec::TST)TagType) - << FixItHint::CreateRemoval(Tok.getLocation()); - ConsumeToken(); + ConsumeExtraSemi(InsideStruct, + DeclSpec::getSpecifierName((DeclSpec::TST)TagType)); continue; } diff --git a/clang/lib/Parse/ParseDeclCXX.cpp b/clang/lib/Parse/ParseDeclCXX.cpp index e7d4ac7..0b5c396 100644 --- a/clang/lib/Parse/ParseDeclCXX.cpp +++ b/clang/lib/Parse/ParseDeclCXX.cpp @@ -1929,9 +1929,8 @@ void Parser::ParseCXXClassMemberDeclaration(AccessSpecifier AS, LateParsedAttrs.clear(); // Consume the ';' - it's optional unless we have a delete or default - if (Tok.is(tok::semi)) { - ConsumeToken(); - } + if (Tok.is(tok::semi)) + ConsumeExtraSemi(AfterDefinition); return; } @@ -2280,10 +2279,8 @@ void Parser::ParseCXXMemberSpecification(SourceLocation RecordLoc, // Check for extraneous top-level semicolon. if (Tok.is(tok::semi)) { - Diag(Tok, diag::ext_extra_struct_semi) - << DeclSpec::getSpecifierName((DeclSpec::TST)TagType) - << FixItHint::CreateRemoval(Tok.getLocation()); - ConsumeToken(); + ConsumeExtraSemi(InsideStruct, + DeclSpec::getSpecifierName((DeclSpec::TST)TagType)); continue; } @@ -3007,10 +3004,8 @@ void Parser::ParseMicrosoftIfExistsClassDeclaration(DeclSpec::TST TagType, // Check for extraneous top-level semicolon. if (Tok.is(tok::semi)) { - Diag(Tok, diag::ext_extra_struct_semi) - << DeclSpec::getSpecifierName((DeclSpec::TST)TagType) - << FixItHint::CreateRemoval(Tok.getLocation()); - ConsumeToken(); + ConsumeExtraSemi(InsideStruct, + DeclSpec::getSpecifierName((DeclSpec::TST)TagType)); continue; } diff --git a/clang/lib/Parse/ParseObjc.cpp b/clang/lib/Parse/ParseObjc.cpp index 65bcdf4..164c2f4 100644 --- a/clang/lib/Parse/ParseObjc.cpp +++ b/clang/lib/Parse/ParseObjc.cpp @@ -1259,9 +1259,7 @@ void Parser::ParseObjCClassInstanceVariables(Decl *interfaceDecl, // Check for extraneous top-level semicolon. if (Tok.is(tok::semi)) { - Diag(Tok, diag::ext_extra_ivar_semi) - << FixItHint::CreateRemoval(Tok.getLocation()); - ConsumeToken(); + ConsumeExtraSemi(InstanceVariableList); continue; } diff --git a/clang/lib/Parse/Parser.cpp b/clang/lib/Parse/Parser.cpp index ad283fa..5040714 100644 --- a/clang/lib/Parse/Parser.cpp +++ b/clang/lib/Parse/Parser.cpp @@ -202,6 +202,33 @@ bool Parser::ExpectAndConsumeSemi(unsigned DiagID) { return ExpectAndConsume(tok::semi, DiagID); } +void Parser::ConsumeExtraSemi(ExtraSemiKind Kind, const char* DiagMsg) { + if (!Tok.is(tok::semi)) return; + + // AfterDefinition should only warn when placed on the same line as the + // definition. Otherwise, defer to another semi warning. + if (Kind == AfterDefinition && Tok.isAtStartOfLine()) return; + + SourceLocation StartLoc = Tok.getLocation(); + SourceLocation EndLoc = Tok.getLocation(); + ConsumeToken(); + + while ((Tok.is(tok::semi) && !Tok.isAtStartOfLine())) { + EndLoc = Tok.getLocation(); + ConsumeToken(); + } + + if (Kind == OutsideFunction && getLangOpts().CPlusPlus0x) { + Diag(StartLoc, diag::warn_cxx98_compat_top_level_semi) + << FixItHint::CreateRemoval(SourceRange(StartLoc, EndLoc)); + return; + } + + Diag(StartLoc, diag::ext_extra_semi) + << Kind << DiagMsg << FixItHint::CreateRemoval(SourceRange(StartLoc, + EndLoc)); +} + //===----------------------------------------------------------------------===// // Error recovery. //===----------------------------------------------------------------------===// @@ -582,11 +609,7 @@ Parser::ParseExternalDeclaration(ParsedAttributesWithRange &attrs, HandlePragmaPack(); return DeclGroupPtrTy(); case tok::semi: - Diag(Tok, getLangOpts().CPlusPlus0x ? - diag::warn_cxx98_compat_top_level_semi : diag::ext_top_level_semi) - << FixItHint::CreateRemoval(Tok.getLocation()); - - ConsumeToken(); + ConsumeExtraSemi(OutsideFunction); // TODO: Invoke action for top-level semicolon. return DeclGroupPtrTy(); case tok::r_brace: diff --git a/clang/test/Misc/warning-flags.c b/clang/test/Misc/warning-flags.c index e899517..98130c5 100644 --- a/clang/test/Misc/warning-flags.c +++ b/clang/test/Misc/warning-flags.c @@ -17,7 +17,7 @@ This test serves two purposes: The list of warnings below should NEVER grow. It should gradually shrink to 0. -CHECK: Warnings without flags (245): +CHECK: Warnings without flags (242): CHECK-NEXT: ext_anonymous_struct_union_qualified CHECK-NEXT: ext_binary_literal CHECK-NEXT: ext_cast_fn_obj @@ -33,8 +33,6 @@ CHECK-NEXT: ext_enumerator_list_comma CHECK-NEXT: ext_expected_semi_decl_list CHECK-NEXT: ext_explicit_instantiation_without_qualified_id CHECK-NEXT: ext_explicit_specialization_storage_class -CHECK-NEXT: ext_extra_ivar_semi -CHECK-NEXT: ext_extra_struct_semi CHECK-NEXT: ext_forward_ref_enum CHECK-NEXT: ext_freestanding_complex CHECK-NEXT: ext_hexconstant_invalid @@ -65,7 +63,6 @@ CHECK-NEXT: ext_return_has_void_expr CHECK-NEXT: ext_subscript_non_lvalue CHECK-NEXT: ext_template_arg_extra_parens CHECK-NEXT: ext_thread_before -CHECK-NEXT: ext_top_level_semi CHECK-NEXT: ext_typecheck_addrof_void CHECK-NEXT: ext_typecheck_cast_nonscalar CHECK-NEXT: ext_typecheck_cast_to_union diff --git a/clang/test/Parser/cxx-class.cpp b/clang/test/Parser/cxx-class.cpp index 1b3dd41..75e3fba 100644 --- a/clang/test/Parser/cxx-class.cpp +++ b/clang/test/Parser/cxx-class.cpp @@ -14,9 +14,9 @@ protected: public: void m() { int l = 2; - }; + }; // expected-warning{{extra ';' after function definition}} - template void mt(T) { }; + template void mt(T) { } ; // expected-warning{{extra ';' inside a class}} virtual int vf() const volatile = 0; diff --git a/clang/test/Parser/cxx-extra-semi.cpp b/clang/test/Parser/cxx-extra-semi.cpp new file mode 100644 index 0000000..35c886b --- /dev/null +++ b/clang/test/Parser/cxx-extra-semi.cpp @@ -0,0 +1,25 @@ +// RUN: %clang_cc1 -fsyntax-only -Wextra-semi -verify %s +// RUN: cp %s %t +// RUN: %clang_cc1 -x c++ -Wextra-semi -fixit %t +// RUN: %clang_cc1 -x c++ -Wextra-semi -Werror %t + +class A { + void A1(); + void A2() { }; // expected-warning{{extra ';' after function definition}} + ; // expected-warning{{extra ';' inside a class}} + void A3() { }; ;; // expected-warning{{extra ';' after function definition}} + ;;;;;;; // expected-warning{{extra ';' inside a class}} + ; // expected-warning{{extra ';' inside a class}} + ; ;; ; ;;; // expected-warning{{extra ';' inside a class}} + ; ; ; ; ;; // expected-warning{{extra ';' inside a class}} + void A4(); +}; + +union B { + int a1; + int a2;; // expected-warning{{extra ';' inside a union}} +}; + +; // expected-warning{{extra ';' outside of a function}} +; ;;// expected-warning{{extra ';' outside of a function}} + -- 2.7.4