[PATCH] D50467: [SEMA] add more -Wfloat-conversion to compound assigment analysis
nickdesaulniers added a comment. Thank you for the code review. Repository: rL LLVM https://reviews.llvm.org/D50467 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50467: [SEMA] add more -Wfloat-conversion to compound assigment analysis
This revision was automatically updated to reflect the committed changes. Closed by commit rL339581: [SEMA] add more -Wfloat-conversion to compound assigment analysis (authored by nickdesaulniers, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.org/D50467 Files: cfe/trunk/lib/Sema/SemaChecking.cpp cfe/trunk/test/SemaCXX/warn-float-conversion.cpp Index: cfe/trunk/lib/Sema/SemaChecking.cpp === --- cfe/trunk/lib/Sema/SemaChecking.cpp +++ cfe/trunk/lib/Sema/SemaChecking.cpp @@ -10282,33 +10282,6 @@ DiagnoseImpCast(S, E, E->getType(), T, CContext, diag, pruneControlFlow); } -/// Analyze the given compound assignment for the possible losing of -/// floating-point precision. -static void AnalyzeCompoundAssignment(Sema &S, BinaryOperator *E) { - assert(isa(E) && - "Must be compound assignment operation"); - // Recurse on the LHS and RHS in here - AnalyzeImplicitConversions(S, E->getLHS(), E->getOperatorLoc()); - AnalyzeImplicitConversions(S, E->getRHS(), E->getOperatorLoc()); - - // Now check the outermost expression - const auto *ResultBT = E->getLHS()->getType()->getAs(); - const auto *RBT = cast(E) -->getComputationResultType() -->getAs(); - - // If both source and target are floating points. - if (ResultBT && ResultBT->isFloatingPoint() && RBT && RBT->isFloatingPoint()) -// Builtin FP kinds are ordered by increasing FP rank. -if (ResultBT->getKind() < RBT->getKind()) - // We don't want to warn for system macro. - if (!S.SourceMgr.isInSystemMacro(E->getOperatorLoc())) -// warn about dropping FP rank. -DiagnoseImpCast(S, E->getRHS(), E->getLHS()->getType(), -E->getOperatorLoc(), -diag::warn_impcast_float_result_precision); -} - /// Diagnose an implicit cast from a floating point value to an integer value. static void DiagnoseFloatingImpCast(Sema &S, Expr *E, QualType T, SourceLocation CContext) { @@ -10411,6 +10384,39 @@ } } +/// Analyze the given compound assignment for the possible losing of +/// floating-point precision. +static void AnalyzeCompoundAssignment(Sema &S, BinaryOperator *E) { + assert(isa(E) && + "Must be compound assignment operation"); + // Recurse on the LHS and RHS in here + AnalyzeImplicitConversions(S, E->getLHS(), E->getOperatorLoc()); + AnalyzeImplicitConversions(S, E->getRHS(), E->getOperatorLoc()); + + // Now check the outermost expression + const auto *ResultBT = E->getLHS()->getType()->getAs(); + const auto *RBT = cast(E) +->getComputationResultType() +->getAs(); + + // The below checks assume source is floating point. + if (!ResultBT || !RBT || !RBT->isFloatingPoint()) return; + + // If source is floating point but target is not. + if (!ResultBT->isFloatingPoint()) +return DiagnoseFloatingImpCast(S, E, E->getRHS()->getType(), + E->getExprLoc()); + + // If both source and target are floating points. + // Builtin FP kinds are ordered by increasing FP rank. + if (ResultBT->getKind() < RBT->getKind() && + // We don't want to warn for system macro. + !S.SourceMgr.isInSystemMacro(E->getOperatorLoc())) +// warn about dropping FP rank. +DiagnoseImpCast(S, E->getRHS(), E->getLHS()->getType(), E->getOperatorLoc(), +diag::warn_impcast_float_result_precision); +} + static std::string PrettyPrintInRange(const llvm::APSInt &Value, IntRange Range) { if (!Range.Width) return "0"; Index: cfe/trunk/test/SemaCXX/warn-float-conversion.cpp === --- cfe/trunk/test/SemaCXX/warn-float-conversion.cpp +++ cfe/trunk/test/SemaCXX/warn-float-conversion.cpp @@ -41,6 +41,32 @@ l = ld; //expected-warning{{conversion}} } +void CompoundAssignment() { + int x = 3; + + x += 1.234; //expected-warning{{conversion}} + x -= -0.0; //expected-warning{{conversion}} + x *= 1.1f; //expected-warning{{conversion}} + x /= -2.2f; //expected-warning{{conversion}} + + int y = x += 1.4f; //expected-warning{{conversion}} + + float z = 1.1f; + double w = -2.2; + + y += z + w; //expected-warning{{conversion}} +} + +# 1 "foo.h" 3 +// ^ the following text comes from a system header file. +#define SYSTEM_MACRO_FLOAT(x) do { (x) += 1.1; } while(0) +# 1 "warn-float-conversion.cpp" 1 +// ^ start of a new file. +void SystemMacro() { + float x = 0.0f; + SYSTEM_MACRO_FLOAT(x); +} + void Test() { int a1 = 10.0/2.0; //expected-warning{{conversion}} int a2 = 1.0/2.0; //expected-warning{{conversion}} ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.
[PATCH] D50467: [SEMA] add more -Wfloat-conversion to compound assigment analysis
aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM! Repository: rC Clang https://reviews.llvm.org/D50467 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50467: [SEMA] add more -Wfloat-conversion to compound assigment analysis
nickdesaulniers marked 3 inline comments as done. nickdesaulniers added a comment. Thanks for the info, I found https://gcc.gnu.org/onlinedocs/cpp/Preprocessor-Output.html helpful. How does this look? Repository: rC Clang https://reviews.llvm.org/D50467 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50467: [SEMA] add more -Wfloat-conversion to compound assigment analysis
nickdesaulniers updated this revision to Diff 160191. nickdesaulniers added a comment. - fix up system macro case and add test coverage for that case. Repository: rC Clang https://reviews.llvm.org/D50467 Files: lib/Sema/SemaChecking.cpp test/SemaCXX/warn-float-conversion.cpp Index: test/SemaCXX/warn-float-conversion.cpp === --- test/SemaCXX/warn-float-conversion.cpp +++ test/SemaCXX/warn-float-conversion.cpp @@ -41,6 +41,32 @@ l = ld; //expected-warning{{conversion}} } +void CompoundAssignment() { + int x = 3; + + x += 1.234; //expected-warning{{conversion}} + x -= -0.0; //expected-warning{{conversion}} + x *= 1.1f; //expected-warning{{conversion}} + x /= -2.2f; //expected-warning{{conversion}} + + int y = x += 1.4f; //expected-warning{{conversion}} + + float z = 1.1f; + double w = -2.2; + + y += z + w; //expected-warning{{conversion}} +} + +# 1 "foo.h" 3 +// ^ the following text comes from a system header file. +#define SYSTEM_MACRO_FLOAT(x) do { (x) += 1.1; } while(0) +# 1 "warn-float-conversion.cpp" 1 +// ^ start of a new file. +void SystemMacro() { + float x = 0.0f; + SYSTEM_MACRO_FLOAT(x); +} + void Test() { int a1 = 10.0/2.0; //expected-warning{{conversion}} int a2 = 1.0/2.0; //expected-warning{{conversion}} Index: lib/Sema/SemaChecking.cpp === --- lib/Sema/SemaChecking.cpp +++ lib/Sema/SemaChecking.cpp @@ -10292,33 +10292,6 @@ DiagnoseImpCast(S, E, E->getType(), T, CContext, diag, pruneControlFlow); } -/// Analyze the given compound assignment for the possible losing of -/// floating-point precision. -static void AnalyzeCompoundAssignment(Sema &S, BinaryOperator *E) { - assert(isa(E) && - "Must be compound assignment operation"); - // Recurse on the LHS and RHS in here - AnalyzeImplicitConversions(S, E->getLHS(), E->getOperatorLoc()); - AnalyzeImplicitConversions(S, E->getRHS(), E->getOperatorLoc()); - - // Now check the outermost expression - const auto *ResultBT = E->getLHS()->getType()->getAs(); - const auto *RBT = cast(E) -->getComputationResultType() -->getAs(); - - // If both source and target are floating points. - if (ResultBT && ResultBT->isFloatingPoint() && RBT && RBT->isFloatingPoint()) -// Builtin FP kinds are ordered by increasing FP rank. -if (ResultBT->getKind() < RBT->getKind()) - // We don't want to warn for system macro. - if (!S.SourceMgr.isInSystemMacro(E->getOperatorLoc())) -// warn about dropping FP rank. -DiagnoseImpCast(S, E->getRHS(), E->getLHS()->getType(), -E->getOperatorLoc(), -diag::warn_impcast_float_result_precision); -} - /// Diagnose an implicit cast from a floating point value to an integer value. static void DiagnoseFloatingImpCast(Sema &S, Expr *E, QualType T, SourceLocation CContext) { @@ -10421,6 +10394,39 @@ } } +/// Analyze the given compound assignment for the possible losing of +/// floating-point precision. +static void AnalyzeCompoundAssignment(Sema &S, BinaryOperator *E) { + assert(isa(E) && + "Must be compound assignment operation"); + // Recurse on the LHS and RHS in here + AnalyzeImplicitConversions(S, E->getLHS(), E->getOperatorLoc()); + AnalyzeImplicitConversions(S, E->getRHS(), E->getOperatorLoc()); + + // Now check the outermost expression + const auto *ResultBT = E->getLHS()->getType()->getAs(); + const auto *RBT = cast(E) +->getComputationResultType() +->getAs(); + + // The below checks assume source is floating point. + if (!ResultBT || !RBT || !RBT->isFloatingPoint()) return; + + // If source is floating point but target is not. + if (!ResultBT->isFloatingPoint()) +return DiagnoseFloatingImpCast(S, E, E->getRHS()->getType(), + E->getExprLoc()); + + // If both source and target are floating points. + // Builtin FP kinds are ordered by increasing FP rank. + if (ResultBT->getKind() < RBT->getKind() && + // We don't want to warn for system macro. + !S.SourceMgr.isInSystemMacro(E->getOperatorLoc())) +// warn about dropping FP rank. +DiagnoseImpCast(S, E->getRHS(), E->getLHS()->getType(), E->getOperatorLoc(), +diag::warn_impcast_float_result_precision); +} + static std::string PrettyPrintInRange(const llvm::APSInt &Value, IntRange Range) { if (!Range.Width) return "0"; ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50467: [SEMA] add more -Wfloat-conversion to compound assigment analysis
aaron.ballman added inline comments. Comment at: lib/Sema/SemaChecking.cpp:10424 + // We don't want to warn for system macro. + S.SourceMgr.isInSystemMacro(E->getOperatorLoc())) +// warn about dropping FP rank. nickdesaulniers wrote: > aaron.ballman wrote: > > This looks incorrect to me -- this is testing the rank difference and that > > it is in a system macro (the `!` was dropped). If this didn't cause any > > tests to fail, we should add a test that would fail for it. > Thanks for the code review! > > Sorry, I don't understand what you mean by "rank difference"? F16 vs F32 vs > F64 vs F128 etc? > > When you say "this looks incorrect," are you commenting on the code I added > (Lines 10415 to 10418) or the prexisting code that I moved (Lines > 10420-10427)? > > Good catch on dropping the `!`, that was definitely not intentional (and now > I'm curious if `dw` in vim will delete `(!` together), will add back. > > I'm happy to add a test for the system macro, but I also don't know what that > is. Can you explain? Sorry, my explanation may have been confusing. A better way to phrase it would have been "this isn't doing what the old code used to do, and I don't think you intended to change it." I was talking about the `!` that disappeared. I think you can use linemarker directives to get this to happen: ``` # 1 "systemheader.h" 1 3 #define MACRO # 1 "test_file.c" 2 // Your code that uses MACRO. ``` Repository: rC Clang https://reviews.llvm.org/D50467 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50467: [SEMA] add more -Wfloat-conversion to compound assigment analysis
nickdesaulniers added inline comments. Comment at: lib/Sema/SemaChecking.cpp:10424 + // We don't want to warn for system macro. + S.SourceMgr.isInSystemMacro(E->getOperatorLoc())) +// warn about dropping FP rank. aaron.ballman wrote: > This looks incorrect to me -- this is testing the rank difference and that it > is in a system macro (the `!` was dropped). If this didn't cause any tests to > fail, we should add a test that would fail for it. Thanks for the code review! Sorry, I don't understand what you mean by "rank difference"? F16 vs F32 vs F64 vs F128 etc? When you say "this looks incorrect," are you commenting on the code I added (Lines 10415 to 10418) or the prexisting code that I moved (Lines 10420-10427)? Good catch on dropping the `!`, that was definitely not intentional (and now I'm curious if `dw` in vim will delete `(!` together), will add back. I'm happy to add a test for the system macro, but I also don't know what that is. Can you explain? Repository: rC Clang https://reviews.llvm.org/D50467 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50467: [SEMA] add more -Wfloat-conversion to compound assigment analysis
aaron.ballman requested changes to this revision. aaron.ballman added inline comments. This revision now requires changes to proceed. Comment at: lib/Sema/SemaChecking.cpp:10424 + // We don't want to warn for system macro. + S.SourceMgr.isInSystemMacro(E->getOperatorLoc())) +// warn about dropping FP rank. This looks incorrect to me -- this is testing the rank difference and that it is in a system macro (the `!` was dropped). If this didn't cause any tests to fail, we should add a test that would fail for it. Repository: rC Clang https://reviews.llvm.org/D50467 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50467: [SEMA] add more -Wfloat-conversion to compound assigment analysis
acoomans accepted this revision. acoomans added a comment. This revision is now accepted and ready to land. LGTM Repository: rC Clang https://reviews.llvm.org/D50467 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50467: [SEMA] add more -Wfloat-conversion to compound assigment analysis
nickdesaulniers updated this revision to Diff 159820. nickdesaulniers added a comment. - clean up conditional and add comment Repository: rC Clang https://reviews.llvm.org/D50467 Files: lib/Sema/SemaChecking.cpp test/SemaCXX/warn-float-conversion.cpp Index: test/SemaCXX/warn-float-conversion.cpp === --- test/SemaCXX/warn-float-conversion.cpp +++ test/SemaCXX/warn-float-conversion.cpp @@ -41,6 +41,22 @@ l = ld; //expected-warning{{conversion}} } +void CompoundAssignment() { + int x = 3; + + x += 1.234; //expected-warning{{conversion}} + x -= -0.0; //expected-warning{{conversion}} + x *= 1.1f; //expected-warning{{conversion}} + x /= -2.2f; //expected-warning{{conversion}} + + int y = x += 1.4f; //expected-warning{{conversion}} + + float z = 1.1f; + double w = -2.2; + + y += z + w; //expected-warning{{conversion}} +} + void Test() { int a1 = 10.0/2.0; //expected-warning{{conversion}} int a2 = 1.0/2.0; //expected-warning{{conversion}} Index: lib/Sema/SemaChecking.cpp === --- lib/Sema/SemaChecking.cpp +++ lib/Sema/SemaChecking.cpp @@ -10292,33 +10292,6 @@ DiagnoseImpCast(S, E, E->getType(), T, CContext, diag, pruneControlFlow); } -/// Analyze the given compound assignment for the possible losing of -/// floating-point precision. -static void AnalyzeCompoundAssignment(Sema &S, BinaryOperator *E) { - assert(isa(E) && - "Must be compound assignment operation"); - // Recurse on the LHS and RHS in here - AnalyzeImplicitConversions(S, E->getLHS(), E->getOperatorLoc()); - AnalyzeImplicitConversions(S, E->getRHS(), E->getOperatorLoc()); - - // Now check the outermost expression - const auto *ResultBT = E->getLHS()->getType()->getAs(); - const auto *RBT = cast(E) -->getComputationResultType() -->getAs(); - - // If both source and target are floating points. - if (ResultBT && ResultBT->isFloatingPoint() && RBT && RBT->isFloatingPoint()) -// Builtin FP kinds are ordered by increasing FP rank. -if (ResultBT->getKind() < RBT->getKind()) - // We don't want to warn for system macro. - if (!S.SourceMgr.isInSystemMacro(E->getOperatorLoc())) -// warn about dropping FP rank. -DiagnoseImpCast(S, E->getRHS(), E->getLHS()->getType(), -E->getOperatorLoc(), -diag::warn_impcast_float_result_precision); -} - /// Diagnose an implicit cast from a floating point value to an integer value. static void DiagnoseFloatingImpCast(Sema &S, Expr *E, QualType T, SourceLocation CContext) { @@ -10421,6 +10394,39 @@ } } +/// Analyze the given compound assignment for the possible losing of +/// floating-point precision. +static void AnalyzeCompoundAssignment(Sema &S, BinaryOperator *E) { + assert(isa(E) && + "Must be compound assignment operation"); + // Recurse on the LHS and RHS in here + AnalyzeImplicitConversions(S, E->getLHS(), E->getOperatorLoc()); + AnalyzeImplicitConversions(S, E->getRHS(), E->getOperatorLoc()); + + // Now check the outermost expression + const auto *ResultBT = E->getLHS()->getType()->getAs(); + const auto *RBT = cast(E) +->getComputationResultType() +->getAs(); + + // The below checks assume source is floating point. + if (!ResultBT || !RBT || !RBT->isFloatingPoint()) return; + + // If source is floating point but target is not. + if (!ResultBT->isFloatingPoint()) +return DiagnoseFloatingImpCast(S, E, E->getRHS()->getType(), + E->getExprLoc()); + + // If both source and target are floating points. + // Builtin FP kinds are ordered by increasing FP rank. + if (ResultBT->getKind() < RBT->getKind() && + // We don't want to warn for system macro. + S.SourceMgr.isInSystemMacro(E->getOperatorLoc())) +// warn about dropping FP rank. +DiagnoseImpCast(S, E->getRHS(), E->getLHS()->getType(), E->getOperatorLoc(), +diag::warn_impcast_float_result_precision); +} + static std::string PrettyPrintInRange(const llvm::APSInt &Value, IntRange Range) { if (!Range.Width) return "0"; ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50467: [SEMA] add more -Wfloat-conversion to compound assigment analysis
nickdesaulniers added inline comments. Comment at: lib/Sema/SemaChecking.cpp:10411 +->getAs(); + if (!ResultBT || !(RBT && RBT->isFloatingPoint())) return; + nickdesaulniers wrote: > pirama wrote: > > Add a comment explaining this conditional as well? > > > > > Return if source and target types are unavailable or if source is not a > > > floating point. > > > > With the comment, it might be cleaner to read if you expand the negation: > > `!ResultBT || !RBT || !RBT->isFloatingPoint()` > `ResultBT` and `RBT` are pointers that may be null and need to be checked. > The previous code did this before accessing them, I've just moved the checks > to be sooner. > > If `!RBT` (if it's `nullptr`), then `!RBT->isFloatingPoint()` would be a null > deref, so unfortunately I can not expand it as recommended. oops, nevermind Repository: rC Clang https://reviews.llvm.org/D50467 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50467: [SEMA] add more -Wfloat-conversion to compound assigment analysis
nickdesaulniers added inline comments. Comment at: lib/Sema/SemaChecking.cpp:10411 +->getAs(); + if (!ResultBT || !(RBT && RBT->isFloatingPoint())) return; + pirama wrote: > Add a comment explaining this conditional as well? > > > Return if source and target types are unavailable or if source is not a > > floating point. > > With the comment, it might be cleaner to read if you expand the negation: > `!ResultBT || !RBT || !RBT->isFloatingPoint()` `ResultBT` and `RBT` are pointers that may be null and need to be checked. The previous code did this before accessing them, I've just moved the checks to be sooner. If `!RBT` (if it's `nullptr`), then `!RBT->isFloatingPoint()` would be a null deref, so unfortunately I can not expand it as recommended. Repository: rC Clang https://reviews.llvm.org/D50467 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50467: [SEMA] add more -Wfloat-conversion to compound assigment analysis
pirama added inline comments. Comment at: lib/Sema/SemaChecking.cpp:10411 +->getAs(); + if (!ResultBT || !(RBT && RBT->isFloatingPoint())) return; + Add a comment explaining this conditional as well? > Return if source and target types are unavailable or if source is not a > floating point. With the comment, it might be cleaner to read if you expand the negation: `!ResultBT || !RBT || !RBT->isFloatingPoint()` Repository: rC Clang https://reviews.llvm.org/D50467 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50467: [SEMA] add more -Wfloat-conversion to compound assigment analysis
nickdesaulniers added inline comments. Comment at: lib/Sema/SemaChecking.cpp:10413-10416 + // If source is floating point but target is not. + if (!ResultBT->isFloatingPoint()) +return DiagnoseFloatingImpCast(S, E, E->getRHS()->getType(), + E->getExprLoc()); This is essentially the only new code. AnalyzeCompoundAssignment was moved in order to be able to call DiagnoseFloatingImpCast (it's defined above). The rest is a small refactoring of the conditionals. Repository: rC Clang https://reviews.llvm.org/D50467 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50467: [SEMA] add more -Wfloat-conversion to compound assigment analysis
nickdesaulniers updated this revision to Diff 159763. nickdesaulniers added a comment. - rework ordering of conditionals to reduce indentation Repository: rC Clang https://reviews.llvm.org/D50467 Files: lib/Sema/SemaChecking.cpp test/SemaCXX/warn-float-conversion.cpp Index: test/SemaCXX/warn-float-conversion.cpp === --- test/SemaCXX/warn-float-conversion.cpp +++ test/SemaCXX/warn-float-conversion.cpp @@ -41,6 +41,22 @@ l = ld; //expected-warning{{conversion}} } +void CompoundAssignment() { + int x = 3; + + x += 1.234; //expected-warning{{conversion}} + x -= -0.0; //expected-warning{{conversion}} + x *= 1.1f; //expected-warning{{conversion}} + x /= -2.2f; //expected-warning{{conversion}} + + int y = x += 1.4f; //expected-warning{{conversion}} + + float z = 1.1f; + double w = -2.2; + + y += z + w; //expected-warning{{conversion}} +} + void Test() { int a1 = 10.0/2.0; //expected-warning{{conversion}} int a2 = 1.0/2.0; //expected-warning{{conversion}} Index: lib/Sema/SemaChecking.cpp === --- lib/Sema/SemaChecking.cpp +++ lib/Sema/SemaChecking.cpp @@ -10292,33 +10292,6 @@ DiagnoseImpCast(S, E, E->getType(), T, CContext, diag, pruneControlFlow); } -/// Analyze the given compound assignment for the possible losing of -/// floating-point precision. -static void AnalyzeCompoundAssignment(Sema &S, BinaryOperator *E) { - assert(isa(E) && - "Must be compound assignment operation"); - // Recurse on the LHS and RHS in here - AnalyzeImplicitConversions(S, E->getLHS(), E->getOperatorLoc()); - AnalyzeImplicitConversions(S, E->getRHS(), E->getOperatorLoc()); - - // Now check the outermost expression - const auto *ResultBT = E->getLHS()->getType()->getAs(); - const auto *RBT = cast(E) -->getComputationResultType() -->getAs(); - - // If both source and target are floating points. - if (ResultBT && ResultBT->isFloatingPoint() && RBT && RBT->isFloatingPoint()) -// Builtin FP kinds are ordered by increasing FP rank. -if (ResultBT->getKind() < RBT->getKind()) - // We don't want to warn for system macro. - if (!S.SourceMgr.isInSystemMacro(E->getOperatorLoc())) -// warn about dropping FP rank. -DiagnoseImpCast(S, E->getRHS(), E->getLHS()->getType(), -E->getOperatorLoc(), -diag::warn_impcast_float_result_precision); -} - /// Diagnose an implicit cast from a floating point value to an integer value. static void DiagnoseFloatingImpCast(Sema &S, Expr *E, QualType T, SourceLocation CContext) { @@ -10421,6 +10394,37 @@ } } +/// Analyze the given compound assignment for the possible losing of +/// floating-point precision. +static void AnalyzeCompoundAssignment(Sema &S, BinaryOperator *E) { + assert(isa(E) && + "Must be compound assignment operation"); + // Recurse on the LHS and RHS in here + AnalyzeImplicitConversions(S, E->getLHS(), E->getOperatorLoc()); + AnalyzeImplicitConversions(S, E->getRHS(), E->getOperatorLoc()); + + // Now check the outermost expression + const auto *ResultBT = E->getLHS()->getType()->getAs(); + const auto *RBT = cast(E) +->getComputationResultType() +->getAs(); + if (!ResultBT || !(RBT && RBT->isFloatingPoint())) return; + + // If source is floating point but target is not. + if (!ResultBT->isFloatingPoint()) +return DiagnoseFloatingImpCast(S, E, E->getRHS()->getType(), + E->getExprLoc()); + + // If both source and target are floating points. + // Builtin FP kinds are ordered by increasing FP rank. + if (ResultBT->getKind() < RBT->getKind() && + // We don't want to warn for system macro. + S.SourceMgr.isInSystemMacro(E->getOperatorLoc())) +// warn about dropping FP rank. +DiagnoseImpCast(S, E->getRHS(), E->getLHS()->getType(), E->getOperatorLoc(), +diag::warn_impcast_float_result_precision); +} + static std::string PrettyPrintInRange(const llvm::APSInt &Value, IntRange Range) { if (!Range.Width) return "0"; ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50467: [SEMA] add more -Wfloat-conversion to compound assigment analysis
nickdesaulniers created this revision. Herald added a subscriber: cfe-commits. Fixes Bug: https://bugs.llvm.org/show_bug.cgi?id=27061 Repository: rC Clang https://reviews.llvm.org/D50467 Files: lib/Sema/SemaChecking.cpp test/SemaCXX/warn-float-conversion.cpp Index: test/SemaCXX/warn-float-conversion.cpp === --- test/SemaCXX/warn-float-conversion.cpp +++ test/SemaCXX/warn-float-conversion.cpp @@ -41,6 +41,22 @@ l = ld; //expected-warning{{conversion}} } +void CompoundAssignment() { + int x = 3; + + x += 1.234; //expected-warning{{conversion}} + x -= -0.0; //expected-warning{{conversion}} + x *= 1.1f; //expected-warning{{conversion}} + x /= -2.2f; //expected-warning{{conversion}} + + int y = x += 1.4f; //expected-warning{{conversion}} + + float z = 1.1f; + double w = -2.2; + + y += z + w; //expected-warning{{conversion}} +} + void Test() { int a1 = 10.0/2.0; //expected-warning{{conversion}} int a2 = 1.0/2.0; //expected-warning{{conversion}} Index: lib/Sema/SemaChecking.cpp === --- lib/Sema/SemaChecking.cpp +++ lib/Sema/SemaChecking.cpp @@ -10292,33 +10292,6 @@ DiagnoseImpCast(S, E, E->getType(), T, CContext, diag, pruneControlFlow); } -/// Analyze the given compound assignment for the possible losing of -/// floating-point precision. -static void AnalyzeCompoundAssignment(Sema &S, BinaryOperator *E) { - assert(isa(E) && - "Must be compound assignment operation"); - // Recurse on the LHS and RHS in here - AnalyzeImplicitConversions(S, E->getLHS(), E->getOperatorLoc()); - AnalyzeImplicitConversions(S, E->getRHS(), E->getOperatorLoc()); - - // Now check the outermost expression - const auto *ResultBT = E->getLHS()->getType()->getAs(); - const auto *RBT = cast(E) -->getComputationResultType() -->getAs(); - - // If both source and target are floating points. - if (ResultBT && ResultBT->isFloatingPoint() && RBT && RBT->isFloatingPoint()) -// Builtin FP kinds are ordered by increasing FP rank. -if (ResultBT->getKind() < RBT->getKind()) - // We don't want to warn for system macro. - if (!S.SourceMgr.isInSystemMacro(E->getOperatorLoc())) -// warn about dropping FP rank. -DiagnoseImpCast(S, E->getRHS(), E->getLHS()->getType(), -E->getOperatorLoc(), -diag::warn_impcast_float_result_precision); -} - /// Diagnose an implicit cast from a floating point value to an integer value. static void DiagnoseFloatingImpCast(Sema &S, Expr *E, QualType T, SourceLocation CContext) { @@ -10421,6 +10394,38 @@ } } +/// Analyze the given compound assignment for the possible losing of +/// floating-point precision. +static void AnalyzeCompoundAssignment(Sema &S, BinaryOperator *E) { + assert(isa(E) && + "Must be compound assignment operation"); + // Recurse on the LHS and RHS in here + AnalyzeImplicitConversions(S, E->getLHS(), E->getOperatorLoc()); + AnalyzeImplicitConversions(S, E->getRHS(), E->getOperatorLoc()); + + // Now check the outermost expression + const auto *ResultBT = E->getLHS()->getType()->getAs(); + const auto *RBT = cast(E) +->getComputationResultType() +->getAs(); + if (!ResultBT || !(RBT && RBT->isFloatingPoint())) return; + + // If both source and target are floating points. + if (ResultBT->isFloatingPoint()) { +// Builtin FP kinds are ordered by increasing FP rank. +if(ResultBT->getKind() < RBT->getKind() && + // We don't want to warn for system macro. + S.SourceMgr.isInSystemMacro(E->getOperatorLoc())) + // warn about dropping FP rank. + DiagnoseImpCast(S, E->getRHS(), E->getLHS()->getType(), + E->getOperatorLoc(), + diag::warn_impcast_float_result_precision); + + } else +// If source is floating point but target is not. +DiagnoseFloatingImpCast(S, E, E->getRHS()->getType(), E->getExprLoc()); +} + static std::string PrettyPrintInRange(const llvm::APSInt &Value, IntRange Range) { if (!Range.Width) return "0"; ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits