Change the behavior of implicit int diagnostics

C89 allowed a type specifier to be elided with the resulting type being
int, aka implicit int behavior. This feature was subsequently removed
in C99 without a deprecation period, so implementations continued to
support the feature. Now, as with implicit function declarations, is a
good time to reevaluate the need for this support.

This patch allows -Wimplicit-int to issue warnings in C89 mode (off by
default), defaults the warning to an error in C99 through C17, and
disables support for the feature entirely in C2x. It also removes a
warning about missing declaration specifiers that really was just an
implicit int warning in disguise and other minor related cleanups.

Author: AaronBallman

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

@@ -1492,8 +1492,8 @@ TEST(DiagsInHeaders, DiagInsideHeader) {
   TU.AdditionalFiles = {{"a.h", std::string(Header.code())}};
   EXPECT_THAT(*TU.build().getDiagnostics(),
               UnorderedElementsAre(AllOf(
-                  Diag(Main.range(), "in included file: C++ requires a "
-                                     "type specifier for all declarations"),
+                  Diag(Main.range(), "in included file: a type specifier is "
+                                     "required for all declarations"),
                   withNote(Diag(Header.range(), "error occurred here")))));
 }
 
@@ -1506,8 +1506,8 @@ TEST(DiagsInHeaders, DiagInTransitiveInclude) {
                         {"b.h", "no_type_spec; // error-ok"}};
   EXPECT_THAT(*TU.build().getDiagnostics(),
               UnorderedElementsAre(
-                  Diag(Main.range(), "in included file: C++ requires a "
-                                     "type specifier for all declarations")));
+                  Diag(Main.range(), "in included file: a type specifier is "
+                                     "required for all declarations")));
 }
 
 TEST(DiagsInHeaders, DiagInMultipleHeaders) {
@@ -1520,10 +1520,10 @@ TEST(DiagsInHeaders, DiagInMultipleHeaders) {
                         {"b.h", "no_type_spec; // error-ok"}};
   EXPECT_THAT(*TU.build().getDiagnostics(),
               UnorderedElementsAre(
-                  Diag(Main.range("a"), "in included file: C++ requires a type "
-                                        "specifier for all declarations"),
-                  Diag(Main.range("b"), "in included file: C++ requires a type "
-                                        "specifier for all declarations")));
+                  Diag(Main.range("a"), "in included file: a type specifier is "
+                                        "required for all declarations"),
+                  Diag(Main.range("b"), "in included file: a type specifier is "
+                                        "required for all declarations")));
 }
 
 TEST(DiagsInHeaders, PreferExpansionLocation) {
@@ -1537,8 +1537,8 @@ TEST(DiagsInHeaders, PreferExpansionLocation) {
       {"b.h", "#ifndef X\n#define X\nno_type_spec; // error-ok\n#endif"}};
   EXPECT_THAT(*TU.build().getDiagnostics(),
               UnorderedElementsAre(Diag(Main.range(),
-                                        "in included file: C++ requires a type "
-                                        "specifier for all declarations")));
+                                        "in included file: a type specifier is "
+                                        "required for all declarations")));
 }
 
 TEST(DiagsInHeaders, PreferExpansionLocationMacros) {
@@ -1555,8 +1555,8 @@ TEST(DiagsInHeaders, PreferExpansionLocationMacros) {
       {"c.h", "#ifndef X\n#define X\nno_type_spec; // error-ok\n#endif"}};
   EXPECT_THAT(*TU.build().getDiagnostics(),
               UnorderedElementsAre(
-                  Diag(Main.range(), "in included file: C++ requires a "
-                                     "type specifier for all declarations")));
+                  Diag(Main.range(), "in included file: a type specifier is "
+                                     "required for all declarations")));
 }
 
 TEST(DiagsInHeaders, LimitDiagsOutsideMainFile) {
@@ -1584,8 +1584,8 @@ TEST(DiagsInHeaders, LimitDiagsOutsideMainFile) {
       #endif)cpp"}};
   EXPECT_THAT(*TU.build().getDiagnostics(),
               UnorderedElementsAre(
-                  Diag(Main.range(), "in included file: C++ requires a "
-                                     "type specifier for all declarations")));
+                  Diag(Main.range(), "in included file: a type specifier is "
+                                     "required for all declarations")));
 }
 
 TEST(DiagsInHeaders, OnlyErrorOrFatal) {
@@ -1599,8 +1599,8 @@ TEST(DiagsInHeaders, OnlyErrorOrFatal) {
   TU.AdditionalFiles = {{"a.h", std::string(Header.code())}};
   EXPECT_THAT(*TU.build().getDiagnostics(),
               UnorderedElementsAre(AllOf(
-                  Diag(Main.range(), "in included file: C++ requires "
-                                     "a type specifier for all declarations"),
+                  Diag(Main.range(), "in included file: a type specifier is "
+                                     "required for all declarations"),
                   withNote(Diag(Header.range(), "error occurred here")))));
 }
 

clang-tools-extra/test/clang-tidy/checkers/altera-struct-pack-align-invalid-decl-no-crash.cpp

@@ -3,4 +3,4 @@
 struct Foo {
   member; // no-crash
 };
-// CHECK-MESSAGES: :[[@LINE-2]]:3: error: C++ requires a type specifier for all declarations [clang-diagnostic-error]
+// CHECK-MESSAGES: :[[@LINE-2]]:3: error: a type specifier is required for all declarations [clang-diagnostic-error]

clang-tools-extra/test/clang-tidy/infrastructure/export-diagnostics.cpp

@@ -20,7 +20,7 @@ struct Foo {
 // CHECK-MESSAGES: -input.cpp:3:7: error: 'a' declared as an array with a negative size [clang-diagnostic-error]
 // CHECK-MESSAGES: -input.cpp:4:7: warning: zero size arrays are an extension [clang-diagnostic-zero-length-array]
 // CHECK-MESSAGES: -input.cpp:6:11: error: unknown type name 'x' [clang-diagnostic-error]
-// CHECK-MESSAGES: -input.cpp:8:3: error: C++ requires a type specifier for all declarations [clang-diagnostic-error]
+// CHECK-MESSAGES: -input.cpp:8:3: error: a type specifier is required for all declarations [clang-diagnostic-error]
 
 // CHECK-YAML: ---
 // CHECK-YAML-NEXT: MainSourceFile:  '{{.*}}-input.cpp'
@@ -88,7 +88,7 @@ struct Foo {
 // CHECK-YAML-NEXT:     BuildDirectory:  '{{.*}}'
 // CHECK-YAML-NEXT:   - DiagnosticName:  clang-diagnostic-error
 // CHECK-YAML-NEXT:     DiagnosticMessage:
-// CHECK-YAML-NEXT:       Message:         'C++ requires a type specifier for all declarations'
+// CHECK-YAML-NEXT:       Message:         a type specifier is required for all declarations
 // CHECK-YAML-NEXT:       FilePath:        '{{.*}}-input.cpp'
 // CHECK-YAML-NEXT:       FileOffset:      86
 // CHECK-YAML-NEXT:       Replacements:    []

clang/docs/ReleaseNotes.rst

@@ -170,7 +170,17 @@ Improvements to Clang's diagnostics
   ``-Wno-implicit-function-declaration``. As of C2x, support for implicit
   function declarations has been removed, and the warning options will have no
   effect.
-
+- The ``-Wimplicit-int`` 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-int``, or disabled entirely with ``-Wno-implicit-int``.
+  As of C2x, support for implicit int has been removed, and the warning options
+  will have no effect. Specifying ``-Wimplicit-int`` in C89 mode will now issue
+  warnings instead of being a noop.
+- No longer issue a "declaration specifiers missing, defaulting to int"
+  diagnostic in C89 mode because it is not an extension in C89, it was valid
+  code. The diagnostic has been removed entirely as it did not have a
+  diagnostic group to disable it, but it can be covered wholly by
+  ``-Wimplicit-int``.
 - ``-Wmisexpect`` warns when the branch weights collected during profiling
   conflict with those added by ``llvm.expect``.
 

clang/include/clang/Basic/DiagnosticCommonKinds.td

@@ -123,7 +123,9 @@ def err_enum_template : Error<"enumeration cannot be a template">;
 def warn_cxx20_compat_consteval : Warning<
   "'consteval' specifier is incompatible with C++ standards before C++20">,
   InGroup<CXX20Compat>, DefaultIgnore;
-
+def warn_missing_type_specifier : Warning<
+  "type specifier missing, defaults to 'int'">,
+  InGroup<ImplicitInt>, DefaultIgnore;
 }
 
 let CategoryName = "Nullability Issue" in {

clang/include/clang/Basic/DiagnosticSemaKinds.td

@@ -4341,8 +4341,9 @@ def err_void_param_qualified : Error<
   "'void' as parameter must not have type qualifiers">;
 def err_ident_list_in_fn_declaration : Error<
   "a parameter list without types is only allowed in a function definition">;
-def ext_param_not_declared : Extension<
-  "parameter %0 was not declared, defaulting to type 'int'">;
+def ext_param_not_declared : ExtWarn<
+  "parameter %0 was not declared, defaults to 'int'; ISO C99 and later do not "
+  "support implicit int">, InGroup<ImplicitInt>, DefaultError;
 def err_param_default_argument : Error<
   "C does not support default arguments">;
 def err_param_default_argument_redefinition : Error<
@@ -9997,15 +9998,13 @@ def warn_receiver_forward_class : Warning<
   "receiver %0 is a forward class and corresponding @interface may not exist">,
   InGroup<ForwardClassReceiver>;
 def note_method_sent_forward_class : Note<"method %0 is used for the forward class">;
-def ext_missing_declspec : ExtWarn<
-  "declaration specifier missing, defaulting to 'int'">;
 def ext_missing_type_specifier : ExtWarn<
-  "type specifier missing, defaults to 'int'">,
-  InGroup<ImplicitInt>;
+  "type specifier missing, defaults to 'int'; ISO C99 and later do not support "
+  "implicit int">, InGroup<ImplicitInt>, DefaultError;
+def err_missing_type_specifier : Error<
+  "a type specifier is required for all declarations">;
 def err_decimal_unsupported : Error<
   "GNU decimal type extension not supported">;
-def err_missing_type_specifier : Error<
-  "C++ requires a type specifier for all declarations">;
 def err_objc_array_of_interfaces : Error<
   "array of interface %0 is invalid (probably should be an array of pointers)">;
 def ext_c99_array_usage : Extension<

clang/include/clang/Basic/LangOptions.def

@@ -120,7 +120,6 @@ BENIGN_LANGOPT(AsmPreprocessor, 1, 0, "preprocessor in asm mode")
 LANGOPT(GNUMode           , 1, 1, "GNU extensions")
 LANGOPT(GNUKeywords       , 1, 1, "GNU keywords")
 VALUE_LANGOPT(GNUCVersion , 32, 0, "GNU C compatibility version")
-BENIGN_LANGOPT(ImplicitInt, 1, 0, "C89 implicit 'int'")
 LANGOPT(DisableKNRFunctions, 1, 0, "require function types to have a prototype")
 LANGOPT(Digraphs          , 1, 0, "digraphs")
 BENIGN_LANGOPT(HexFloats  , 1, 0, "C99 hexadecimal float constants")

clang/include/clang/Basic/LangOptions.h

@@ -537,6 +537,12 @@ class LangOptions : public LangOptionsBase {
     return !requiresStrictPrototypes() && !OpenCL;
   }
 
+  /// Returns true if implicit int is part of the language requirements.
+  bool isImplicitIntRequired() const { return !CPlusPlus && !C99; }
+
+  /// Returns true if implicit int is supported at all.
+  bool isImplicitIntAllowed() const { return !CPlusPlus && !C2x; }
+
   /// Check if return address signing is enabled.
   bool hasSignReturnAddress() const {
     return getSignReturnAddressScope() != SignReturnAddressScopeKind::None;

clang/include/clang/Basic/LangStandard.h

@@ -59,9 +59,8 @@ enum LangFeatures {
   Digraphs = (1 << 11),
   GNUMode = (1 << 12),
   HexFloat = (1 << 13),
-  ImplicitInt = (1 << 14),
-  OpenCL = (1 << 15),
-  HLSL = (1 << 16)
+  OpenCL = (1 << 14),
+  HLSL = (1 << 15)
 };
 
 /// LangStandard - Information about the properties of a particular language
@@ -131,9 +130,6 @@ struct LangStandard {
   /// hasHexFloats - Language supports hexadecimal float constants.
   bool hasHexFloats() const { return Flags & HexFloat; }
 
-  /// hasImplicitInt - Language allows variables to be typed as int implicitly.
-  bool hasImplicitInt() const { return Flags & ImplicitInt; }
-
   /// isOpenCL - Language is a OpenCL variant.
   bool isOpenCL() const { return Flags & OpenCL; }
 

clang/include/clang/Basic/LangStandards.def

@@ -36,18 +36,17 @@
 
 // C89-ish modes.
 LANGSTANDARD(c89, "c89",
-             C, "ISO C 1990",
-             ImplicitInt)
+             C, "ISO C 1990", 0)
 LANGSTANDARD_ALIAS(c89, "c90")
 LANGSTANDARD_ALIAS(c89, "iso9899:1990")
 
 LANGSTANDARD(c94, "iso9899:199409",
              C, "ISO C 1990 with amendment 1",
-             Digraphs | ImplicitInt)
+             Digraphs)
 
 LANGSTANDARD(gnu89, "gnu89",
              C, "ISO C 1990 with GNU extensions",
-             LineComment | Digraphs | GNUMode | ImplicitInt)
+             LineComment | Digraphs | GNUMode)
 LANGSTANDARD_ALIAS(gnu89, "gnu90")
 
 // C99-ish modes

clang/lib/Basic/LangOptions.cpp

@@ -113,7 +113,6 @@ void LangOptions::setLangDefaults(LangOptions &Opts, Language Lang,
   Opts.GNUMode = Std.isGNUMode();
   Opts.GNUCVersion = 0;
   Opts.HexFloats = Std.hasHexFloats();
-  Opts.ImplicitInt = Std.hasImplicitInt();
   Opts.WChar = Std.isCPlusPlus();
   Opts.Digraphs = Std.hasDigraphs();
 

clang/lib/Parse/ParseDecl.cpp

@@ -2637,8 +2637,8 @@ bool Parser::ParseImplicitInt(DeclSpec &DS, CXXScopeSpec *SS,
   // error, do lookahead to try to do better recovery. This never applies
   // within a type specifier. Outside of C++, we allow this even if the
   // language doesn't "officially" support implicit int -- we support
-  // implicit int as an extension in C99 and C11.
-  if (!isTypeSpecifier(DSC) && !getLangOpts().CPlusPlus &&
+  // implicit int as an extension in some language modes.
+  if (!isTypeSpecifier(DSC) && getLangOpts().isImplicitIntAllowed() &&
       isValidAfterIdentifierInDeclarator(NextToken())) {
     // If this token is valid for implicit int, e.g. "static x = 4", then
     // we just avoid eating the identifier, so it will be parsed as the

clang/lib/Parse/Parser.cpp

@@ -1195,10 +1195,12 @@ Decl *Parser::ParseFunctionDefinition(ParsingDeclarator &D,
   const DeclaratorChunk::FunctionTypeInfo &FTI = D.getFunctionTypeInfo();
   TemplateParameterDepthRAII CurTemplateDepthTracker(TemplateParameterDepth);
 
-  // If this is C90 and the declspecs were completely missing, fudge in an
+  // If this is C89 and the declspecs were completely missing, fudge in an
   // implicit int.  We do this here because this is the only place where
   // declaration-specifiers are completely optional in the grammar.
-  if (getLangOpts().ImplicitInt && D.getDeclSpec().isEmpty()) {
+  if (getLangOpts().isImplicitIntRequired() && D.getDeclSpec().isEmpty()) {
+    Diag(D.getIdentifierLoc(), diag::warn_missing_type_specifier)
+        << D.getDeclSpec().getSourceRange();
     const char *PrevSpec;
     unsigned DiagID;
     const PrintingPolicy &Policy = Actions.getASTContext().getPrintingPolicy();

clang/lib/Sema/SemaDecl.cpp

@@ -14317,18 +14317,28 @@ void Sema::ActOnFinishKNRParamDeclarations(Scope *S, Declarator &D,
                                            SourceLocation LocAfterDecls) {
   DeclaratorChunk::FunctionTypeInfo &FTI = D.getFunctionTypeInfo();
 
-  // Verify 6.9.1p6: 'every identifier in the identifier list shall be declared'
-  // for a K&R function.
+  // C99 6.9.1p6 "If a declarator includes an identifier list, each declaration
+  // in the declaration list shall have at least one declarator, those
+  // declarators shall only declare identifiers from the identifier list, and
+  // every identifier in the identifier list shall be declared.
+  //
+  // C89 3.7.1p5 "If a declarator includes an identifier list, only the
+  // identifiers it names shall be declared in the declaration list."
+  //
+  // This is why we only diagnose in C99 and later. Note, the other conditions
+  // listed are checked elsewhere.
   if (!FTI.hasPrototype) {
     for (int i = FTI.NumParams; i != 0; /* decrement in loop */) {
       --i;
       if (FTI.Params[i].Param == nullptr) {
-        SmallString<256> Code;
-        llvm::raw_svector_ostream(Code)
-            << "  int " << FTI.Params[i].Ident->getName() << ";\n";
-        Diag(FTI.Params[i].IdentLoc, diag::ext_param_not_declared)
-            << FTI.Params[i].Ident
-            << FixItHint::CreateInsertion(LocAfterDecls, Code);
+        if (getLangOpts().C99) {
+          SmallString<256> Code;
+          llvm::raw_svector_ostream(Code)
+              << "  int " << FTI.Params[i].Ident->getName() << ";\n";
+          Diag(FTI.Params[i].IdentLoc, diag::ext_param_not_declared)
+              << FTI.Params[i].Ident
+              << FixItHint::CreateInsertion(LocAfterDecls, Code);
+        }
 
         // Implicitly declare the argument as type 'int' for lack of a better
         // type.

clang/lib/Sema/SemaType.cpp

@@ -1343,35 +1343,34 @@ static QualType ConvertDeclSpecToType(TypeProcessingState &state) {
     // allowed to be completely missing a declspec.  This is handled in the
     // parser already though by it pretending to have seen an 'int' in this
     // case.
-    if (S.getLangOpts().ImplicitInt) {
-      // In C89 mode, we only warn if there is a completely missing declspec
-      // when one is not allowed.
-      if (DS.isEmpty()) {
-        S.Diag(DeclLoc, diag::ext_missing_declspec)
-            << DS.getSourceRange()
-            << FixItHint::CreateInsertion(DS.getBeginLoc(), "int");
-      }
+    if (S.getLangOpts().isImplicitIntRequired()) {
+      S.Diag(DeclLoc, diag::warn_missing_type_specifier)
+          << DS.getSourceRange()
+          << FixItHint::CreateInsertion(DS.getBeginLoc(), "int");
     } else if (!DS.hasTypeSpecifier()) {
       // C99 and C++ require a type specifier.  For example, C99 6.7.2p2 says:
       // "At least one type specifier shall be given in the declaration
       // specifiers in each declaration, and in the specifier-qualifier list in
       // each struct declaration and type name."
-      if (S.getLangOpts().CPlusPlus && !DS.isTypeSpecPipe()) {
+      if (!S.getLangOpts().isImplicitIntAllowed() && !DS.isTypeSpecPipe()) {
         S.Diag(DeclLoc, diag::err_missing_type_specifier)
-          << DS.getSourceRange();
+            << DS.getSourceRange();
 
-        // When this occurs in C++ code, often something is very broken with the
-        // value being declared, poison it as invalid so we don't get chains of
+        // When this occurs, often something is very broken with the value
+        // being declared, poison it as invalid so we don't get chains of
         // errors.
         declarator.setInvalidType(true);
       } else if (S.getLangOpts().getOpenCLCompatibleVersion() >= 200 &&
                  DS.isTypeSpecPipe()) {
         S.Diag(DeclLoc, diag::err_missing_actual_pipe_type)
-          << DS.getSourceRange();
+            << DS.getSourceRange();
         declarator.setInvalidType(true);
       } else {
+        assert(S.getLangOpts().isImplicitIntAllowed() &&
+               "implicit int is disabled?");
         S.Diag(DeclLoc, diag::ext_missing_type_specifier)
-          << DS.getSourceRange();
+            << DS.getSourceRange()
+            << FixItHint::CreateInsertion(DS.getBeginLoc(), "int");
       }
     }
 

clang/test/Analysis/PR49642.c

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

clang/test/Analysis/inline-unique-reports.c

@@ -1,4 +1,4 @@
-// RUN: %clang_analyze_cc1 %s -analyzer-checker=core.NullDereference -analyzer-output=plist -o %t > /dev/null 2>&1
+// RUN: %clang_analyze_cc1 %s -analyzer-checker=core.NullDereference -analyzer-output=plist -Wno-error=implicit-int -o %t > /dev/null 2>&1
 // RUN: %normalize_plist <%t | diff -ub %S/Inputs/expected-plists/inline-unique-reports.c.plist -
 
 static inline bug(int *p) {

clang/test/Analysis/malloc.c

@@ -1,4 +1,4 @@
-// RUN: %clang_analyze_cc1 -Wno-strict-prototypes -analyzer-store=region -verify %s \
+// RUN: %clang_analyze_cc1 -Wno-strict-prototypes -Wno-error=implicit-int -analyzer-store=region -verify %s \
 // RUN:   -analyzer-checker=core \
 // RUN:   -analyzer-checker=alpha.deadcode.UnreachableCode \
 // RUN:   -analyzer-checker=alpha.core.CastSize \

clang/test/CXX/dcl.dcl/dcl.spec/dcl.type/dcl.spec.auto/p2.cpp

@@ -7,7 +7,7 @@ struct S {
 
   // Note, this is not permitted: conversion-declarator cannot have a trailing return type.
   // FIXME: don't issue the second diagnostic for this.
-  operator auto(*)()->int(); // expected-error{{'auto' not allowed in conversion function type}} expected-error {{C++ requires a type specifier}}
+  operator auto(*)()->int(); // expected-error{{'auto' not allowed in conversion function type}} expected-error {{a type specifier is required}}
 };
 
 typedef auto Fun(int a) -> decltype(a + a);

clang/test/CXX/drs/dr2xx.cpp

@@ -1094,7 +1094,7 @@ namespace dr298 { // dr298: yes
   struct B b; // expected-error {{typedef 'B' cannot be referenced with a struct specifier}}
   struct C c; // expected-error {{typedef 'C' cannot be referenced with a struct specifier}}
 
-  B::B() {} // expected-error {{requires a type specifier}}
+  B::B() {} // expected-error {{a type specifier is required}}
   B::A() {} // ok
   C::~C() {} // expected-error {{destructor cannot be declared using a typedef 'dr298::C' (aka 'const dr298::A') of the class name}}
 

clang/test/CXX/drs/dr4xx.cpp

@@ -1054,7 +1054,7 @@ namespace dr484 { // dr484: yes
   struct N::DT {}; // expected-error {{conflicts with typedef}}
 
   typedef struct {
-    S(); // expected-error {{requires a type}}
+    S(); // expected-error {{a type specifier is required}}
   } S;
 }