Bug 20796 - GCC's -Wstrict-prototypes warning not implemented in Clang

[arphaman]

Summary

https://llvm.org/bugs/show_bug.cgi?id=20796
Implementation for C only warning -Wstrict-prototypes. Function declarations which have no parameters specified are diagnosed and also K&R function definitions with more than 0 parameters which are not preceded by previous prototype declaration.

[cfe-commits]

Thanks for working on this!

[ptitei]

Thanks for the fast review.

[thakis]

(This is http://llvm.org/PR20796, which has some discussion and test cases. If you haven't, maybe you want to look at the test cases there. It's possible they're a subset of your test though.)

[bruno]

Hi Paul,

I'm interested in this patch. Do you have any plans to wrap it up?

[arphaman]

Hi Paul,

Are you planning to work on this patch? If not, can I take a shot at it?

Cheers,
Alex

[arphaman]

I rebased the patch, adjusted the test and added a test case for Objective-C blocks.

[arphaman]

Ping

[bruno]

Hi Alex, thanks for following up with this!

lib/Sema/SemaDecl.cpp

11819

There's an extra 'declaration' here

lib/Sema/SemaType.cpp

4332

No need for the curly braces here!

[arphaman]

The updated patch addresses Bruno's comments.

[bruno]

LGTM

include/clang/Basic/DiagnosticSemaKinds.td

def warn_missing_sentinel : Warning <
   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">; 
+def warn_strict_prototypes : Warning<
+  "this %select{function 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 :
   Error<"static data member %0 already has an initializer">;
 def err_redefinition : Error<"redefinition of %0">;
 def err_alias_after_tentative :
   Error<"alias definition of %0 after tentative definition">;

lib/Sema/SemaDecl.cpp

         FD->setType(Context.getFunctionType(RetType, Proto->getParamTypes(),
                                             Proto->getExtProtoInfo()));
       }
     }
 
     // The only way to be included in UndefinedButUsed is if there is an
     // ODR use before the definition. Avoid the expensive map lookup if this
     // is the first declaration.
     if (!FD->isFirstDecl() && FD->getPreviousDecl()->isUsed()) {
       if (!FD->isExternallyVisible())
         UndefinedButUsed.erase(FD);
       else if (FD->isInlined() &&
                !LangOpts.GNUInline &&
                (!FD->getPreviousDecl()->hasAttr<GNUInlineAttr>()))
         UndefinedButUsed.erase(FD);
     }
 

           TypeLoc TL = TI->getTypeLoc();
           if (FunctionNoProtoTypeLoc FTL = TL.getAs<FunctionNoProtoTypeLoc>())
             Diag(PossibleZeroParamPrototype->getLocation(),
                  diag::note_declaration_not_a_prototype)
                 << PossibleZeroParamPrototype
                 << FixItHint::CreateInsertion(FTL.getRParenLoc(), "void");
         }
       }
+
+      // 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
+      //   of that function specifies that the function has no parameters
+      //   (C99 6.7.5.3p14)
+      if (!FD->hasWrittenPrototype() && FD->getNumParams() > 0 &&
+          !LangOpts.CPlusPlus) {
+        TypeSourceInfo *TI = FD->getTypeSourceInfo();
+        TypeLoc TL = TI->getTypeLoc();
+        FunctionTypeLoc FTL = TL.castAs<FunctionTypeLoc>();
+        Diag(FTL.getLParenLoc(), diag::warn_strict_prototypes) << 1;
+      }
     }
 
     if (auto *MD = dyn_cast<CXXMethodDecl>(FD)) {
       const CXXMethodDecl *KeyFunction;
       if (MD->isOutOfLine() && (MD = MD->getCanonicalDecl()) &&
           MD->isVirtual() &&
           (KeyFunction = Context.getCurrentKeyFunction(MD->getParent())) &&
           MD == KeyFunction->getCanonicalDecl()) {

lib/Sema/SemaType.cpp

case DeclaratorChunk::Function: {
             << (D.getContext() == Declarator::AliasDeclContext ||
                 D.getContext() == Declarator::AliasTemplateContext);
 
       // If we see "T var();" or "T var(T());" at block scope, it is probably
       // an attempt to initialize a variable, not a function declaration.
       if (FTI.isAmbiguous)
         warnAboutAmbiguousFunction(S, D, DeclType, T);
 
+      // GNU warning -Wstrict-prototypes
+      //   Warn if a function declaration is without a prototype.
+      //   This warning is issued for all kinds of unprototyped function
+      //   declarations (i.e. function type typedef, function pointer etc.)
+      //   C99 6.7.5.3p14:
+      //   The empty list in a function declarator that is not part of a
+      //   definition of that function specifies that no information
+      //   about the number or types of the parameters is supplied.
+      if (D.getFunctionDefinitionKind() == FDK_Declaration &&
+          FTI.NumParams == 0 && !LangOpts.CPlusPlus)
+        S.Diag(DeclType.Loc, diag::warn_strict_prototypes)
+            << 0 << FixItHint::CreateInsertion(FTI.getRParenLoc(), "void");
+
       FunctionType::ExtInfo EI(getCCForDeclaratorChunk(S, D, FTI, chunkIndex));
 
       if (!FTI.NumParams && !FTI.isVariadic && !LangOpts.CPlusPlus) {
         // Simple void foo(), where the incoming T is the result type.
         T = Context.getFunctionNoProtoType(T, EI);
       } else {
         // We allow a zero-parameter variadic function in C if the
         // function is marked with the "overloadable" attribute. Scan

test/Sema/warn-strict-prototypes.c

+// RUN: %clang_cc1 -fsyntax-only -Wstrict-prototypes -verify %s
+// RUN: %clang_cc1 -fsyntax-only -Wstrict-prototypes -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
+
+// function declaration with unspecified params
+void foo1(); // expected-warning {{this function declaration is not a prototype}}
+             // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:11-[[@LINE-1]]:11}:"void"
+// function declaration with 0 params
+void foo2(void);
+
+// function definition with 0 params(for both cases),
+// valid according to 6.7.5.3/14
+void foo1() {}
+void foo2(void) {}
+
+// function type typedef unspecified params
+typedef void foo3(); // expected-warning {{this function declaration is not a prototype}}
+                     // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:19-[[@LINE-1]]:19}:"void"
+
+// global fp unspecified params
+void (*foo4)(); // expected-warning {{this function declaration is not a prototype}}
+                // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:14-[[@LINE-1]]:14}:"void"
+
+// struct member fp unspecified params
+struct { void (*foo5)(); } s; // expected-warning {{this function declaration is not a prototype}}
+                              // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:23-[[@LINE-1]]:23}:"void"
+
+// param fp unspecified params
+void bar2(void (*foo6)()) { // expected-warning {{this function declaration is not a prototype}}
+                            // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:24-[[@LINE-1]]:24}:"void"
+  // local fp unspecified params
+  void (*foo7)() = 0; // expected-warning {{this function declaration is not a prototype}}
+                      // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:16-[[@LINE-1]]:16}:"void"
+  // array fp unspecified params
+  void (*foo8[2])() = {0}; // expected-warning {{this function declaration is not a prototype}}
+                           // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:19-[[@LINE-1]]:19}:"void"
+}
+
+// function type cast using using an anonymous function declaration
+void bar3(void) {
+  // casting function w/out prototype to unspecified params function type
+  (void)(void(*)()) foo1; // expected-warning {{this function declaration is not a prototype}}
+                          // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:18-[[@LINE-1]]:18}:"void"
+  // .. specified params
+  (void)(void(*)(void)) foo1;
+}
+
+// K&R function definition not preceded by full prototype
+int foo9(a, b) // expected-warning {{old-style function definition is not preceded by a prototype}}
+  int a, b;
+{
+  return a + b;
+}
+
+// Function declaration with no types
+void foo10(); // expected-warning {{this function declaration is not a prototype}}
+              // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:12-[[@LINE-1]]:12}:"void"
+// K&R function definition with incomplete param list declared
+void foo10(p, p2) void *p; {} // expected-warning {{old-style function definition is not preceded by a prototype}}
+
+// K&R function definition with previous prototype declared is not diagnosed.
+void foo11(int p, int p2);
+void foo11(p, p2) int p; int p2; {}

test/Sema/warn-strict-prototypes.m

+// RUN: %clang_cc1 -fsyntax-only -Wstrict-prototypes -verify -fblocks %s
+
+@interface Foo
+
+@property (nonatomic, copy) void (^noProtoBlock)(); // expected-warning {{this function declaration is not a prototype}}
+@property (nonatomic, copy) void (^block)(void); // no warning
+
+- doStuff:(void (^)()) completionHandler; // expected-warning {{this function declaration is not a prototype}}
+- doOtherStuff:(void (^)(void)) completionHandler; // no warning
+
+@end
+
+void foo() {
+  void (^block)() = // expected-warning {{this function declaration is not a prototype}}
+                    ^void(int arg) { // no warning
+  };
+  void (^block2)(void) = // no warning
+                         ^void() { // expected-warning {{this function declaration is not a prototype}}
+  };
+}