mibintc added a comment. some inline comments for reviewers
================ Comment at: clang/include/clang/Basic/TargetInfo.h:1162 /// the language based on the target options where applicable. - virtual void adjust(LangOptions &Opts); + virtual void adjust(DiagnosticsEngine &Diags, LangOptions &Opts); ---------------- There's no good place to diagnose when LangOptions and TargetOptions conflict, I see "conflict" diagnostics being emitted in clang/CodeGen when creating the func or module, which seems wrong to me. On the other hand, the "adjust" function makes a good place to do the reconciliation I think. This is the change that could be pulled out in a refactoring and committed prior to this patch. What do you think? ================ Comment at: clang/include/clang/Driver/Options.td:4300 defm pack_derived : BooleanFFlag<"pack-derived">, Group<gfortran_Group>; -defm protect_parens : BooleanFFlag<"protect-parens">, Group<gfortran_Group>; +//defm protect_parens : BooleanFFlag<"protect-parens">, Group<gfortran_Group>; defm range_check : BooleanFFlag<"range-check">, Group<gfortran_Group>; ---------------- @jansvoboda11 This is a gfortran option, but I don't think there's a way to allow the same option spelling for gfortran and clang. Other clang options, like -short-enum, also have been pulled out of gfortran group, and the gfortran option test is an expected-fail test. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D100118/new/ https://reviews.llvm.org/D100118 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
