Suggestions to fix -Wmissing-{prototypes,variable-declarations}

Summary:
I've found that most often the proper way to fix this warning is to add
`static`, because if the code otherwise compiles and links, the function
or variable is apparently not needed outside of the TU.

We can't provide a fix-it hint for variable declarations, because
multiple VarDecls can share the same type, and if we put static in front
of that, we affect all declared variables, some of which might have
previous declarations.

We also provide no fix-it hint for the rare case of an `extern` function
definition, because that would require removing `extern` and I have no
idea how to get the source location of the storage class specifier from
a FunctionDecl. I believe this information is only available earlier in
the AST construction from DeclSpec::getStorageClassSpecLoc(), but we
don't have that here.

Reviewed By: aaron.ballman

Differential Revision: https://reviews.llvm.org/D59402

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@363749 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td
index b1816c238608780bf16c79d7dd9e1a25fe55c78c..019518204ac30135068e9cdf3fa7dc3f483e9c56 100644 (file)
--- a/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/include/clang/Basic/DiagnosticSemaKinds.td
@@ -4681,6 +4681,9 @@ def warn_strict_prototypes : Warning<
 def warn_missing_variable_declarations : Warning<
   "no previous extern declaration for non-static variable %0">,
   InGroup<DiagGroup<"missing-variable-declarations">>, DefaultIgnore;
+def note_static_for_internal_linkage : Note<
+  "declare 'static' if the %select{variable|function}0 is not intended to be "
+  "used outside of this translation unit">;
 def err_static_data_member_reinitialization :
   Error<"static data member %0 already has an initializer">;
 def err_redefinition : Error<"redefinition of %0">;

diff --git a/lib/Sema/SemaDecl.cpp b/lib/Sema/SemaDecl.cpp
index e92c3d0a19a66292e136d192228ebe928ec24438..03b6cc916570833dcd21d5eec5e8166fe70bec55 100644 (file)
--- a/lib/Sema/SemaDecl.cpp
+++ b/lib/Sema/SemaDecl.cpp
@@ -11899,8 +11899,11 @@ void Sema::CheckCompleteVariableDeclaration(VarDecl *var) {
     while (prev && prev->isThisDeclarationADefinition())
       prev = prev->getPreviousDecl();
 
-    if (!prev)
+    if (!prev) {
       Diag(var->getLocation(), diag::warn_missing_variable_declarations) << var;
+      Diag(var->getTypeSpecStartLoc(), diag::note_static_for_internal_linkage)
+          << /* variable */ 0;
+    }
   }
 
   // Cache the result of checking for constant initialization.
@@ -13364,6 +13367,13 @@ Decl *Sema::ActOnFinishFunctionBody(Decl *dcl, Stmt *Body,
                         ? FixItHint::CreateInsertion(FTL.getRParenLoc(), "void")
                         : FixItHint{});
         }
+      } else {
+        Diag(FD->getTypeSpecStartLoc(), diag::note_static_for_internal_linkage)
+            << /* function */ 1
+            << (FD->getStorageClass() == SC_None
+                    ? FixItHint::CreateInsertion(FD->getTypeSpecStartLoc(),
+                                                 "static ")
+                    : FixItHint{});
       }
 
       // GNU warning -Wstrict-prototypes

diff --git a/test/Sema/warn-missing-prototypes.c b/test/Sema/warn-missing-prototypes.c
index 892e06643180da3c6b8380768e98d8c94d828e77..5940a49572ba13a3325b791391e5729baced2a6c 100644 (file)
--- a/test/Sema/warn-missing-prototypes.c
+++ b/test/Sema/warn-missing-prototypes.c
@@ -9,11 +9,17 @@ int f(int x) { return x; } // expected-warning{{no previous prototype for functi
 static int g(int x) { return x; }
 
 int h(int x) { return x; } // expected-warning{{no previous prototype for function 'h'}}
+// expected-note@-1{{declare 'static' if the function is not intended to be used outside of this translation unit}}
+// CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:1-[[@LINE-2]]:1}:"static "
 
 static int g2();
 
 int g2(int x) { return x; }
 
+extern int g3(int x) { return x; } // expected-warning{{no previous prototype for function 'g3'}}
+// expected-note@-1{{declare 'static' if the function is not intended to be used outside of this translation unit}}
+// CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-2]]:{{.*}}-[[@LINE-2]]:{{.*}}}:"{{.*}}"
+
 void test(void);
 
 int h3(); // expected-note{{this declaration is not a prototype; add parameter declarations to make it one}}
@@ -28,6 +34,7 @@ void test(void) {
 }
 
 int h2(int x) { return x; } // expected-warning{{no previous prototype for function 'h2'}}
+// expected-note@-1{{declare 'static' if the function is not intended to be used outside of this translation unit}}
 int h3(int x) { return x; } // expected-warning{{no previous prototype for function 'h3'}}
 int h4(int x) { return x; }
 

diff --git a/test/Sema/warn-missing-variable-declarations.c b/test/Sema/warn-missing-variable-declarations.c
index e5ce97da2164ee660788bf2528b9defee8825ad7..f9644bec603acbdab4acd7ded974ae71a54d63dd 100644 (file)
--- a/test/Sema/warn-missing-variable-declarations.c
+++ b/test/Sema/warn-missing-variable-declarations.c
@@ -1,16 +1,19 @@
 // RUN: %clang_cc1 -Wmissing-variable-declarations -fsyntax-only -verify %s
 
 int vbad1; // expected-warning{{no previous extern declaration for non-static variable 'vbad1'}}
+// expected-note@-1{{declare 'static' if the variable is not intended to be used outside of this translation unit}}
 
 int vbad2;
 int vbad2 = 10; // expected-warning{{no previous extern declaration for non-static variable 'vbad2'}}
+// expected-note@-1{{declare 'static' if the variable is not intended to be used outside of this translation unit}}
 
-struct {
+struct { // expected-note{{declare 'static' if the variable is not intended to be used outside of this translation unit}}
   int mgood1;
 } vbad3; // expected-warning{{no previous extern declaration for non-static variable 'vbad3'}}
 
 int vbad4;
 int vbad4 = 10; // expected-warning{{no previous extern declaration for non-static variable 'vbad4'}}
+// expected-note@-1{{declare 'static' if the variable is not intended to be used outside of this translation unit}}
 extern int vbad4;
 
 extern int vgood1;

diff --git a/test/SemaCXX/warn-missing-prototypes.cpp b/test/SemaCXX/warn-missing-prototypes.cpp
index cb419336020e614f751f8bf33f1ee5a48d42fa90..bb71aa8b142d17e0e672099d3575ee898218115f 100644 (file)
--- a/test/SemaCXX/warn-missing-prototypes.cpp
+++ b/test/SemaCXX/warn-missing-prototypes.cpp
@@ -1,9 +1,13 @@
 // RUN: %clang_cc1 -fsyntax-only -verify -Wmissing-prototypes -std=c++11 %s
+// RUN: %clang_cc1 -fsyntax-only -Wmissing-prototypes -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
 
 void f() { } // expected-warning {{no previous prototype for function 'f'}}
+// expected-note@-1{{declare 'static' if the function is not intended to be used outside of this translation unit}}
+// CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:1-[[@LINE-2]]:1}:"static "
 
 namespace NS {
   void f() { } // expected-warning {{no previous prototype for function 'f'}}
+  // expected-note@-1{{declare 'static' if the function is not intended to be used outside of this translation unit}}
 }
 
 namespace {
@@ -32,3 +36,7 @@ class I {
 
 // Don't warn on explicitly deleted functions.
 void j() = delete;
+
+extern void k() {} // expected-warning {{no previous prototype for function 'k'}}
+// expected-note@-1{{declare 'static' if the function is not intended to be used outside of this translation unit}}
+// CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-2]]:{{.*}}-[[@LINE-2]]:{{.*}}}:"{{.*}}"

diff --git a/test/SemaCXX/warn-missing-variable-declarations.cpp b/test/SemaCXX/warn-missing-variable-declarations.cpp
index 5b71f38c811f0c0f49f1ecd27da4004489ee3002..e2480fd663b0a2b314b7e7a582de19c9b4b6cb5d 100644 (file)
--- a/test/SemaCXX/warn-missing-variable-declarations.cpp
+++ b/test/SemaCXX/warn-missing-variable-declarations.cpp
@@ -1,11 +1,15 @@
-// RUN: %clang -Wmissing-variable-declarations -fsyntax-only -Xclang -verify -std=c++17 %s
+// RUN: %clang_cc1 -fsyntax-only -verify -Wmissing-variable-declarations -std=c++17 %s
 
 // Variable declarations that should trigger a warning.
 int vbad1; // expected-warning{{no previous extern declaration for non-static variable 'vbad1'}}
+// expected-note@-1{{declare 'static' if the variable is not intended to be used outside of this translation unit}}
+
 int vbad2 = 10; // expected-warning{{no previous extern declaration for non-static variable 'vbad2'}}
+// expected-note@-1{{declare 'static' if the variable is not intended to be used outside of this translation unit}}
 
 namespace x {
   int vbad3; // expected-warning{{no previous extern declaration for non-static variable 'vbad3'}}
+  // expected-note@-1{{declare 'static' if the variable is not intended to be used outside of this translation unit}}
 }
 
 // Variable declarations that should not trigger a warning.
@@ -58,7 +62,9 @@ const int const_var = 0;
 constexpr int constexpr_var = 0;
 inline constexpr int inline_constexpr_var = 0;
 extern const int extern_const_var = 0; // expected-warning {{no previous extern declaration}}
+// expected-note@-1{{declare 'static' if the variable is not intended to be used outside of this translation unit}}
 extern constexpr int extern_constexpr_var = 0; // expected-warning {{no previous extern declaration}}
+// expected-note@-1{{declare 'static' if the variable is not intended to be used outside of this translation unit}}
 
 template<typename> int var_template = 0;
 template<typename> constexpr int const_var_template = 0;
@@ -69,7 +75,9 @@ int use_var_template() { return var_template<int[2]>; }
 template int var_template<int[3]>;
 extern template int var_template<int[4]>;
 template<> int var_template<int[5]>; // expected-warning {{no previous extern declaration}}
+// expected-note@-1{{declare 'static' if the variable is not intended to be used outside of this translation unit}}
 
 // FIXME: We give this specialization internal linkage rather than inheriting
 // the linkage from the template! We should not warn here.
 template<> int static_var_template<int[5]>; // expected-warning {{no previous extern declaration}}
+// expected-note@-1{{declare 'static' if the variable is not intended to be used outside of this translation unit}}

diff --git a/test/SemaOpenCL/warn-missing-prototypes.cl b/test/SemaOpenCL/warn-missing-prototypes.cl
index 487cb28399a32fe4b58a70e56546e9e02b652cd4..1b2eed555d475c13abed1f3350bb71cfb76523cb 100644 (file)
--- a/test/SemaOpenCL/warn-missing-prototypes.cl
+++ b/test/SemaOpenCL/warn-missing-prototypes.cl
@@ -1,6 +1,7 @@
 // RUN: %clang_cc1 -fsyntax-only -verify -Wmissing-prototypes %s
 
 void f() { } // expected-warning {{no previous prototype for function 'f'}}
+// expected-note@-1{{declare 'static' if the function is not intended to be used outside of this translation unit}}
 
 // Don't warn about kernel functions.
 kernel void g() { }