Re: [lmi] value_cast() with two arguments

[Greg Chicares]

On 7/28/22 13:39, Vadim Zeitlin wrote:
> On Thu, 28 Jul 2022 02:00:39 +0000 Greg Chicares <gchicares@sbcglobal.net> 
> wrote:
[...]
> 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);
[...]
>  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.

[...trying anyway...]

> GC> +    object_->*held_ = 
> value_cast<std::remove_reference_t<decltype(object_->*held_)>>(s);

[...yick...]

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

Agreed. [Taking it by value meant copying an uninitialized bool, hence UB.]
Committed and pushed. Thanks.

> 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.

Like much other old code, this should be redesigned rather than
incrementally improved. If we don't redesign it, we should try
not to touch it except to fix important defects. UB (above) is
an important defect. An unused bool that's explicitly default-
initialized, and probably optimized away, is unimportant.
That's why I decided it was wiser not to touch stream_cast<>().