Re: [lmi] MSVS compilation problem with 100_cents in ternary operator

[Greg Chicares]

On 5/18/21 12:33 AM, Vadim Zeitlin wrote:
> 
>  I've tried building the latest master with MSVS and it complains about
> using operator _cents in non-constant expression when it's used in the
> following line
> 
>         return InvariantValues().IsMec ? -100_cents : 100_cents;
> 
> of ihs_avsolve.cpp.
> 
>  I'm not sure if this is a compiler bug or the expression is really
> non-constant (which could be the case if it's only the whole expression and
> not each of the 2 subexpressions, that counts),

As you suggested, in commit b88a684a2, I've partially reverted 237cf7245:

-        return InvariantValues().IsMec ? -100_cents : 100_cents;
+        static constexpr currency C100 = 100_cents;
+        return InvariantValues().IsMec ? -C100 : C100;

I'm surprised by this: [dcl.constexpr/8] says

| An invocation of a constexpr function in a given context produces
| the same result as an invocation of an equivalent non-constexpr
| function in the same context in all respects [except for two
| points that don't matter here]

and I can't imagine msvc wouldn't allow us to call a non-immediate
function here--IOW, this should be okay:

  return InvariantValues().IsMec ? -foo() : foo();

The only reason why I had defined a local variable representing
100 cents prior to partially-reverted 237cf7245, i.e., in
commit 026214ceb:

-        return InvariantValues().IsMec ? -1.0 : 1.0;
+        static const currency C100 = from_cents(100); // one dollar
+        return InvariantValues().IsMec ? -C100 : C100;

was that the alternative
  return InvariantValues().IsMec ? -from_cents(100) : from_cents(100);
seemed too ugly.

>  Do you think we could do this or should I look more into this and, at
> least, ascertain whether it's really a compiler bug or not? To be honest,
> I'd prefer to make the simple change above to continue working on other,
> more useful, things right now, and while I'd understand if you didn't want
> to make the code uglier to just accommodate MSVS, the version above doesn't
> really seem to be that much worse than the current one to me, so I hope we
> could just use it.

In this one isolated instance, I don't mind. But does msvc similarly
dislike the changes that 237cf72453 made in 'loads_test.cpp'? E.g.:

-    loads_.monthly_policy_fee_   [mce_gen_curr] = 
std::vector<currency>(length, from_cents(525));
-    loads_.annual_policy_fee_    [mce_gen_curr] = 
std::vector<currency>(length, from_cents(100));
+    loads_.monthly_policy_fee_   [mce_gen_curr] = 
std::vector<currency>(length, 525_cents);
+    loads_.annual_policy_fee_    [mce_gen_curr] = 
std::vector<currency>(length, 100_cents);

If that's also a problem, then maybe we need a workaround for msvc,
such as:

#if defined MSVC_MACRO
  inline currency operator""_cents(unsigned long long int cents)
#else // !defined MSVC_MACRO
  consteval currency operator""_cents(unsigned long long int cents)
#endif