aaron.ballman added inline comments.
================ Comment at: clang/include/clang/Basic/DiagnosticFrontendKinds.td:50-53 +def warn_eval_method_setting_via_option_in_value_unsafe_context : Warning< + "setting the eval method via '-ffp-eval-method' has not effect when numeric " + "results of floating-point calculations aren't value-safe.">, + InGroup<IncompatibleFPOpts>; ---------------- zahiraam wrote: > aaron.ballman wrote: > > andrew.w.kaylor wrote: > > > zahiraam wrote: > > > > aaron.ballman wrote: > > > > > Unless you have a strong reason for this to be a warning, this seems > > > > > like a situation we should diagnose as an error with a much clearer > > > > > message. > > > > May be @andrew.w.kaylor would weigh in on this? > > > I was going to say that for the command line option we could just issue a > > > warning saying that the later option overrides the earlier, but it's a > > > bit complicated to sort out what that would mean if the eval method > > > follows a fast-math option and it might not always be what the user > > > intended. So, I guess I'd agree that it should be an error. > > > > > > For the case with pragmas, the model I'd follow is the mixing of #pragma > > > float_control(except, on) with a fast-math mode or #pragma > > > float_control(precise, off) with a non-ignore exception mode. In both > > > those cases we issue an error. > > > For the case with pragmas, the model I'd follow is the mixing of #pragma > > > float_control(except, on) with a fast-math mode or #pragma > > > float_control(precise, off) with a non-ignore exception mode. In both > > > those cases we issue an error. > > > > Good catch, I think that's a good approach as well. > I think i will have the issue with the order of appearance of the options on > the command line. > // RUN: -freciprocal-math -mreassociate -ffp-eval-method=source > and > // RUN: -mreassociate -ffp-eval-method=source > > will depend on which order I will test for > LangOpts.ApproxFunc/AllowFPReasson/AllowRecip being used or not? > > The run lines above might give the same diagnostic. Unless I do something > really complicated to check the order of the options on the command line? > I think i will have the issue with the order of appearance of the options on > the command line. You shouldn't -- you should be able to test the language options after the command line was fully parsed. See `FixupInvocation()` in `CompilerInvocation.cpp`. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122155/new/ https://reviews.llvm.org/D122155 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
