[PATCH] D124258: [C89/C2x] Change the behavior of implicit int diagnostics

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin added inline comments.


================
Comment at: clang/include/clang/Basic/LangOptions.h:534
+  /// Returns true if implicit int is supported at all.
+  bool implicitIntEnabled() const { return !CPlusPlus && !C2x; }
+
----------------
erichkeane wrote:
> This name seems inverse of what you mean to me?  IDK if you're bike-shed-sniping me here, but:
> 
> `prohibitsImplicitInt` to be the reverse of above? It becomes SLIGHTLY ambiguous to me (in that we don't mean "the language standard prohibits", we mean "the compiler implementation prohibits"), but that can be fixed up with a comment.
> 
> If you want to keep the direction, perhaps `implicitIntPermitted`, at which point I might suggest the one above become `implicitIntRequired`.
@erichkeane `requiresImplicitInt` is only used twice. Does it needs a name?



================
Comment at: clang/lib/Sema/SemaDecl.cpp:14329
       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;
----------------
erichkeane wrote:
> IMO there should be a warning here for C89.  Warning for non-future-proof code is pretty typical.
Isn't the counter argument that was made on similar changes that people specifically asking for C89 have peculiar expectations?
Otherwise i agree


================
Comment at: clang/lib/Sema/SemaDecl.cpp:14342-14343
         DeclSpec DS(attrs);
-        const char* PrevSpec; // unused
-        unsigned DiagID; // unused
+        const char *PrevSpec; // unused
+        unsigned DiagID;      // unused
         DS.SetTypeSpecType(DeclSpec::TST_int, FTI.Params[i].IdentLoc, PrevSpec,
----------------
Nitpick: whitespace only change


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D124258/new/

https://reviews.llvm.org/D124258