Re: [lmi] Cating doubles to enums

[Greg Chicares]

On 2018-11-02 15:20, Vadim Zeitlin wrote:
> 
>  Building the latest sources with MSVS 2017 results in
> 
> input_harmonization.cpp(101): error C2440: 'static_cast': cannot convert from 
> 'double' to 'mcenum_ledger_type'
> input_harmonization.cpp(101): note: Conversions between enumeration and 
> floating point values are no longer allowed
> 
> and many other similar errors in the same file as well as in gpt_input.cpp,
> ihs_acctval.cpp, ihs_basicval.cpp, ledger_invariant_init.cpp and
> mec_input.cpp.

That set of files sounds like an exact match for
  "static_cast directly from double to enum"
  git log b48b07fd --stat

>  Somewhat to my surprise, I confirmed that casting floating point values to
> (complete) enum values is indeed valid in C++ but nevertheless MSVS seems
> to intentionally disallow this.

I believe I've followed the standard, as quoted in the commit message:

| A value of floating-point type can also be explicitly converted to an
| enumeration type.

so this seems to be plainly a defect in their compiler.

>  This is, of course, a big problem for me, so I'd like to fix it in some
> way. I can make MSVS-specific changes on my local branch, of course, but
> this is going to be rather painful considering the number of such casts and
> the fact that they will probably keep changing in the future.

I don't think these lines of code change often. And I have the
impression that MS have begun to take standard compliance more
seriously than they once did, so they'll presumably fix this.
But meanwhile, it's an actual problem for you, and I suppose we
could temporarily revert that commit if there's no other way.
First of all, though, is there some #pragma we could use to
disable it, preferably on a global basis (say, OAOO, in a
PCH header)?

>  So I wonder if you would consider adding some kind of enum_cast<> function
> which would cast a double to int first using bourn_cast<> and then,
> possibly after checking the validity of the int as an enum element, cast it
> to the target type using static_cast<>? This would solve my problem, which
> is the immediate reason for proposing it, but would also IMO be slightly
> more readable and potentially safer.
I'm guessing that b48b07fd is where this arose; and it looks like
every line changed by that commit contains a call to Query(); so
I think the proper solution is to "overload" Query(). "Overload"
is probably the wrong word, because what I have in mind is adding
a template argument. Given...

    oenum_waiver_charge_method   WaiverChargeMethod;

improve this b48b07fd change...

-    WaiverChargeMethod  = 
static_cast<oenum_waiver_charge_method>(static_cast<int>(Database_->Query(DB_WpChargeMethod)));
+    WaiverChargeMethod  = 
static_cast<oenum_waiver_charge_method>(Database_->Query(DB_WpChargeMethod));

...thus:

-   WaiverChargeMethod  = 
static_cast<oenum_waiver_charge_method>(Database_->Query(DB_WpChargeMethod));
+   Database_->Query(WaiverChargeMethod, DB_WpChargeMethod);

and ultimately, because Query() is probably always invoked on
BasicValues::Database_,

+   Query(WaiverChargeMethod, DB_WpChargeMethod);

for a new member function BasicValues::Query(...) that would forward
to product_database::Query(...), which would have to become, e.g.:

     double Query(e_database_key) const;
     double Query(e_database_key, database_index const&) const;
+    template<typename T>
+    void Query(T& t, e_database_key) const;
     void Query(std::vector<double>&, e_database_key) const;
     void Query(std::vector<double>&, e_database_key, database_index const&) 
const;

where the new function would, as you suggest, first bourn_cast
from double for safety, and then...by the most appropriate method,
which is not immediately apparent to me...cast to the right enum type.
And if MSVS fails to support the most appropriate standard method,
then a workaround in one single place (the Query<>() implementation)
wouldn't be too terrible.

Sound good?