sammccall accepted this revision. sammccall added a comment. This revision is now accepted and ready to land.
Code LG, let's chat before landing though - want to understand the state of the internal testing. ================ Comment at: clang/include/clang/Basic/LangOptions.def:151 -COMPATIBLE_LANGOPT(RecoveryAST, 1, 0, "Preserve expressions in AST when encountering errors") +COMPATIBLE_LANGOPT(RecoveryAST, 1, CPlusPlus, "Preserve expressions in AST when encountering errors") ---------------- hokein wrote: > rsmith wrote: > > Does this work? I would expect that we set all the options to the defaults > > at the same time, so this just sets this option to 0 (the default for > > `CPlusPlus`). If so, it'd be clearer to explicitly write that default here. > I'm following the scheme of other fields, e.g. `WChar`, the real > initialization is done in `CompilerInvocation.cpp`. > > I think writing `CPlusPlus` is a bit clearer here, which indicates this > option is associated with CPlusPlus flag. I think I suggested this in the past (based on precedent) but agree with Richard it's probably more confusing than enlightening. ================ Comment at: clang/lib/Frontend/CompilerInvocation.cpp:2892 Diags.Report(diag::warn_fe_concepts_ts_flag); Opts.RecoveryAST = + Args.hasFlag(OPT_frecovery_ast, OPT_fno_recovery_ast, Opts.CPlusPlus); ---------------- you could add a comment here like "Recovery AST still heavily relies on dependent-type machinery" or something. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78350/new/ https://reviews.llvm.org/D78350 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits