Show note for -Wmissing-prototypes for functions with parameters

[aaronpuchert]

Summary

There was a search for non-prototype declarations for the function, but
we only showed the results for zero-parameter functions. Now we show the
note for functions with parameters as well, but we omit the fix-it hint
suggesting to add void.

[aaronpuchert]

Add checks that we don't emit the fix-it when we shouldn't.

[aaron.ballman]

LGTM!

lib/Sema/SemaDecl.cpp

13335 ↗

(On Diff #204171)

In this case, we could probably generate the parameter declarations for the user still, couldn't we? e.g.,

int f(); // We could insert "int i, int j" here as the fixit
int g(); // Just like we try to suggest "void" here already

int f(int i, int j) { return i + j; }
int g(void) { return 12; }

That could be done in a follow-up patch though.

[aaronpuchert]

lib/Sema/SemaDecl.cpp

13335 ↗

(On Diff #204171)

One difficulty might be that certain types used in the parameter list of the definition aren't available where the declaration is, like

int f();

struct X { int n; };
int f(struct X *x) { return x->n;}

So I'm not sure we can always do it, but sometimes perhaps.

[aaron.ballman]

lib/Sema/SemaDecl.cpp

13335 ↗

(On Diff #204171)

That's a good point and reason enough to not try for it in this version of the patch (if at all).

cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td

def warn_missing_sentinel : Warning <
   "missing sentinel in %select{function call|method dispatch|block call}0">,
   InGroup<Sentinel>;
 def note_sentinel_here : Note<
   "%select{function|method|block}0 has been explicitly marked sentinel here">;
 def warn_missing_prototype : Warning<
   "no previous prototype for function %0">,
   InGroup<DiagGroup<"missing-prototypes">>, DefaultIgnore;
 def note_declaration_not_a_prototype : Note<
-  "this declaration is not a prototype; add 'void' to make it a prototype for a zero-parameter function">;
+  "this declaration is not a prototype; add %select{'void'|parameter declarations}0 "
+  "to make it %select{a prototype for a zero-parameter function|one}0">;
 def warn_strict_prototypes : Warning<
   "this %select{function declaration is not|block declaration is not|"
   "old-style function definition is not preceded by}0 a prototype">,
   InGroup<DiagGroup<"strict-prototypes">>, DefaultIgnore;
 def warn_missing_variable_declarations : Warning<
   "no previous extern declaration for non-static variable %0">,
   InGroup<DiagGroup<"missing-variable-declarations">>, DefaultIgnore;
 def err_static_data_member_reinitialization :

cfe/trunk/lib/Sema/SemaDecl.cpp

Sema::ActOnStartOfFunctionDef(Scope *FnBodyScope, Declarator &D,
   Decl *DP = HandleDeclarator(ParentScope, D, TemplateParameterLists);
   return ActOnStartOfFunctionDef(FnBodyScope, DP, SkipBody);
 }
 
 void Sema::ActOnFinishInlineFunctionDef(FunctionDecl *D) {
   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;
 
   // Or declarations that aren't global.
   if (!FD->isGlobal())
     return false;
 
ShouldWarnAboutMissingPrototype(const FunctionDecl *FD,
   // Don't warn for OpenCL kernels.
   if (FD->hasAttr<OpenCLKernelAttr>())
     return false;
 
   // Don't warn on explicitly deleted functions.
   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
     // scope, because they aren't visible from the header.
     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
 Sema::CheckForFunctionRedefinition(FunctionDecl *FD,
                                    const FunctionDecl *EffectiveDefinition,
                                    SkipBodyInfo *SkipBody) {
   const FunctionDecl *Definition = EffectiveDefinition;
   if (!Definition && !FD->isDefined(Definition) && !FD->isCXXClassMember()) {
if (!FD->isInvalidDecl()) {
         computeNRVO(Body, getCurFunction());
     }
 
     // GNU warning -Wmissing-prototypes:
     //   Warn if a global function is defined without a previous
     //   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)) {
+    const FunctionDecl *PossiblePrototype = nullptr;
+    if (ShouldWarnAboutMissingPrototype(FD, PossiblePrototype)) {
       Diag(FD->getLocation(), diag::warn_missing_prototype) << FD;
 
-      if (PossibleZeroParamPrototype) {
+      if (PossiblePrototype) {
         // 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
-                << FixItHint::CreateInsertion(FTL.getRParenLoc(), "void");
+                << (FD->getNumParams() != 0)
+                << (FD->getNumParams() == 0
+                        ? FixItHint::CreateInsertion(FTL.getRParenLoc(), "void")
+                        : FixItHint{});
         }
       }
 
       // GNU warning -Wstrict-prototypes
       //   Warn if K&R function is defined without a previous declaration.
       //   This warning is issued only if the definition itself does not provide
       //   a prototype. Only K&R definitions do not provide a prototype.
       //   An empty list in a function declarator that is part of a definition

cfe/trunk/test/Sema/warn-missing-prototypes.c

 // RUN: %clang_cc1 -fsyntax-only -Wdocumentation -Wmissing-prototypes -verify %s
 // RUN: %clang_cc1 -fsyntax-only -Wdocumentation -Wmissing-prototypes -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
 
-int f();
+int f(); // expected-note{{this declaration is not a prototype; add parameter declarations to make it one}}
+// CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]:{{.*}}-[[@LINE-1]]:{{.*}}}:"{{.*}}"
 
 int f(int x) { return x; } // expected-warning{{no previous prototype for function 'f'}}
 
 static int g(int x) { return x; }
 
 int h(int x) { return x; } // expected-warning{{no previous prototype for function 'h'}}
 
 static int g2();
 
 int g2(int x) { return x; }
 
 void test(void);
 
-int h3();
+int h3(); // expected-note{{this declaration is not a prototype; add parameter declarations to make it one}}
+// CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]:{{.*}}-[[@LINE-1]]:{{.*}}}:"{{.*}}"
 int h4(int);
 int h4();
 
 void test(void) {
   int h2(int x);
   int h3(int x);
   int h4();
 }
 
 int h2(int x) { return x; } // expected-warning{{no previous prototype for function 'h2'}}
 int h3(int x) { return x; } // expected-warning{{no previous prototype for function 'h3'}}
 int h4(int x) { return x; }
 
 int f2(int);
 int f2();
 
 int f2(int x) { return x; }
 
 // rdar://6759522
 int main(void) { return 0; }
 
 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}}
+// CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:27-[[@LINE-1]]:27}:"void"
 void not_a_prototype_test() { } // expected-warning{{no previous prototype for function 'not_a_prototype_test'}}
-
-// CHECK: fix-it:"{{.*}}":{40:27-40:27}:"void"