From a3ee2c625ef760b4de7b0fd72e5810be04b4c19e Mon Sep 17 00:00:00 2001 From: Stephan Bergmann Date: Mon, 12 Aug 2019 11:35:28 +0200 Subject: Fix Clang 10 -Werror,-Wimplicit-int-float-conversion > idlc/source/astexpression.cxx:330:68: error: implicit conversion from 'sal_Int32' (aka 'int') to 'float' changes value from 2147483647 to 2147483648 [-Werror,-Wimplicit-int-float-conversion] > if (ev->u.fval < SAL_MIN_INT32 || ev->u.fval > SAL_MAX_INT32) > ~ ^~~~~~~~~~~~~ > include/sal/types.h:209:32: note: expanded from macro 'SAL_MAX_INT32' > #define SAL_MAX_INT32 ((sal_Int32) 0x7FFFFFFF) > ^~~~~~~~~~~~~~~~~~~~~~~ > idlc/source/astexpression.cxx:414:58: error: implicit conversion from 'sal_uInt32' (aka 'unsigned int') to 'float' changes value from 4294967295 to 4294967296 [-Werror,-Wimplicit-int-float-conversion] > if (ev->u.fval < 0.0 || ev->u.fval > SAL_MAX_UINT32) > ~ ^~~~~~~~~~~~~~ > include/sal/types.h:210:32: note: expanded from macro 'SAL_MAX_UINT32' > #define SAL_MAX_UINT32 ((sal_uInt32) 0xFFFFFFFF) > ^~~~~~~~~~~~~~~~~~~~~~~ > idlc/source/astexpression.cxx:492:68: error: implicit conversion from 'sal_Int64' (aka 'long') to 'float' changes value from 9223372036854775807 to 9223372036854775808 [-Werror,-Wimplicit-int-float-conversion] > if (ev->u.fval < SAL_MIN_INT64 || ev->u.fval > SAL_MAX_INT64) > ~ ^~~~~~~~~~~~~ > include/sal/types.h:212:32: note: expanded from macro 'SAL_MAX_INT64' > #define SAL_MAX_INT64 ((sal_Int64) SAL_CONST_INT64(0x7FFFFFFFFFFFFFFF)) > ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > idlc/source/astexpression.cxx:501:68: error: implicit conversion from 'sal_Int64' (aka 'long') to 'double' changes value from 9223372036854775807 to 9223372036854775808 [-Werror,-Wimplicit-int-float-conversion] > if (ev->u.dval < SAL_MIN_INT64 || ev->u.dval > SAL_MAX_INT64) > ~ ^~~~~~~~~~~~~ > include/sal/types.h:212:32: note: expanded from macro 'SAL_MAX_INT64' > #define SAL_MAX_INT64 ((sal_Int64) SAL_CONST_INT64(0x7FFFFFFFFFFFFFFF)) > ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > idlc/source/astexpression.cxx:574:58: error: implicit conversion from 'sal_uInt64' (aka 'unsigned long') to 'float' changes value from 18446744073709551615 to 18446744073709551616 [-Werror,-Wimplicit-int-float-conversion] > if (ev->u.fval < 0.0 || ev->u.fval > SAL_MAX_UINT64) > ~ ^~~~~~~~~~~~~~ > include/sal/types.h:213:32: note: expanded from macro 'SAL_MAX_UINT64' > #define SAL_MAX_UINT64 ((sal_uInt64) SAL_CONST_UINT64(0xFFFFFFFFFFFFFFFF)) > ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > idlc/source/astexpression.cxx:583:58: error: implicit conversion from 'sal_uInt64' (aka 'unsigned long') to 'double' changes value from 18446744073709551615 to 18446744073709551616 [-Werror,-Wimplicit-int-float-conversion] > if (ev->u.dval < 0.0 || ev->u.dval > SAL_MAX_UINT64) > ~ ^~~~~~~~~~~~~~ > include/sal/types.h:213:32: note: expanded from macro 'SAL_MAX_UINT64' > #define SAL_MAX_UINT64 ((sal_uInt64) SAL_CONST_UINT64(0xFFFFFFFFFFFFFFFF)) > ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ Consitently use the new o3tl::convertsToAtLeast/Most(o3tl::roundAway(...), ...) for all cases in coerce_value that check that a floating-point value falls into an integer range, even those that don't cause a warning. The new idlc/test/parser/conversion.tests is deliberately left out of unoidl/CustomTarget_unoidl-write_test.mk. as unoidl-write doesn't support such conversions from floating-point to integer types. Change-Id: Ie00923e665f2bcb306e1e328614c75b9247512ee Reviewed-on: https://gerrit.libreoffice.org/77353 Tested-by: Jenkins Reviewed-by: Stephan Bergmann --- idlc/CustomTarget_parser_test.mk | 6 +++ idlc/source/astexpression.cxx | 71 ++++++++++++++++++++++++------- idlc/test/parser/conversion.tests | 79 +++++++++++++++++++++++++++++++++++ include/o3tl/float_int_conversion.hxx | 57 +++++++++++++++++++++++++ 4 files changed, 199 insertions(+), 14 deletions(-) create mode 100644 idlc/test/parser/conversion.tests create mode 100644 include/o3tl/float_int_conversion.hxx diff --git a/idlc/CustomTarget_parser_test.mk b/idlc/CustomTarget_parser_test.mk index 0c9d15e4f86b..f8c7632bde67 100644 --- a/idlc/CustomTarget_parser_test.mk +++ b/idlc/CustomTarget_parser_test.mk @@ -18,6 +18,7 @@ $(call gb_CustomTarget_get_target,idlc/parser_test) : \ $(SRCDIR)/idlc/test/parser/attribute.tests \ $(SRCDIR)/idlc/test/parser/constant.tests \ $(SRCDIR)/idlc/test/parser/constructor.tests \ + $(SRCDIR)/idlc/test/parser/conversion.tests \ $(SRCDIR)/idlc/test/parser/interfaceinheritance.tests \ $(SRCDIR)/idlc/test/parser/methodoverload.tests \ $(SRCDIR)/idlc/test/parser/polystruct.tests \ @@ -44,6 +45,11 @@ else $(call gb_CustomTarget_get_workdir,idlc/parser_test)/in.idl 0 \ $(call gb_Executable_get_command,idlc) \ -O $(call gb_CustomTarget_get_workdir,idlc/parser_test) {} && \ + $(PERL) $(SRCDIR)/solenv/bin/exectest.pl \ + $(SRCDIR)/idlc/test/parser/conversion.tests \ + $(call gb_CustomTarget_get_workdir,idlc/parser_test)/in.idl 0 \ + $(call gb_Executable_get_command,idlc) \ + -O $(call gb_CustomTarget_get_workdir,idlc/parser_test) {} && \ $(PERL) $(SRCDIR)/solenv/bin/exectest.pl \ $(SRCDIR)/idlc/test/parser/interfaceinheritance.tests \ $(call gb_CustomTarget_get_workdir,idlc/parser_test)/in.idl 0 \ diff --git a/idlc/source/astexpression.cxx b/idlc/source/astexpression.cxx index 4ed35a8ef1f4..3da8db708d78 100644 --- a/idlc/source/astexpression.cxx +++ b/idlc/source/astexpression.cxx @@ -23,6 +23,7 @@ #include #include +#include #include #include @@ -159,8 +160,11 @@ coerce_value(AstExprValue *ev, ExprType t) } case ET_float: { - if (ev->u.fval < SAL_MIN_INT16 || ev->u.fval > SAL_MAX_INT16) + if (!(o3tl::convertsToAtLeast(o3tl::roundAway(ev->u.fval), SAL_MIN_INT16) + && o3tl::convertsToAtMost(o3tl::roundAway(ev->u.fval), SAL_MAX_INT16))) + { return false; + } auto tmp = static_cast(ev->u.fval); ev->u.sval = tmp; ev->et = ET_short; @@ -168,8 +172,11 @@ coerce_value(AstExprValue *ev, ExprType t) } case ET_double: { - if (ev->u.dval < SAL_MIN_INT16 || ev->u.dval > SAL_MAX_INT16) + if (!(o3tl::convertsToAtLeast(o3tl::roundAway(ev->u.dval), SAL_MIN_INT16) + && o3tl::convertsToAtMost(o3tl::roundAway(ev->u.dval), SAL_MAX_INT16))) + { return false; + } auto tmp = static_cast(ev->u.dval); ev->u.sval = tmp; ev->et = ET_short; @@ -245,8 +252,11 @@ coerce_value(AstExprValue *ev, ExprType t) } case ET_float: { - if (ev->u.fval < 0.0 || ev->u.fval > SAL_MAX_UINT16) + if (ev->u.fval < 0.0 + || !o3tl::convertsToAtMost(o3tl::roundAway(ev->u.fval), SAL_MAX_UINT16)) + { return false; + } auto tmp = static_cast(ev->u.fval); ev->u.usval = tmp; ev->et = ET_short; @@ -254,8 +264,11 @@ coerce_value(AstExprValue *ev, ExprType t) } case ET_double: { - if (ev->u.dval < 0.0 || ev->u.dval > SAL_MAX_UINT16) + if (ev->u.dval < 0.0 + || !o3tl::convertsToAtMost(o3tl::roundAway(ev->u.dval), SAL_MAX_UINT16)) + { return false; + } auto tmp = static_cast(ev->u.dval); ev->u.usval = tmp; ev->et = ET_short; @@ -327,8 +340,11 @@ coerce_value(AstExprValue *ev, ExprType t) } case ET_float: { - if (ev->u.fval < SAL_MIN_INT32 || ev->u.fval > SAL_MAX_INT32) + if (!(o3tl::convertsToAtLeast(o3tl::roundAway(ev->u.fval), SAL_MIN_INT32) + && o3tl::convertsToAtMost(o3tl::roundAway(ev->u.fval), SAL_MAX_INT32))) + { return false; + } auto tmp = static_cast(ev->u.fval); ev->u.lval = tmp; ev->et = ET_long; @@ -336,8 +352,11 @@ coerce_value(AstExprValue *ev, ExprType t) } case ET_double: { - if (ev->u.dval < SAL_MIN_INT32 || ev->u.dval > SAL_MAX_INT32) + if (!(o3tl::convertsToAtLeast(o3tl::roundAway(ev->u.dval), SAL_MIN_INT32) + && o3tl::convertsToAtMost(o3tl::roundAway(ev->u.dval), SAL_MAX_INT32))) + { return false; + } auto tmp = static_cast(ev->u.dval); ev->u.lval = tmp; ev->et = ET_long; @@ -411,8 +430,11 @@ coerce_value(AstExprValue *ev, ExprType t) } case ET_float: { - if (ev->u.fval < 0.0 || ev->u.fval > SAL_MAX_UINT32) + if (ev->u.fval < 0.0 + || !o3tl::convertsToAtMost(o3tl::roundAway(ev->u.fval), SAL_MAX_UINT32)) + { return false; + } auto tmp = static_cast(ev->u.fval); ev->u.ulval = tmp; ev->et = ET_ulong; @@ -420,8 +442,11 @@ coerce_value(AstExprValue *ev, ExprType t) } case ET_double: { - if (ev->u.dval < 0.0 || ev->u.dval > SAL_MAX_UINT32) + if (ev->u.dval < 0.0 + || !o3tl::convertsToAtMost(o3tl::roundAway(ev->u.dval), SAL_MAX_UINT32)) + { return false; + } auto tmp = static_cast(ev->u.dval); ev->u.ulval = tmp; ev->et = ET_ulong; @@ -489,8 +514,11 @@ coerce_value(AstExprValue *ev, ExprType t) } case ET_float: { - if (ev->u.fval < SAL_MIN_INT64 || ev->u.fval > SAL_MAX_INT64) + if (!(o3tl::convertsToAtLeast(o3tl::roundAway(ev->u.fval), SAL_MIN_INT64) + && o3tl::convertsToAtMost(o3tl::roundAway(ev->u.fval), SAL_MAX_INT64))) + { return false; + } auto tmp = static_cast(ev->u.fval); ev->u.hval = tmp; ev->et = ET_hyper; @@ -498,8 +526,11 @@ coerce_value(AstExprValue *ev, ExprType t) } case ET_double: { - if (ev->u.dval < SAL_MIN_INT64 || ev->u.dval > SAL_MAX_INT64) + if (!(o3tl::convertsToAtLeast(o3tl::roundAway(ev->u.dval), SAL_MIN_INT64) + && o3tl::convertsToAtMost(o3tl::roundAway(ev->u.dval), SAL_MAX_INT64))) + { return false; + } auto tmp = static_cast(ev->u.dval); ev->u.hval = tmp; ev->et = ET_hyper; @@ -571,8 +602,11 @@ coerce_value(AstExprValue *ev, ExprType t) } case ET_float: { - if (ev->u.fval < 0.0 || ev->u.fval > SAL_MAX_UINT64) + if (ev->u.fval < 0.0 + || !o3tl::convertsToAtMost(o3tl::roundAway(ev->u.fval), SAL_MAX_UINT64)) + { return false; + } auto tmp = static_cast(ev->u.fval); ev->u.uhval = tmp; ev->et = ET_uhyper; @@ -580,8 +614,11 @@ coerce_value(AstExprValue *ev, ExprType t) } case ET_double: { - if (ev->u.dval < 0.0 || ev->u.dval > SAL_MAX_UINT64) + if (ev->u.dval < 0.0 + || !o3tl::convertsToAtMost(o3tl::roundAway(ev->u.dval), SAL_MAX_UINT64)) + { return false; + } auto tmp = static_cast(ev->u.dval); ev->u.uhval = tmp; ev->et = ET_uhyper; @@ -850,8 +887,11 @@ coerce_value(AstExprValue *ev, ExprType t) return true; case ET_float: { - if (ev->u.fval < SAL_MIN_INT8 || ev->u.fval > SAL_MAX_UINT8) + if (!(o3tl::convertsToAtLeast(o3tl::roundAway(ev->u.fval), SAL_MIN_INT8) + && o3tl::convertsToAtMost(o3tl::roundAway(ev->u.fval), SAL_MAX_UINT8))) + { return false; + } auto tmp = static_cast(ev->u.fval); ev->u.byval = tmp; ev->et = ET_byte; @@ -859,8 +899,11 @@ coerce_value(AstExprValue *ev, ExprType t) } case ET_double: { - if (ev->u.dval < SAL_MIN_INT8 || ev->u.dval > SAL_MAX_UINT8) + if (!(o3tl::convertsToAtLeast(o3tl::roundAway(ev->u.dval), SAL_MIN_INT8) + && o3tl::convertsToAtMost(o3tl::roundAway(ev->u.dval), SAL_MAX_UINT8))) + { return false; + } auto tmp = static_cast(ev->u.dval); ev->u.byval = tmp; ev->et = ET_byte; diff --git a/idlc/test/parser/conversion.tests b/idlc/test/parser/conversion.tests new file mode 100644 index 000000000000..4f1d6bc7d3e9 --- /dev/null +++ b/idlc/test/parser/conversion.tests @@ -0,0 +1,79 @@ +# +# This file is part of the LibreOffice project. +# +# This Source Code Form is subject to the terms of the Mozilla Public +# License, v. 2.0. If a copy of the MPL was not distributed with this +# file, You can obtain one at http://mozilla.org/MPL/2.0/. +# + +EXPECT SUCCESS "conversion.tests 1": +constants C { + const byte C1 = -128.0; + const byte C2 = -127.9; + const byte C3 = 254.9; + const byte C4 = 255.0; + const short C5 = -32768.0; + const short C6 = -32767.9; + const short C7 = 32766.9; + const short C8 = 32767.0; + const unsigned short C9 = 0.0; + const unsigned short C10 = 0.1; + const unsigned short C11 = 65534.9; + const unsigned short C12 = 65535.0; + const long C13 = -2147483648.0; + const long C14 = -2147483647.9; + const long C15 = 2147483646.9; + const long C16 = 2147483647.0; + const unsigned long C17 = 0.0; + const unsigned long C18 = 0.1; + const unsigned long C19 = 4294967294.9; + const unsigned long C20 = 4294967295.0; + const hyper C21 = -9223372036854774784.0; // -0x7FFFFFFFFFFFFC00 = -0x1.FFFFFFFFFFFFFp62 + const hyper C22 = 9223372036854774784.0; // 0x7FFFFFFFFFFFFC00 = 0x1.FFFFFFFFFFFFFp62 + const unsigned hyper C23 = 0.0; + const unsigned hyper C24 = 0.1; + const unsigned hyper C25 = 18446744073709549568.0; // 0xFFFFFFFFFFFFF800 = 0x1.FFFFFFFFFFFFFp63 +}; + +EXPECT FAILURE "conversion.tests 2": +constants C { const byte C1 = -128.1; }; + +EXPECT FAILURE "conversion.tests 3": +constants C { const byte C1 = 255.1; }; + +EXPECT FAILURE "conversion.tests 4": +constants C { const short C1 = -32768.1; }; + +EXPECT FAILURE "conversion.tests 5": +constants C { const short C1 = 32767.1; }; + +EXPECT FAILURE "conversion.tests 6": +constants C { const unsigned short C1 = -0.1; }; + +EXPECT FAILURE "conversion.tests 7": +constants C { const unsigned short C1 = 65535.1; }; + +EXPECT FAILURE "conversion.tests 8": +constants C { const long C1 = -2147483648.1; }; + +EXPECT FAILURE "conversion.tests 9": +constants C { const long C1 = 2147483647.1; }; + +EXPECT FAILURE "conversion.tests 10": +constants C { const unsigned long C1 = -0.1; }; + +EXPECT FAILURE "conversion.tests 11": +constants C { const unsigned long C1 = 4294967295.1; }; + +EXPECT FAILURE "conversion.tests 12": +constants C { const hyper C1 = -9223372036854775808.0; }; // -0x8000000000000000 = -0x1p63 + +EXPECT FAILURE "conversion.tests 13": +constants C { const hyper C1 = 9223372036854775807.0; }; // 0x7FFFFFFFFFFFFFFF rounds up to 0x1p63 + +EXPECT FAILURE "conversion.tests 14": +constants C { const unsigned hyper C1 = -0.1; }; + +EXPECT FAILURE "conversion.tests 15": +constants C { const unsigned hyper C1 = 18446744073709551615.0; }; + // 0xFFFFFFFFFFFFFFFF rounds up to 0x1p64 diff --git a/include/o3tl/float_int_conversion.hxx b/include/o3tl/float_int_conversion.hxx new file mode 100644 index 000000000000..dfaea5ce3bd2 --- /dev/null +++ b/include/o3tl/float_int_conversion.hxx @@ -0,0 +1,57 @@ +/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4; fill-column: 100 -*- */ +/* + * This file is part of the LibreOffice project. + * + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. + */ + +#ifndef INCLUDED_O3TL_FLOAT_INT_CONVERSION_HXX +#define INCLUDED_O3TL_FLOAT_INT_CONVERSION_HXX + +#include + +#include +#include + +namespace o3tl +{ +// Return true iff `value` of floating-point type `F` converts to a value of integral type `I` no +// smaller than `min`: +template +std::enable_if_t && std::is_integral_v, bool> +convertsToAtLeast(F value, I min) +{ + // If `F(min)`, `F(min) - F(1)` are too large in magnitude for `F`'s precision, then they either + // fall into the same bucket, in which case we should return false if `value` represents that + // bucket, or they are on the boundary of two adjacent buckets, in which case we should return + // true if `value`represents the higher bucket containing `F(min)`: + return value > F(min) - F(1); +} + +// Return true iff `value` of floating-point type `F` converts to a value of integral type `I` no +// larger than `max`: +template +std::enable_if_t && std::is_integral_v, bool> +convertsToAtMost(F value, I max) +{ + // If `F(max)`, `F(max) + F(1)` are too large in magnitude for `F`'s precision, then they either + // fall into the same bucket, in which case we should return false if `value` represents that + // bucket, or they are on the boundary of two adjacent buckets, in which case we should return + // true if `value`represents the lower bucket containing `F(max)`: + return value < F(max) + F(1); +} + +// Return `value` of floating-point type `F` rounded to the nearest integer away from zero (which +// can be useful in calls to convertsToAtLeast/Most(roundAway(x), n), to reject x that are +// smaller/larger than n because they have a fractional part): +template std::enable_if_t, F> roundAway(F value) +{ + return value >= 0 ? std::ceil(value) : std::floor(value); +} +} + +#endif + +/* vim:set shiftwidth=4 softtabstop=4 expandtab cinoptions=b1,g0,N-s cinkeys+=0=break: */ -- cgit v1.2.3