Re: [lmi] value_cast() with two arguments

[Vadim Zeitlin]

On Thu, 28 Jul 2022 02:00:39 +0000 Greg Chicares <gchicares@sbcglobal.net> 
wrote:

GC> My first thought was to write a failing unit test that duplicates
GC> the reported problem, but that seems to be impossible: even if we
GC> ask the compiler to give us an illegally-initialized bool, that's
GC> UB and it can do anything it pleases, such as giving us 'true'.

 FWIW in practice this is exactly what happens with bool variables in debug
MSVC builds: the debug CRT helpfully (no sarcasm intended!) initializes all
not initialized memory to 0xcd bytes, so when you see a bool with this
value under debugger you can immediately know that it wasn't initialized
(which is helpful), but the code using it still "works" just fine because
it's handled as if it were initialized to true.

GC> So, aside from unit tests (and antediluvian stuff that end users never
GC> see), that single call in 'any_member.hpp':
GC>     object_->*held_ = value_cast(s, object_->*held_);
GC> is the only reason for this two-argument value_cast<>() overload to exist.
GC> Thus, we should remove that overload, and replace that single line:
GC>     object_->*held_ = value_cast(s, object_->*held_);
GC>     object_->*held_ = value_cast<DESIRED_TYPE>(s);
GC> and then clean up anything not covered by 'make install', e.g., the
GC> any_member unit test.

 I'm not sure if it's really better to specify the DESIRED_TYPE explicitly
like this rather than just keeping letting the compiler deduce it, as was
done before.

GC> Clearly decltype() is the tool for this job because it doesn't evaluate
GC> its operand. This sounded easy in principle, but it took me a few tries
GC> to come up with something that seems to work:
GC> 
GC>      LMI_ASSERT(object_);
GC> -    object_->*held_ = value_cast(s, object_->*held_);
GC> +    object_->*held_ = 
value_cast<std::remove_reference_t<decltype(object_->*held_)>>(s);
GC>      return *this;
GC> 
GC> Does it really have to be that verbose? I suspect that it does:

 I suspect so too, if we insist on specifying the type.

GC> Is there any better way?

 IMO my original proposal is a better way, i.e. just keep this code as is,
but take a const reference rather than value in value_cast().

 If you find passing an unused reference just for the purpose of type
deduction really distasteful, what about something like

        set_to_value_with_cast(object_->*held, s);

that would take a non-const reference and set it to the provided value
after the cast?

GC> This:
GC>   template<typename To, typename From>
GC>   To stream_cast(From from, To = To())
GC> should also be reworked. If 'To' is bool, then at least "To()"
GC> isn't undefined behavior, but it's unnecessary to construct
GC> anything (even if the compiler would elide it) and there's no
GC> reason to restrict this to types that have a default ctor.

 I think that the unused object may be optimized away in production builds
and could check it if you're curious. In any case, using a const reference
rather than a value here wouldn't cost anything and would avoid a
potentially expensive copy even in a debug build.

 Regards,
VZ

pgpI4Bl6R6Zpy.pgp
Description: PGP signature