[PATCH] D52835: [Diagnostics] Check integer to floating point number implicit conversions

aaron.ballman added a comment. In D52835#1412416 , @xbolva00 wrote: > @aaron.ballman does it make sense to warn for this case only in C/pre-C++11 > mode? Since this case in C++11/14/17 is handled by -Wnarrowing, as we see > from tests. I don't think

[PATCH] D52835: [Diagnostics] Check integer to floating point number implicit conversions

xbolva00 added a comment. @aaron.ballman does it make sense to warn for this case only in C mode? Since this case in C++ is handled by -Wnarrowing, as we see from tests. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D52835/new/ https://reviews.llvm.org/D52835

[PATCH] D52835: [Diagnostics] Check integer to floating point number implicit conversions

xbolva00 marked an inline comment as done. xbolva00 added inline comments. Comment at: test/CXX/dcl.decl/dcl.init/dcl.init.list/p7-0x.cpp:136 Agg f8 = {EnumVal}; // OK + // expected-warning@+1 {{implicit conversion from 'int' to 'float' changes value from 123456789 to

[PATCH] D52835: [Diagnostics] Check integer to floating point number implicit conversions

aaron.ballman added inline comments. Comment at: test/CXX/dcl.decl/dcl.init/dcl.init.list/p7-0x.cpp:136 Agg f8 = {EnumVal}; // OK + // expected-warning@+1 {{implicit conversion from 'int' to 'float' changes value from 123456789 to 1.2345679E+8}} Agg f9 = {123456789};

[PATCH] D52835: [Diagnostics] Check integer to floating point number implicit conversions

xbolva00 updated this revision to Diff 175777. xbolva00 added a comment. - Updated. Addressed review comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D52835/new/ https://reviews.llvm.org/D52835 Files: include/clang/Basic/DiagnosticSemaKinds.td lib/Sema/SemaChecking.cpp

[PATCH] D52835: [Diagnostics] Check integer to floating point number implicit conversions

xbolva00 added a comment. I removed _Float16 related tests since some bots may fall with it, i dont know much about this custom type. Anyway, It is fine on linux x86 64. I will restore them. System macro - I will fix it. https://reviews.llvm.org/D52835

[PATCH] D52835: [Diagnostics] Check integer to floating point number implicit conversions

aaron.ballman added a comment. It looks like you removed a considerable amount of testing coverage; why? Comment at: lib/Sema/SemaChecking.cpp:10920-10921 +if (E->EvaluateAsInt(IntValue, S.Context, Expr::SE_AllowSideEffects)) { + if (S.SourceMgr.isInSystemMacro(CC)) +

[PATCH] D52835: [Diagnostics] Check integer to floating point number implicit conversions

xbolva00 updated this revision to Diff 174505. xbolva00 added a comment. Addressed comments, fixed tests https://reviews.llvm.org/D52835 Files: include/clang/Basic/DiagnosticSemaKinds.td lib/Sema/SemaChecking.cpp test/CXX/dcl.decl/dcl.init/dcl.init.list/p7-0x.cpp

[PATCH] D52835: [Diagnostics] Check integer to floating point number implicit conversions

lebedev.ri added a comment. Are you sure `check-clang` passes with this patch? It looks like some bots weren't happy before the revert. (Also, is this warning enabled when building llvm? If yes, you might want to double-check stage2 build.) Repository: rC Clang

[PATCH] D52835: [Diagnostics] Check integer to floating point number implicit conversions

aaron.ballman added a comment. In https://reviews.llvm.org/D52835#1298428, @xbolva00 wrote: > Reverted. > Ok, I will address your comments soon. Thanks! I'm sorry for the delayed reviews -- there were wg21 meetings last week, which several of the reviewers were attending. Repository: rC

[PATCH] D52835: [Diagnostics] Check integer to floating point number implicit conversions

xbolva00 added a comment. Ok, I will address your comments soon. Repository: rC Clang https://reviews.llvm.org/D52835 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D52835: [Diagnostics] Check integer to floating point number implicit conversions

This revision was automatically updated to reflect the committed changes. Closed by commit rC346865: [Diagnostics] Check integer to floating point number implicit conversions (authored by xbolva00, committed by ). Changed prior to commit: https://reviews.llvm.org/D52835?vs=172509=174030#toc

[PATCH] D52835: [Diagnostics] Check integer to floating point number implicit conversions

aaron.ballman added inline comments. Comment at: lib/Sema/SemaChecking.cpp:113-115 +static void PrettyPrintFloat(const llvm::APFloat , + const llvm::fltSemantics , + SmallVectorImpl ) { Parameter names

[PATCH] D52835: [Diagnostics] Check integer to floating point number implicit conversions

xbolva00 added a comment. Since no futher comments, i think it is ok. After day or two I will commit it. https://reviews.llvm.org/D52835 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D52835: [Diagnostics] Check integer to floating point number implicit conversions

xbolva00 added a comment. @aaron.ballman https://reviews.llvm.org/D52835 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D52835: [Diagnostics] Check integer to floating point number implicit conversions

xbolva00 added a comment. Ping :) https://reviews.llvm.org/D52835 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D52835: [Diagnostics] Check integer to floating point number implicit conversions

xbolva00 updated this revision to Diff 172509. https://reviews.llvm.org/D52835 Files: include/clang/Basic/DiagnosticSemaKinds.td lib/Sema/SemaChecking.cpp test/Sema/ext_vector_casts.c test/Sema/impcast-integer-float.c Index: test/Sema/impcast-integer-float.c

[PATCH] D52835: [Diagnostics] Check integer to floating point number implicit conversions

xbolva00 added a comment. Since I don't think we have any false positives here, maybe it would be useful to add it to -Wall or -Wextra? https://reviews.llvm.org/D52835 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D52835: [Diagnostics] Check integer to floating point number implicit conversions

xbolva00 added a comment. maybe @aaron.ballman wants to take a look too https://reviews.llvm.org/D52835 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D52835: [Diagnostics] Check integer to floating point number implicit conversions

xbolva00 added a comment. Ping Anything else to be done here? I would like to finish this patch to work on other things. https://reviews.llvm.org/D52835 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D52835: [Diagnostics] Check integer to floating point number implicit conversions

xbolva00 added a comment. ping https://reviews.llvm.org/D52835 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D52835: [Diagnostics] Check integer to floating point number implicit conversions

xbolva00 added a comment. Ping https://reviews.llvm.org/D52835 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D52835: [Diagnostics] Check integer to floating point number implicit conversions

xbolva00 added a comment. Is PrecisionConversion acceptable? If not, any suggestions for name? https://reviews.llvm.org/D52835 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D52835: [Diagnostics] Check integer to floating point number implicit conversions

xbolva00 updated this revision to Diff 169881. xbolva00 added a comment. - Added missing test file https://reviews.llvm.org/D52835 Files: include/clang/Basic/DiagnosticGroups.td include/clang/Basic/DiagnosticSemaKinds.td lib/Sema/SemaChecking.cpp test/Sema/ext_vector_casts.c

[PATCH] D52835: [Diagnostics] Check integer to floating point number implicit conversions

efriedma added a comment. Herald added a subscriber: nhaehnle. Looks like the latest update is missing a test file? https://reviews.llvm.org/D52835 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D52835: [Diagnostics] Check integer to floating point number implicit conversions

xbolva00 updated this revision to Diff 169567. xbolva00 added a comment. - New diagnostic group https://reviews.llvm.org/D52835 Files: include/clang/Basic/DiagnosticGroups.td include/clang/Basic/DiagnosticSemaKinds.td lib/Sema/SemaChecking.cpp test/Sema/ext_vector_casts.c Index:

[PATCH] D52835: [Diagnostics] Check integer to floating point number implicit conversions

xbolva00 added a comment. So any suggestions? https://reviews.llvm.org/D52835 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D52835: [Diagnostics] Check integer to floating point number implicit conversions

lebedev.ri added a comment. In https://reviews.llvm.org/D52835#1263032, @xbolva00 wrote: > As noted, this in under LiteralConversion (-Wconversion). GCC has it too > under -Wconversion, so I think it is fine as is, or? > @rsmith It's not so much about "which flag group do i need to enable

[PATCH] D52835: [Diagnostics] Check integer to floating point number implicit conversions

xbolva00 added a comment. As noted, this in under LiteralConversion (-Wconversion). GCC has it too under -Wconversion, so I think it is fine as is, or? @rsmith https://reviews.llvm.org/D52835 ___ cfe-commits mailing list

[PATCH] D52835: [Diagnostics] Check integer to floating point number implicit conversions

xbolva00 added inline comments. Comment at: lib/Sema/SemaChecking.cpp:10881-10882 +S.DiagRuntimeBehavior( +E->getExprLoc(), E, +S.PDiag(diag::warn_impcast_literal_float_to_integer) +<< E->getType() << T << PrettySourceValue <<

[PATCH] D52835: [Diagnostics] Check integer to floating point number implicit conversions

efriedma added inline comments. Comment at: lib/Sema/SemaChecking.cpp:10881-10882 +S.DiagRuntimeBehavior( +E->getExprLoc(), E, +S.PDiag(diag::warn_impcast_literal_float_to_integer) +<< E->getType() << T << PrettySourceValue <<

[PATCH] D52835: [Diagnostics] Check integer to floating point number implicit conversions

xbolva00 updated this revision to Diff 169162. xbolva00 added a comment. - Addressed comments https://reviews.llvm.org/D52835 Files: lib/Sema/SemaChecking.cpp test/Sema/ext_vector_casts.c test/Sema/impcast-integer-float.c Index: test/Sema/impcast-integer-float.c

[PATCH] D52835: [Diagnostics] Check integer to floating point number implicit conversions

xbolva00 added inline comments. Comment at: lib/Sema/SemaChecking.cpp:10874 + if (Target->isSpecificBuiltinType(BuiltinType::LongDouble)) +FloatSem = ::APFloat::x87DoubleExtended(); + efriedma wrote: > ASTContext::getFloatTypeSemantics. (Your

[PATCH] D52835: [Diagnostics] Check integer to floating point number implicit conversions

efriedma added a comment. Probably should have a test for something like `float x = (__uint128_t)-1;`, to make sure we print something reasonable. Comment at: lib/Sema/SemaChecking.cpp:10874 + if (Target->isSpecificBuiltinType(BuiltinType::LongDouble)) +FloatSem

[PATCH] D52835: [Diagnostics] Check integer to floating point number implicit conversions

xbolva00 added a comment. Any comments? Waiting for approval here to move focus on some other patches :) https://reviews.llvm.org/D52835 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D52835: [Diagnostics] Check integer to floating point number implicit conversions

xbolva00 added a comment. Ping :) https://reviews.llvm.org/D52835 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D52835: [Diagnostics] Check integer to floating point number implicit conversions

xbolva00 added a comment. Ok now? https://reviews.llvm.org/D52835 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D52835: [Diagnostics] Check integer to floating point number implicit conversions

xbolva00 added inline comments. Comment at: lib/Sema/SemaChecking.cpp:110 +/// are usually useless +static unsigned AdjustPrecision(unsigned precision) { + return (precision * 59 + 195) / 196; rsmith wrote: > xbolva00 wrote: > > xbolva00 wrote: > > > xbolva00

[PATCH] D52835: [Diagnostics] Check integer to floating point number implicit conversions

rsmith added inline comments. Comment at: lib/Sema/SemaChecking.cpp:110 +/// are usually useless +static unsigned AdjustPrecision(unsigned precision) { + return (precision * 59 + 195) / 196; xbolva00 wrote: > xbolva00 wrote: > > xbolva00 wrote: > > >

[PATCH] D52835: [Diagnostics] Check integer to floating point number implicit conversions

xbolva00 updated this revision to Diff 168176. xbolva00 added a comment. - Support for Float16 https://reviews.llvm.org/D52835 Files: lib/Sema/SemaChecking.cpp test/Sema/ext_vector_casts.c test/Sema/impcast-integer-float.c Index: test/Sema/impcast-integer-float.c

[PATCH] D52835: [Diagnostics] Check integer to floating point number implicit conversions

xbolva00 updated this revision to Diff 168173. xbolva00 added a comment. - Check also int128_t to float https://reviews.llvm.org/D52835 Files: lib/Sema/SemaChecking.cpp test/Sema/ext_vector_casts.c test/Sema/impcast-integer-float.c Index: test/Sema/impcast-integer-float.c

[PATCH] D52835: [Diagnostics] Check integer to floating point number implicit conversions

xbolva00 added inline comments. Comment at: lib/Sema/SemaChecking.cpp:110 +/// are usually useless +static unsigned AdjustPrecision(unsigned precision) { + return (precision * 59 + 195) / 196; xbolva00 wrote: > xbolva00 wrote: > > craig.topper wrote: > > >

[PATCH] D52835: [Diagnostics] Check integer to floating point number implicit conversions

xbolva00 updated this revision to Diff 168171. xbolva00 added a comment. - Addressed some comments https://reviews.llvm.org/D52835 Files: lib/Sema/SemaChecking.cpp test/Sema/ext_vector_casts.c test/Sema/impcast-integer-float.c Index: test/Sema/impcast-integer-float.c

[PATCH] D52835: [Diagnostics] Check integer to floating point number implicit conversions

xbolva00 added inline comments. Comment at: lib/Sema/SemaChecking.cpp:110 +/// are usually useless +static unsigned AdjustPrecision(unsigned precision) { + return (precision * 59 + 195) / 196; xbolva00 wrote: > craig.topper wrote: > > scanon wrote: > > >

[PATCH] D52835: [Diagnostics] Check integer to floating point number implicit conversions

xbolva00 added inline comments. Comment at: lib/Sema/SemaChecking.cpp:110 +/// are usually useless +static unsigned AdjustPrecision(unsigned precision) { + return (precision * 59 + 195) / 196; craig.topper wrote: > scanon wrote: > > erichkeane wrote: > > >

[PATCH] D52835: [Diagnostics] Check integer to floating point number implicit conversions

craig.topper added inline comments. Comment at: lib/Sema/SemaChecking.cpp:110 +/// are usually useless +static unsigned AdjustPrecision(unsigned precision) { + return (precision * 59 + 195) / 196; scanon wrote: > erichkeane wrote: > > Hmm I don't terribly

[PATCH] D52835: [Diagnostics] Check integer to floating point number implicit conversions

xbolva00 added inline comments. Comment at: lib/Sema/SemaChecking.cpp:10883 + precision = AdjustPrecision(precision); + FloatValue.toString(PrettyTargetValue, precision); + IntValue.toString(PrettySourceValue); erichkeane wrote: > I

[PATCH] D52835: [Diagnostics] Check integer to floating point number implicit conversions

xbolva00 added inline comments. Comment at: lib/Sema/SemaChecking.cpp:10862 + if (Source->isIntegerType() && Target->isFloatingType()) { +const llvm::fltSemantics *FloatSem = nullptr; +if (Target->isSpecificBuiltinType(BuiltinType::Float)) { erichkeane

[PATCH] D52835: [Diagnostics] Check integer to floating point number implicit conversions

scanon added inline comments. Comment at: lib/Sema/SemaChecking.cpp:110 +/// are usually useless +static unsigned AdjustPrecision(unsigned precision) { + return (precision * 59 + 195) / 196; erichkeane wrote: > Hmm I don't terribly understand how this

[PATCH] D52835: [Diagnostics] Check integer to floating point number implicit conversions

erichkeane added inline comments. Comment at: lib/Sema/SemaChecking.cpp:110 +/// are usually useless +static unsigned AdjustPrecision(unsigned precision) { + return (precision * 59 + 195) / 196; Hmm I don't terribly understand how this function works.

[PATCH] D52835: [Diagnostics] Check integer to floating point number implicit conversions

xbolva00 updated this revision to Diff 168150. xbolva00 added a comment. - better warning https://reviews.llvm.org/D52835 Files: lib/Sema/SemaChecking.cpp test/Sema/ext_vector_casts.c test/Sema/impcast-integer-float.c Index: test/Sema/impcast-integer-float.c

[PATCH] D52835: [Diagnostics] Check integer to floating point number implicit conversions

lebedev.ri added inline comments. Comment at: test/Sema/ext_vector_casts.c:121 vf = d + vf; // expected-warning {{implicit conversion loses floating-point precision}} - vf = vf + 0x; + vf = vf + 0x; // expected-warning {{implicit conversion loses

[PATCH] D52835: [Diagnostics] Check integer to floating point number implicit conversions

xbolva00 updated this revision to Diff 168141. https://reviews.llvm.org/D52835 Files: lib/Sema/SemaChecking.cpp test/Sema/ext_vector_casts.c test/Sema/impcast-integer-float.c Index: test/Sema/impcast-integer-float.c === ---

[PATCH] D52835: [Diagnostics] Check integer to floating point number implicit conversions

xbolva00 added inline comments. Comment at: test/Sema/ext_vector_casts.c:121 vf = d + vf; // expected-warning {{implicit conversion loses floating-point precision}} - vf = vf + 0x; + vf = vf + 0x; // expected-warning {{implicit conversion loses

[PATCH] D52835: [Diagnostics] Check integer to floating point number implicit conversions

lebedev.ri added inline comments. Comment at: test/Sema/ext_vector_casts.c:121 vf = d + vf; // expected-warning {{implicit conversion loses floating-point precision}} - vf = vf + 0x; + vf = vf + 0x; // expected-warning {{implicit conversion loses

[PATCH] D52835: [Diagnostics] Check integer to floating point number implicit conversions

xbolva00 updated this revision to Diff 168140. xbolva00 added a comment. - Added tests https://reviews.llvm.org/D52835 Files: lib/Sema/SemaChecking.cpp test/Sema/ext_vector_casts.c test/Sema/impcast-integer-float.c Index: test/Sema/impcast-integer-float.c

[PATCH] D52835: [Diagnostics] Check integer to floating point number implicit conversions

xbolva00 updated this revision to Diff 168137. https://reviews.llvm.org/D52835 Files: lib/Sema/SemaChecking.cpp test/Sema/ext_vector_casts.c Index: test/Sema/ext_vector_casts.c === --- test/Sema/ext_vector_casts.c +++

[PATCH] D52835: [Diagnostics] Check integer to floating point number implicit conversions

xbolva00 created this revision. xbolva00 added a reviewer: rsmith. Herald added a subscriber: cfe-commits. xbolva00 updated this revision to Diff 168137. https://reviews.llvm.org/D52835 Files: lib/Sema/SemaChecking.cpp test/Sema/ext_vector_casts.c Index: test/Sema/ext_vector_casts.c