[PATCH] D125272: [clang] Add -fcheck-new support

Tue Apr 25 15:24:56 PDT 2023

MaskRay added inline comments.

================
Comment at: clang/lib/Frontend/CompilerInvocation.cpp:4117
+  if (const Arg *A = Args.getLastArg(OPT_fcheck_new))
+    Opts.CheckNew = true;
+
----------------
heatd wrote:
> MaskRay wrote:
> > Use `CodeGenOpts<"CheckNew">` and avoid change to this file. CodeGenOpts seems more suitable than LangOpts.
> Sorry, I haven't looked at this rev in a good while due to $REASONS. Could you elaborate on this? I'm not opposed to moving it to CodeGenOpts per se, but I would like to understand your reasoning. It seems to me that fcheck-new is more of a language change (since it removes the implicit [[nonnull]] on the result of operator new) than a codegen change, although it does have its effects on codegen, naturally. A quick look at CodeGenOpts in Options.td would hint to me that these mostly/only affect codegen itself. What do you think? 
OK. LangOpts looks good since it affects Sema and in GCC this option is a C++ dialect option (https://gcc.gnu.org/onlinedocs/gcc/C_002b_002b-Dialect-Options.html).

Can you use `MarshallingInfoFlag<LangOpts` and remove the change to `CompilerInvocation.cpp`? You can find other `MarshallingInfoFlag<LangOpts` examples in Options.td

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125272