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>;
----------------
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.
================
Comment at: clang/include/clang/Basic/DiagnosticGroups.td:127
def UnsupportedFPOpt : DiagGroup<"unsupported-floating-point-opt">;
+def IncompatibleFPOpts : DiagGroup<"incompatible-floating-point-opts">;
def UnsupportedCB : DiagGroup<"unsupported-cb">;
----------------
This change won't be needed any longer.
================
Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:6479-6481
+ "setting the eval method via the `pragma clang fp eval_method` "
+ "has no effect when numeric results of floating-point calculations aren't "
+ "value-safe.">,
----------------
Similar wording suggestion here as above -- this makes it more clear what the
issue is and how the user can fix it. You'll have to figure out which pragmas
and which command line options set the language options you're checking for the
predicate. But the goal is to let users know where the conflict is.
Bonus points if you can add a note to point to the previous pragma that caused
the conflict with `clang fp eval_method` so the user doesn't have to hunt for
it. However, I don't insist on that as it may be difficult to track. But if it
isn't difficult, it would allow us to improve the `%select` slightly to
something like: `"'#pragma clang fp eval_method' cannot be used with
%select{'#pragma clang fp reassociate'|'-mreassociate'|'#pragma
whatever'|'-fwhatever'|..}0">,`
================
Comment at: clang/lib/Sema/SemaAttr.cpp:492
+ Diag(Loc,
+ diag::warn_eval_method_setting_via_pragma_in_value_unsafe_context);
FpPragmaStack.Act(Loc, PSK_Set, StringRef(), NewFPFeatures);
----------------
zahiraam wrote:
> Not sure if I should repeat the comment here (same one than in
> CompilerInvocation.cpp?).
Er, given that they're pretty involved comments, I'd only put the comments in
one place, and then have the other one say "See the comments in <some
reasonable identifying location> for more information about why this is
diagnosed." or something to that effect.
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
