[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

[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

[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

[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:

[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: >

[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

[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. +

[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

[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:

[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

[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? > > >

[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

[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(), +

[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:

[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: