[Libreoffice-commits] core.git: idlc/CustomTarget_parser_test.mk idlc/source idlc/test include/o3tl

Stephan Bergmann (via logerrit) logerrit at kemper.freedesktop.org
Tue Aug 13 06:22:40 UTC 2019


 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(-)

New commits:
commit a3ee2c625ef760b4de7b0fd72e5810be04b4c19e
Author:     Stephan Bergmann <sbergman at redhat.com>
AuthorDate: Mon Aug 12 11:35:28 2019 +0200
Commit:     Stephan Bergmann <sbergman at redhat.com>
CommitDate: Tue Aug 13 08:22:07 2019 +0200

    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 <sbergman at redhat.com>

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 \
@@ -45,6 +46,11 @@ else
                 $(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 \
                 $(call gb_Executable_get_command,idlc) \
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 <astscope.hxx>
 #include <errorhandler.hxx>
 
+#include <o3tl/float_int_conversion.hxx>
 #include <osl/diagnose.h>
 
 #include <limits.h>
@@ -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<sal_Int16>(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<sal_Int16>(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<sal_uInt16>(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<sal_uInt16>(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<sal_Int32>(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<sal_Int32>(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<sal_uInt32>(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<sal_uInt32>(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<sal_Int64>(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<sal_Int64>(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<sal_uInt64>(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<sal_uInt64>(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<unsigned char>(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<unsigned char>(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 <sal/config.h>
+
+#include <cmath>
+#include <type_traits>
+
+namespace o3tl
+{
+// Return true iff `value` of floating-point type `F` converts to a value of integral type `I` no
+// smaller than `min`:
+template <typename F, typename I>
+std::enable_if_t<std::is_floating_point_v<F> && std::is_integral_v<I>, 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 <typename F, typename I>
+std::enable_if_t<std::is_floating_point_v<F> && std::is_integral_v<I>, 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 <typename F> std::enable_if_t<std::is_floating_point_v<F>, 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: */


More information about the Libreoffice-commits mailing list