[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[lmi] patch to remove unused argument from numeric_io_cast()

From: Vadim Zeitlin
Subject: [lmi] patch to remove unused argument from numeric_io_cast()
Date: Tue, 23 Mar 2010 23:30:51 +0100

 Hello (Greg),

 I'd like to ask you what do you think about the following patch:

--- a/numeric_io_cast.hpp
+++ b/numeric_io_cast.hpp
@@ -103,9 +103,6 @@
 /// of degenerate string capable of holding single-digit decimal
 /// integers as numerals.

-template<typename To, typename From>
-To numeric_io_cast(From, To = To());
 // A compile-time failure iff this template is ever instantiated is
 // desired, but the straightforward
@@ -352,7 +349,7 @@ struct numeric_converter<std::string, char const*>

 template<typename To, typename From>
-To numeric_io_cast(From from, To)
+To numeric_io_cast(From from)
     numeric_converter<To,From> converter;
     return converter.operator()(from);
--- a/numeric_io_test.cpp
+++ b/numeric_io_test.cpp
@@ -57,21 +57,15 @@ void test_interconvertibility

     T t0 = t;
     INVOKE_BOOST_TEST_EQUAL(s, numeric_io_cast<std::string>(t0   ), file, 
-    INVOKE_BOOST_TEST_EQUAL(s, numeric_io_cast<std::string>(t0, s), file, 
-    INVOKE_BOOST_TEST_EQUAL(v, numeric_io_cast             (s, t0), file, 

     T const t1 = t;
     INVOKE_BOOST_TEST_EQUAL(s, numeric_io_cast<std::string>(t1   ), file, 
-    INVOKE_BOOST_TEST_EQUAL(s, numeric_io_cast<std::string>(t1, s), file, 
-    INVOKE_BOOST_TEST_EQUAL(v, numeric_io_cast             (s, t1), file, 

     T const& t2 = t;
     INVOKE_BOOST_TEST_EQUAL(s, numeric_io_cast<std::string>(t2   ), file, 
-    INVOKE_BOOST_TEST_EQUAL(s, numeric_io_cast<std::string>(t2, s), file, 
-    INVOKE_BOOST_TEST_EQUAL(v, numeric_io_cast             (s, t2), file, 

     std::string const s0 = s;
-    INVOKE_BOOST_TEST_EQUAL(v, numeric_io_cast(s0, t), file, line);
+    INVOKE_BOOST_TEST_EQUAL(v, numeric_io_cast<T>(s0), file, line);

 // These tests generally assume IEC 60559 floating point. Hardware

? The very first change is immaterial, I removed the template function
declaration just because it doesn't seem to be needed, it could be kept if
you think it's useful (although I'd be curious to know why). The really
important change is the second one which removes the unused "To" argument
(and the changes to the test just remove the code which doesn't compile
(and so doesn't need to be tested) any longer).

 I hope you will accept this patch because the argument is really useless
here and I think that it's a leftover from the bad old days when a lot of
compilers didn't support explicitly choosing the specialization of the
template function to call and so you had to write horrors like

        numeric_io_cast(from, *static_cast<std::string *>(NULL))

instead of just


 However to the best of my knowledge all the compilers currently used by
LMI do support the standard syntax and the only compiler which doesn't
support that I know about is MSVC6 which we definitely don't plan to use.

 But the real reason I am submitting this patch is that I just lost more
than an hour hunting down mysterious crashes and heap corruptions in
MSVC9-compiled version of LMI. And it turns out that the presence of this
parameter (or probably its default value) somehow confuses MSVC9 so much
that it generates completely incorrect code for this function. The
disassembly just doesn't make any sense to me (and this is *without*
optimization) but what is certain is that it destroys a bogus std::string
instance, i.e. calls dtor for something that is not a valid std::string
object at all (and neither is some global object representing the default
value of the function parameter as might be expected -- it's just some
really strange address, which is offset from the address of the string
which is returned by the function (and which shouldn't be destroyed at all
anyhow, of course)). So without this patch LMI basically crashes as soon as
numeric_io_cast<std::string>() is called. With it, everything seems to work
ok. I can't run the full unit tests suite under MSVC but I did run it under
Linux/g++4 and don't see any regressions after applying this patch.


reply via email to

[Prev in Thread] Current Thread [Next in Thread]