[C11/C2x] Change the behavior of the implicit function declaration warning

C89 had a questionable feature where the compiler would implicitly
declare a function that the user called but was never previously
declared. The resulting function would be globally declared as
extern int func(); -- a function without a prototype which accepts zero
or more arguments.

C99 removed support for this questionable feature due to severe
security concerns. However, there was no deprecation period; C89 had
the feature, C99 didn't. So Clang (and GCC) both supported the
functionality as an extension in C99 and later modes.

C2x no longer supports that function signature as it now requires all
functions to have a prototype, and given the known security issues with
the feature, continuing to support it as an extension is not tenable.

This patch changes the diagnostic behavior for the
-Wimplicit-function-declaration warning group depending on the language
mode in effect. We continue to warn by default in C89 mode (due to the
feature being dangerous to use). However, because this feature will not
be supported in C2x mode, we've diagnosed it as being invalid for so
long, the security concerns with the feature, and the trivial
workaround for users (declare the function), we now default the
extension warning to an error in C99-C17 mode. This still gives users
an easy workaround if they are extensively using the extension in those
modes (they can disable the warning or use -Wno-error to downgrade the
error), but the new diagnostic makes it more clear that this feature is
not supported and should be avoided. In C2x mode, we no longer allow an
implicit function to be defined and treat the situation the same as any
other lookup failure.

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

Author: AaronBallman

clang-tools-extra/clangd/IncludeFixer.cpp

@@ -197,7 +197,7 @@ std::vector<Fix> IncludeFixer::fix(DiagnosticsEngine::Level DiagLevel,
   case diag::err_no_member_template:
   case diag::err_no_member_template_suggest:
   case diag::warn_implicit_function_decl:
-  case diag::ext_implicit_function_decl:
+  case diag::ext_implicit_function_decl_c99:
   case diag::err_opencl_implicit_function_decl:
     dlog("Unresolved name at {0}, last typo was {1}",
          Info.getLocation().printToString(Info.getSourceManager()),

clang-tools-extra/clangd/ParsedAST.cpp

@@ -428,7 +428,8 @@ ParsedAST::build(llvm::StringRef Filename, const ParseInputs &Inputs,
     // this is an error. (The actual typo correction is nice too).
     // We restore the original severity in the level adjuster.
     // FIXME: It would be better to have a real API for this, but what?
-    for (auto ID : {diag::ext_implicit_function_decl,
+    for (auto ID : {diag::ext_implicit_function_decl_c99,
+                    diag::ext_implicit_lib_function_decl_c99,
                     diag::warn_implicit_function_decl}) {
       OverriddenSeverity.try_emplace(
           ID, Clang->getDiagnostics().getDiagnosticLevel(ID, SourceLocation()));

clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp

@@ -1409,7 +1409,7 @@ TEST(IncludeFixerTest, NoCrashOnTemplateInstantiations) {
 TEST(IncludeFixerTest, HeaderNamedInDiag) {
   Annotations Test(R"cpp(
     $insert[[]]int main() {
-      [[printf]]("");
+      [[printf]](""); // error-ok
     }
   )cpp");
   auto TU = TestTU::withCode(Test.code());
@@ -1420,16 +1420,19 @@ TEST(IncludeFixerTest, HeaderNamedInDiag) {
   EXPECT_THAT(
       *TU.build().getDiagnostics(),
       ElementsAre(AllOf(
-          Diag(Test.range(), "implicitly declaring library function 'printf' "
-                             "with type 'int (const char *, ...)'"),
+          Diag(Test.range(), "call to undeclared library function 'printf' "
+                             "with type 'int (const char *, ...)'; ISO C99 "
+                             "and later do not support implicit function "
+                             "declarations"),
           withFix(Fix(Test.range("insert"), "#include <stdio.h>\n",
                       "Include <stdio.h> for symbol printf")))));
 }
 
 TEST(IncludeFixerTest, CImplicitFunctionDecl) {
-  Annotations Test("void x() { [[foo]](); }");
+  Annotations Test("void x() { [[foo]](); /* error-ok */ }");
   auto TU = TestTU::withCode(Test.code());
   TU.Filename = "test.c";
+  TU.ExtraArgs.push_back("-std=c99");
 
   Symbol Sym = func("foo");
   Sym.Flags |= Symbol::IndexedForCodeCompletion;
@@ -1446,7 +1449,8 @@ TEST(IncludeFixerTest, CImplicitFunctionDecl) {
       *TU.build().getDiagnostics(),
       ElementsAre(AllOf(
           Diag(Test.range(),
-               "implicit declaration of function 'foo' is invalid in C99"),
+               "call to undeclared function 'foo'; ISO C99 and later do not "
+               "support implicit function declarations"),
           withFix(Fix(Range{}, "#include \"foo.h\"\n",
                       "Include \"foo.h\" for symbol foo")))));
 }

clang/docs/ReleaseNotes.rst

@@ -156,6 +156,12 @@ Improvements to Clang's diagnostics
   without a prototype and with no arguments is an invalid redeclaration of a
   function with a prototype. e.g., ``void f(int); void f() {}`` is now properly
   diagnosed.
+- The ``-Wimplicit-function-declaration`` warning diagnostic now defaults to
+  an error in C99 and later. Prior to C2x, it may be downgraded to a warning
+  with ``-Wno-error=implicit-function-declaration``, or disabled entirely with
+  ``-Wno-implicit-function-declaration``. As of C2x, support for implicit
+  function declarations has been removed, and the warning options will have no
+  effect.
 
 - ``-Wmisexpect`` warns when the branch weights collected during profiling
   conflict with those added by ``llvm.expect``.
@@ -230,6 +236,9 @@ C2x Feature Support
 - Implemented the `*_WIDTH` macros to complete support for
   `WG14 N2412 Two's complement sign representation for C2x <https://www9.open-std.org/jtc1/sc22/wg14/www/docs/n2412.pdf>`_.
 - Implemented `WG14 N2418 Adding the u8 character prefix <http://www.open-std.org/jtc1/sc22/wg14/www/docs/n2418.pdf>`_.
+- Removed support for implicit function declarations. This was a C89 feature
+  that was removed in C99, but cannot be supported in C2x because it requires
+  support for functions without prototypes, which no longer exist in C2x.
 
 C++ Language Changes in Clang
 -----------------------------

clang/include/clang/Basic/DiagnosticSemaKinds.td

@@ -417,9 +417,9 @@ def warn_return_value_udt_incomplete: Warning<
 def warn_implicit_function_decl : Warning<
   "implicit declaration of function %0">,
   InGroup<ImplicitFunctionDeclare>, DefaultIgnore;
-def ext_implicit_function_decl : ExtWarn<
-  "implicit declaration of function %0 is invalid in C99">,
-  InGroup<ImplicitFunctionDeclare>;
+def ext_implicit_function_decl_c99 : ExtWarn<
+  "call to undeclared function %0; ISO C99 and later do not support implicit "
+  "function declarations">, InGroup<ImplicitFunctionDeclare>, DefaultError;
 def note_function_suggestion : Note<"did you mean %0?">;
 
 def err_ellipsis_first_param : Error<
@@ -698,6 +698,10 @@ def note_unreachable_silence : Note<
 def ext_implicit_lib_function_decl : ExtWarn<
   "implicitly declaring library function '%0' with type %1">,
   InGroup<ImplicitFunctionDeclare>;
+def ext_implicit_lib_function_decl_c99 : ExtWarn<
+  "call to undeclared library function '%0' with type %1; ISO C99 and later "
+  "do not support implicit function declarations">,
+  InGroup<ImplicitFunctionDeclare>, DefaultError;
 def note_include_header_or_declare : Note<
   "include the header <%0> or explicitly provide a declaration for '%1'">;
 def note_previous_builtin_declaration : Note<"%0 is a builtin with type %1">;

clang/lib/Sema/SemaDecl.cpp

@@ -933,9 +933,12 @@ Sema::NameClassification Sema::ClassifyName(Scope *S, CXXScopeSpec &SS,
       //
       //   appeared.
       //
-      // We also allow this in C99 as an extension.
-      if (NamedDecl *D = ImplicitlyDefineFunction(NameLoc, *Name, S))
-        return NameClassification::NonType(D);
+      // We also allow this in C99 as an extension. However, this is not
+      // allowed in C2x as there are no functions without prototypes there.
+      if (!getLangOpts().C2x) {
+        if (NamedDecl *D = ImplicitlyDefineFunction(NameLoc, *Name, S))
+          return NameClassification::NonType(D);
+      }
     }
 
     if (getLangOpts().CPlusPlus20 && SS.isEmpty() && NextToken.is(tok::less)) {
@@ -2302,7 +2305,8 @@ NamedDecl *Sema::LazilyCreateBuiltin(IdentifierInfo *II, unsigned ID,
   if (!ForRedeclaration &&
       (Context.BuiltinInfo.isPredefinedLibFunction(ID) ||
        Context.BuiltinInfo.isHeaderDependentFunction(ID))) {
-    Diag(Loc, diag::ext_implicit_lib_function_decl)
+    Diag(Loc, LangOpts.C99 ? diag::ext_implicit_lib_function_decl_c99
+                           : diag::ext_implicit_lib_function_decl)
         << Context.BuiltinInfo.getName(ID) << R;
     if (const char *Header = Context.BuiltinInfo.getHeaderName(ID))
       Diag(Loc, diag::note_include_header_or_declare)
@@ -15267,6 +15271,9 @@ void Sema::ActOnFinishDelayedAttribute(Scope *S, Decl *D,
 /// call, forming a call to an implicitly defined function (per C99 6.5.1p2).
 NamedDecl *Sema::ImplicitlyDefineFunction(SourceLocation Loc,
                                           IdentifierInfo &II, Scope *S) {
+  // It is not valid to implicitly define a function in C2x.
+  assert(!LangOpts.C2x && "Cannot implicitly define a function in C2x");
+
   // Find the scope in which the identifier is injected and the corresponding
   // DeclContext.
   // FIXME: C89 does not say what happens if there is no enclosing block scope.
@@ -15305,15 +15312,15 @@ NamedDecl *Sema::ImplicitlyDefineFunction(SourceLocation Loc,
     }
   }
 
-  // Extension in C99.  Legal in C90, but warn about it.
+  // Extension in C99 (defaults to error). Legal in C89, but warn about it.
   unsigned diag_id;
   if (II.getName().startswith("__builtin_"))
     diag_id = diag::warn_builtin_unknown;
   // OpenCL v2.0 s6.9.u - Implicit function declaration is not supported.
   else if (getLangOpts().OpenCL)
     diag_id = diag::err_opencl_implicit_function_decl;
   else if (getLangOpts().C99)
-    diag_id = diag::ext_implicit_function_decl;
+    diag_id = diag::ext_implicit_function_decl_c99;
   else
     diag_id = diag::warn_implicit_function_decl;
 
@@ -15331,9 +15338,16 @@ NamedDecl *Sema::ImplicitlyDefineFunction(SourceLocation Loc,
   }
 
   Diag(Loc, diag_id) << &II;
-  if (Corrected)
-    diagnoseTypo(Corrected, PDiag(diag::note_function_suggestion),
-                 /*ErrorRecovery*/ false);
+  if (Corrected) {
+    // If the correction is going to suggest an implicitly defined function,
+    // skip the correction as not being a particularly good idea.
+    bool Diagnose = true;
+    if (const auto *D = Corrected.getCorrectionDecl())
+      Diagnose = !D->isImplicit();
+    if (Diagnose)
+      diagnoseTypo(Corrected, PDiag(diag::note_function_suggestion),
+                   /*ErrorRecovery*/ false);
+  }
 
   // If we found a prior declaration of this function, don't bother building
   // another one. We've already pushed that one into scope, so there's nothing

clang/lib/Sema/SemaExpr.cpp

@@ -2554,8 +2554,9 @@ Sema::ActOnIdExpression(Scope *S, CXXScopeSpec &SS,
     return ExprError();
 
   // This could be an implicitly declared function reference (legal in C90,
-  // extension in C99, forbidden in C++).
-  if (R.empty() && HasTrailingLParen && II && !getLangOpts().CPlusPlus) {
+  // extension in C99, forbidden in C++ and C2x).
+  if (R.empty() && HasTrailingLParen && II && !getLangOpts().CPlusPlus &&
+      !getLangOpts().C2x) {
     NamedDecl *D = ImplicitlyDefineFunction(NameLoc, *II, S);
     if (D) R.addDecl(D);
   }

clang/lib/Sema/SemaLookup.cpp

@@ -930,9 +930,11 @@ bool Sema::LookupBuiltin(LookupResult &R) {
 
       // If this is a builtin on this (or all) targets, create the decl.
       if (unsigned BuiltinID = II->getBuiltinID()) {
-        // In C++ and OpenCL (spec v1.2 s6.9.f), we don't have any predefined
-        // library functions like 'malloc'. Instead, we'll just error.
-        if ((getLangOpts().CPlusPlus || getLangOpts().OpenCL) &&
+        // In C++, C2x, and OpenCL (spec v1.2 s6.9.f), we don't have any
+        // predefined library functions like 'malloc'. Instead, we'll just
+        // error.
+        if ((getLangOpts().CPlusPlus || getLangOpts().OpenCL ||
+             getLangOpts().C2x) &&
             Context.BuiltinInfo.isPredefinedLibFunction(BuiltinID))
           return false;
 

clang/test/ARCMT/objcmt-arc-cf-annotations.m

@@ -80,6 +80,7 @@ static __inline__ __attribute__((always_inline)) CFRange CFRangeMake(CFIndex loc
 extern const CFAllocatorRef kCFAllocatorDefault;
 extern CFTypeRef CFRetain(CFTypeRef cf);
 extern void CFRelease(CFTypeRef cf);
+extern CFTypeRef CFAutorelease(CFTypeRef cf);
 extern CFTypeRef CFMakeCollectable(CFTypeRef cf);
 typedef struct {
 }

clang/test/ARCMT/objcmt-arc-cf-annotations.m.result

@@ -86,6 +86,7 @@ extern void CFRelease(CFTypeRef cf);
 
 CF_IMPLICIT_BRIDGING_DISABLED
 
+extern CFTypeRef CFAutorelease(CFTypeRef cf);
 extern CFTypeRef CFMakeCollectable(CFTypeRef cf);
 typedef struct {
 }

clang/test/Analysis/OSAtomic_mac.c

@@ -1,6 +1,8 @@
 // RUN: %clang_analyze_cc1 -w -analyzer-checker=core,debug.ExprInspection \
 // RUN:                    -analyzer-output=text -verify %s
 
+extern void clang_analyzer_eval(int);
+
 int OSAtomicCompareAndSwapPtrBarrier(void *, void *, void **);
 int OSAtomicCompareAndSwapPtrBarrier(void *, void *, void **) {
   // There is some body in the actual header,

clang/test/Analysis/ObjCProperties.m

@@ -2,6 +2,7 @@
 // RUN:     -analyzer-checker=core,alpha.core,debug.ExprInspection
 
 #ifdef HEADER // A clever trick to avoid splitting up the test.
+extern void clang_analyzer_eval(int);
 
 @interface NSObject
 @end

clang/test/Analysis/PR49642.c

@@ -1,4 +1,4 @@
-// RUN: %clang_analyze_cc1 -w -verify %s \
+// RUN: %clang_analyze_cc1 -Wno-implicit-function-declaration -w -verify %s \
 // RUN:   -analyzer-checker=core \
 // RUN:   -analyzer-checker=apiModeling.StdCLibraryFunctions
 

clang/test/Analysis/dead-stores.c

@@ -12,6 +12,8 @@
 // RUN:   -analyzer-config deadcode.DeadStores:ShowFixIts=true \
 // RUN:   -verify=non-nested,nested
 
+extern int printf(const char *, ...);
+
 void f1(void) {
   int k, y; // non-nested-warning {{unused variable 'k'}}
             // non-nested-warning@-1 {{unused variable 'y'}}
@@ -30,8 +32,6 @@ void f2(void *b) {
   // CHECK-FIXES-NEXT: char *d;
 
   printf("%s", c);
-  // non-nested-warning@-1 {{implicitly declaring library function 'printf' with type 'int (const char *, ...)'}}
-  // non-nested-note@-2 {{include the header <stdio.h> or explicitly provide a declaration for 'printf'}}
 }
 
 int f(void);

clang/test/Analysis/diagnostics/no-store-func-path-notes.c

@@ -1,4 +1,4 @@
-// RUN: %clang_analyze_cc1 -w -x c -analyzer-checker=core -analyzer-output=text\
+// RUN: %clang_analyze_cc1 -w -Wno-implicit-function-declaration -analyzer-checker=core -analyzer-output=text\
 // RUN:     -verify %s
 
 typedef __typeof(sizeof(int)) size_t;

clang/test/Analysis/exercise-ps.c

@@ -1,4 +1,4 @@
-// RUN: %clang_analyze_cc1 %s -verify \
+// RUN: %clang_analyze_cc1 %s -verify -Wno-error=implicit-function-declaration \
 // RUN:   -analyzer-checker=core \
 // RUN:   -analyzer-config core.CallAndMessage:ArgPointeeInitializedness=true
 //
@@ -19,7 +19,7 @@ void_typedef f2_helper(void);
 static void f2(void *buf) {
   F12_typedef* x;
   x = f2_helper();
-  memcpy((&x[1]), (buf), 1); // expected-warning{{implicitly declaring library function 'memcpy' with type 'void *(void *, const void *}} \
+  memcpy((&x[1]), (buf), 1); // expected-warning{{call to undeclared library function 'memcpy' with type 'void *(void *, const void *}} \
   // expected-note{{include the header <string.h> or explicitly provide a declaration for 'memcpy'}}
 }
 

clang/test/Analysis/malloc-three-arg.c

@@ -6,6 +6,7 @@
 #define NULL ((void *)0)
 
 void *malloc(size_t, void *, int);
+void free(void *);
 
 struct test {
 };

clang/test/Analysis/misc-ps-region-store.m

@@ -1,5 +1,5 @@
-// RUN: %clang_analyze_cc1 -triple i386-apple-darwin9 -analyzer-checker=core,alpha.core.CastToStruct,alpha.security.ReturnPtrRange,alpha.security.ArrayBound -analyzer-store=region -verify -fblocks -analyzer-opt-analyze-nested-blocks -Wno-objc-root-class -Wno-strict-prototypes %s
-// RUN: %clang_analyze_cc1 -triple x86_64-apple-darwin9 -DTEST_64 -analyzer-checker=core,alpha.core.CastToStruct,alpha.security.ReturnPtrRange,alpha.security.ArrayBound -analyzer-store=region -verify -fblocks   -analyzer-opt-analyze-nested-blocks -Wno-objc-root-class -Wno-strict-prototypes %s
+// RUN: %clang_analyze_cc1 -triple i386-apple-darwin9 -analyzer-checker=core,alpha.core.CastToStruct,alpha.security.ReturnPtrRange,alpha.security.ArrayBound -analyzer-store=region -verify -fblocks -analyzer-opt-analyze-nested-blocks -Wno-objc-root-class -Wno-strict-prototypes -Wno-error=implicit-function-declaration %s
+// RUN: %clang_analyze_cc1 -triple x86_64-apple-darwin9 -DTEST_64 -analyzer-checker=core,alpha.core.CastToStruct,alpha.security.ReturnPtrRange,alpha.security.ArrayBound -analyzer-store=region -verify -fblocks   -analyzer-opt-analyze-nested-blocks -Wno-objc-root-class -Wno-strict-prototypes -Wno-error=implicit-function-declaration %s
 
 typedef long unsigned int size_t;
 void *memcpy(void *, const void *, size_t);
@@ -1255,7 +1255,7 @@ void Rdar_9103310_E(Rdar_9103310_A * x, struct Rdar_9103310_C * b) { // expected
   int i;
   Rdar_9103310_B_t *y = (Rdar_9103310_B_t *) x;
   for (i = 0; i < 101; i++) {
-    Rdar_9103310_F(b, "%2d%s ", (y->Rdar_9103310_C[i]) / 4, Rdar_9103310_D[(y->Rdar_9103310_C[i]) % 4]); // expected-warning {{implicit declaration of function 'Rdar_9103310_F' is invalid in C99}}
+    Rdar_9103310_F(b, "%2d%s ", (y->Rdar_9103310_C[i]) / 4, Rdar_9103310_D[(y->Rdar_9103310_C[i]) % 4]); // expected-warning {{call to undeclared function 'Rdar_9103310_F'; ISO C99 and later do not support implicit function declarations}}
   }
 }
 

clang/test/Analysis/novoidtypecrash.c

@@ -1,4 +1,4 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=core %s
+// RUN: %clang_analyze_cc1 -std=c89 -analyzer-checker=core %s
 x;
 y(void **z) { // no-crash
   *z = x;

clang/test/Analysis/plist-macros-with-expansion.c

@@ -1,4 +1,4 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=core %s  \
+// RUN: %clang_analyze_cc1 -Wno-error=implicit-function-declaration -analyzer-checker=core %s  \
 // RUN:   -analyzer-output=plist -o %t.plist \
 // RUN:   -analyzer-config expand-macros=true -verify
 //
@@ -8,7 +8,7 @@
 void test_strange_macro_expansion(void) {
   char *path;
   STRANGE_FN(path); // no-crash
-  // expected-warning@-1 {{implicit declaration of function}}
+  // expected-warning@-1 {{call to undeclared function}}
   // expected-warning@-2 {{1st function call argument is an uninitialized value}}
 }
 

clang/test/CodeGen/2002-07-14-MiscTests3.c

@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -w -emit-llvm %s  -o /dev/null
+// RUN: %clang_cc1 -Wno-implicit-function-declaration -w -emit-llvm %s  -o /dev/null
 
 void *malloc(unsigned);
 int puts(const char *s);

clang/test/CodeGen/2002-07-31-SubregFailure.c

@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -emit-llvm %s  -o /dev/null
+// RUN: %clang_cc1 -Wno-implicit-function-declaration -emit-llvm %s  -o /dev/null
 
 
 typedef union {

clang/test/CodeGen/2003-08-18-SigSetJmp.c

@@ -3,6 +3,7 @@
 #define _JBLEN ((9 * 2) + 3 + 16)
 typedef int sigjmp_buf[_JBLEN + 1];
 int sigsetjmp(sigjmp_buf env, int savemask);
+void bar(void);
 sigjmp_buf B;
 int foo(void) {
   sigsetjmp(B, 1);

clang/test/CodeGen/2004-11-27-StaticFunctionRedeclare.c

@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -emit-llvm %s -o - | FileCheck %s
+// RUN: %clang_cc1 -std=c89 -emit-llvm %s -o - | FileCheck %s
 
 // There should not be an unresolved reference to func here.  Believe it or not,
 // the "expected result" is a function named 'func' which is internal and

clang/test/CodeGen/2005-01-02-ConstantInits.c

@@ -5,6 +5,7 @@
 // array subscripts.  This corresponds to PR487.
 
 struct X { int a[2]; };
+extern int bar();
 
 //.
 // CHECK: @test.i23 = internal global i32 4, align 4

clang/test/CodeGen/2005-01-02-VAArgError-ICE.c

@@ -1,6 +1,6 @@
 // This file is erroneous, but should not cause the compiler to ICE.
 // PR481
-// RUN: %clang_cc1 %s -emit-llvm -o /dev/null
+// RUN: %clang_cc1 %s -Wno-implicit-function-declaration -emit-llvm -o /dev/null
 
 int flags(int a, int b, ...) {
         __builtin_va_list         args;

clang/test/CodeGen/2005-02-20-AggregateSAVEEXPR.c

@@ -1,5 +1,7 @@
 // RUN: %clang_cc1 %s -o /dev/null -emit-llvm
 
+double creal(_Complex double);
+
 int foo(__complex float c) {
     return creal(c);
 }

clang/test/CodeGen/2006-01-13-StackSave.c

@@ -2,6 +2,8 @@
 // RUN: %clang_cc1 -no-opaque-pointers %s -emit-llvm -o - | FileCheck %s
 // CHECK: call i8* @llvm.stacksave()
 
+extern void external(int[*]);
+
 void test(int N) {
   int i;
   for (i = 0; i < N; ++i) {

clang/test/CodeGen/2006-03-03-MissingInitializer.c

@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 %s -emit-llvm -o - | FileCheck %s
+// RUN: %clang_cc1 %s -emit-llvm -std=c89 -o - | FileCheck %s
 
 struct X { int *XX; int Y;};