erichkeane 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; }
+
----------------
cor3ntin wrote:
> 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?
>
*shrug*, I tend to be of the feeling that if you have to say something this
often, and functions are basically free, mind as well make one.
================
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;
----------------
cor3ntin wrote:
> 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
Yep, folks asking for C89 ARE peculiar :D However, warnings-not-as-error are,
IMO, simply 'helpful'.
================
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,
----------------
cor3ntin wrote:
> Nitpick: whitespace only change
This might be a clang-format thing based on the previous line.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D124258/new/
https://reviews.llvm.org/D124258
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
