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

@@ -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<CXX98CompatPedantic>, 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<DiagGroup<"extra-semi">>;
 
 def ext_duplicate_declspec : Extension<"duplicate '%0' declaration specifier">;
 def ext_plain_complex : ExtWarn<

clang/include/clang/Parse/Parser.h

@@ -654,6 +654,17 @@ class Parser : public CodeCompletionHandler {
   /// 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
 

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;
     }
 

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;
     }
 

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;
     }
 

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:

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

clang/test/Parser/cxx-class.cpp

@@ -14,9 +14,9 @@ class C {
 public:
   void m() {
     int l = 2;
-  };
+  }; // expected-warning{{extra ';' after function definition}}
 
-  template<typename T> void mt(T) { };
+  template<typename T> void mt(T) { }
   ; // expected-warning{{extra ';' inside a class}}
 
   virtual int vf() const volatile = 0;

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}}
+