nickdesaulniers added a comment.
Thanks for the patch @ziangwan !
================
Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:3245
+def warn_impcast_integer_float_precision : Warning<
+ "implicit conversion from %0 to %1 may loses integer precision">,
+ InGroup<ImplicitFloatConversion>, DefaultIgnore;
----------------
`may lose` or just `loses`.
================
Comment at: clang/lib/Sema/SemaChecking.cpp:11404
+ // If we are casting an integer type to a floating point type, we might
+ // lose accuracy if the floating point type has a narrower signicand
+ // than the floating point type. Issue warnings for that accuracy loss.
----------------
`significand`
================
Comment at: clang/lib/Sema/SemaChecking.cpp:11412
+
+ // Determine the number of precision bits in the target floating point type
+ // Also create the APFloat object to represent the converted value.
----------------
End the sentence w/ punctuation.
================
Comment at: clang/lib/Sema/SemaChecking.cpp:11417
+ llvm::APFloat SourceToFloatValue(
+ S.Context.getFloatTypeSemantics(QualType(TargetBT, 0)));
+
----------------
should this use `SourceBT` rather than `TargetBT`?
================
Comment at: clang/lib/Sema/SemaChecking.cpp:11420
+ // If we manage to find out the precisions for both types, perform
+ // acccuracy loss check and issue warning if necessary.
+ if (SourcePrecision > 0 && TargetPrecision > 0 &&
----------------
`accuracy`
`:set spell` if you're using vim
================
Comment at: clang/lib/Sema/SemaChecking.cpp:11421
+ // acccuracy loss check and issue warning if necessary.
+ if (SourcePrecision > 0 && TargetPrecision > 0 &&
+ SourcePrecision > TargetPrecision) {
----------------
Maybe negate this conditional and return early?
================
Comment at: clang/lib/Sema/SemaChecking.cpp:11437
+ // Issue constant integer to float precision loss warning.
+ S.DiagRuntimeBehavior(
+ E->getExprLoc(), E,
----------------
Should this just be a generic `Diag` rather than a `DiagRuntimeBehavior`?
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D64666/new/
https://reviews.llvm.org/D64666
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
