aaronpuchert updated this revision to Diff 190966.
aaronpuchert added a comment.
Don't suggest adding `static` if there is a non-prototype declaration. This
required a minor refactoring: we let `ShouldWarnAboutMissingPrototype` return
any kind of declaration it finds and check for the number of parameters at the
call site.
Repository:
rC Clang
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D59402/new/
https://reviews.llvm.org/D59402
Files:
lib/Sema/SemaDecl.cpp
test/Sema/warn-missing-prototypes.c
test/Sema/warn-missing-variable-declarations.c
test/SemaCXX/warn-missing-variable-declarations.cpp
Index: test/SemaCXX/warn-missing-variable-declarations.cpp
===================================================================
--- test/SemaCXX/warn-missing-variable-declarations.cpp
+++ 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
+// RUN: %clang_cc1 -fsyntax-only -Wmissing-variable-declarations -fdiagnostics-parseable-fixits -std=c++17 %s 2>&1 | FileCheck %s
// Variable declarations that should trigger a warning.
int vbad1; // expected-warning{{no previous extern declaration for non-static variable 'vbad1'}}
+// CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:1-[[@LINE-1]]:1}:"static "
int vbad2 = 10; // expected-warning{{no previous extern declaration for non-static variable 'vbad2'}}
+// CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:1-[[@LINE-1]]:1}:"static "
namespace x {
int vbad3; // expected-warning{{no previous extern declaration for non-static variable 'vbad3'}}
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:3}:"static "
}
// Variable declarations that should not trigger a warning.
@@ -58,7 +62,9 @@
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}}
+// CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]{{.*}}}:"{{.*}}"
extern constexpr int extern_constexpr_var = 0; // expected-warning {{no previous extern declaration}}
+// CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]{{.*}}}:"{{.*}}"
template<typename> int var_template = 0;
template<typename> constexpr int const_var_template = 0;
Index: test/Sema/warn-missing-variable-declarations.c
===================================================================
--- test/Sema/warn-missing-variable-declarations.c
+++ test/Sema/warn-missing-variable-declarations.c
@@ -1,6 +1,8 @@
// RUN: %clang_cc1 -Wmissing-variable-declarations -fsyntax-only -verify %s
+// RUN: %clang_cc1 -fsyntax-only -Wmissing-variable-declarations -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
int vbad1; // expected-warning{{no previous extern declaration for non-static variable 'vbad1'}}
+// CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:1-[[@LINE-1]]:1}:"static "
int vbad2;
int vbad2 = 10; // expected-warning{{no previous extern declaration for non-static variable 'vbad2'}}
@@ -8,6 +10,7 @@
struct {
int mgood1;
} vbad3; // expected-warning{{no previous extern declaration for non-static variable 'vbad3'}}
+// CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:1-[[@LINE-3]]:1}:"static "
int vbad4;
int vbad4 = 10; // expected-warning{{no previous extern declaration for non-static variable 'vbad4'}}
Index: test/Sema/warn-missing-prototypes.c
===================================================================
--- test/Sema/warn-missing-prototypes.c
+++ test/Sema/warn-missing-prototypes.c
@@ -4,15 +4,20 @@
int f();
int f(int x) { return x; } // expected-warning{{no previous prototype for function 'f'}}
+// CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]{{.*}}}:"{{.*}}"
static int g(int x) { return x; }
int h(int x) { return x; } // expected-warning{{no previous prototype for function 'h'}}
+// CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:1-[[@LINE-1]]: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'}}
+// CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]{{.*}}}:"{{.*}}"
+
void test(void);
int h3();
@@ -40,4 +45,4 @@
void not_a_prototype_test(); // expected-note{{this declaration is not a prototype; add 'void' to make it a prototype for a zero-parameter function}}
void not_a_prototype_test() { } // expected-warning{{no previous prototype for function 'not_a_prototype_test'}}
-// CHECK: fix-it:"{{.*}}":{40:27-40:27}:"void"
+// CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:27-[[@LINE-3]]:27}:"void"
Index: lib/Sema/SemaDecl.cpp
===================================================================
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -11805,7 +11805,11 @@
prev = prev->getPreviousDecl();
if (!prev)
- Diag(var->getLocation(), diag::warn_missing_variable_declarations) << var;
+ Diag(var->getLocation(), diag::warn_missing_variable_declarations)
+ << var
+ << ((var->getStorageClass() != SC_Extern)
+ ? FixItHint::CreateInsertion(var->getBeginLoc(), "static ")
+ : FixItHint{});
}
// Cache the result of checking for constant initialization.
@@ -12664,8 +12668,9 @@
Consumer.HandleInlineFunctionDefinition(D);
}
-static bool ShouldWarnAboutMissingPrototype(const FunctionDecl *FD,
- const FunctionDecl*& PossibleZeroParamPrototype) {
+static bool
+ShouldWarnAboutMissingPrototype(const FunctionDecl *FD,
+ const FunctionDecl *&PossiblePrototype) {
// Don't warn about invalid declarations.
if (FD->isInvalidDecl())
return false;
@@ -12702,7 +12707,6 @@
if (FD->isDeleted())
return false;
- bool MissingPrototype = true;
for (const FunctionDecl *Prev = FD->getPreviousDecl();
Prev; Prev = Prev->getPreviousDecl()) {
// Ignore any declarations that occur in function or method
@@ -12710,13 +12714,11 @@
if (Prev->getLexicalDeclContext()->isFunctionOrMethod())
continue;
- MissingPrototype = !Prev->getType()->isFunctionProtoType();
- if (FD->getNumParams() == 0)
- PossibleZeroParamPrototype = Prev;
- break;
+ PossiblePrototype = Prev;
+ return Prev->getType()->isFunctionNoProtoType();
}
- return MissingPrototype;
+ return true;
}
void
@@ -13207,20 +13209,23 @@
// prototype declaration. This warning is issued even if the
// definition itself provides a prototype. The aim is to detect
// global functions that fail to be declared in header files.
- const FunctionDecl *PossibleZeroParamPrototype = nullptr;
- if (ShouldWarnAboutMissingPrototype(FD, PossibleZeroParamPrototype)) {
- Diag(FD->getLocation(), diag::warn_missing_prototype) << FD;
-
- if (PossibleZeroParamPrototype) {
+ const FunctionDecl *PossiblePrototype = nullptr;
+ if (ShouldWarnAboutMissingPrototype(FD, PossiblePrototype)) {
+ Diag(FD->getLocation(), diag::warn_missing_prototype)
+ << FD
+ << (!PossiblePrototype && (FD->getStorageClass() != SC_Extern)
+ ? FixItHint::CreateInsertion(FD->getBeginLoc(), "static ")
+ : FixItHint{});
+
+ if (PossiblePrototype && (FD->getNumParams() == 0)) {
// We found a declaration that is not a prototype,
// but that could be a zero-parameter prototype
- if (TypeSourceInfo *TI =
- PossibleZeroParamPrototype->getTypeSourceInfo()) {
+ if (TypeSourceInfo *TI = PossiblePrototype->getTypeSourceInfo()) {
TypeLoc TL = TI->getTypeLoc();
if (FunctionNoProtoTypeLoc FTL = TL.getAs<FunctionNoProtoTypeLoc>())
- Diag(PossibleZeroParamPrototype->getLocation(),
+ Diag(PossiblePrototype->getLocation(),
diag::note_declaration_not_a_prototype)
- << PossibleZeroParamPrototype
+ << PossiblePrototype
<< FixItHint::CreateInsertion(FTL.getRParenLoc(), "void");
}
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
