[Dune-devel] PSA: please avoid implicit conversion integral_constant<bool, ...> to bool

[Carsten Gräser]

Hi Jö,

Am 21.02.2017 um 01:54 schrieb Jö Fahlke:
> Am Mon, 20. Feb 2017, 21:12:43 +0100 schrieb Carsten Gräser:
>> Hi Jö,
>>
>> Am 20.02.2017 um 13:10 schrieb Jö Fahlke:
>>> Hi!
>>>
>>> I've seen expression like `std::is_same<T1, T2>()` (or any other type
>>> predicate) where the result is implicitly converted to `bool` pop up
>>> throughout Dune for some time now.  Please stop that; they can't work in all
>>> cases, and when they break they lead to hard-to-diagnose errors.
>> the guideline to alway avoid this implicit conversion seems
>> to be a little to general to me. I don't see a problem if
>> you already have an object of type integral_constant<bool, ...>.
> 
> The problem with making exceptions to guidelines is that it makes them more
> complicated.  The distinction between a type trait and its BaseCharacteristic
> is easy to overlook -- picking your personal favorite and sticking with it for
> all `integral_constant`-like thing seems simpler to me.
this is why I object to this guideline and want to propose
another one. Blaming the very natural and useful cast of
integral_constant<T,k> to T just to avoid problems that
may occur in this context but have an unrelated origin is
not very helpful.

>> By the same argument one would not be allowed to use
>> v[Indices::_0] for e.g. a std::vector v because this
>> involves such an implicit conversion.
> 
> If I know it is a `std::vector` I cannot think of any reason to prefer an
> index constant literal over an integer literal for the subscript.  So I'm
> assuming you're thinking about code that does not know whether the container
> is a `std::vector` or actually something that needs to be subcripted with an
> index constant.
> 
> In that case you were right in calling it a guideline, and the rule about
> guidelines applies: you sometimes need to ignore them.  I can see no way
> around relying on the implicit conversion there.
I strongly object to this. Relying on the conversion is
a standard use case and the problem you mentioned is the
exception. This cast is one of the reasons why integral_constant
is to useful. See e.g. hybridutilitiestest.cc where this
is used extensively. Your rule essentially forbids such code.

>> Isn't the real guideline: Never use the conversion if the
>> expression may involve incomplete types?
> 
> That would work, but then you have to determine whether the expression may
> involve incomplete types.  Most of the time in template code you can't be sure
> that the passed-in types are sufficiently complete.
But you can be sure if you have a type or an object.
In case of the latter there is no problem whereas
you may want to play safe in case of the former.
This will lead to:

  As a rule of thumb try to avoid creating an object of type
  integral_constant<T,k> just to extract its value.

Best,
Carsten

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 455 bytes
Desc: OpenPGP digital signature
URL: <https://lists.dune-project.org/pipermail/dune-devel/attachments/20170221/dc6810bf/attachment.sig>