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

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

I recommend picking whatever appears least ugly to you from the following
list, and to use it whenever you need the value of an `integral_constant`.

- `std::is_same<T1, T2>::value`

- `std::is_same<T1, T2>().value` or `std::is_same<T1, T2>{}.value`

- `std::is_same<T1, T2>()()` or `std::is_same<T1, T2>{}()`

- `(bool)std::is_same<T1, T2>()` or `(bool)std::is_same<T1, T2>{}` or other
  explicit casts

At the very least, please use it whenever you deal with `integral_constant`
results of type predicates or type property queries.

Rationale
=========

Trick Question
--------------

In the following program, will the `static_assert()` trigger, or will the
program compile?

```c++
#include <tuple>
#include <type_traits>

int main() {
  static_assert(!std::is_same<std::tuple<void>, int>(), "Strange...");
}

```

Spoiler
-------

You will get an error similar to the following:
```
g++ -Wall -std=c++14 -o test test.cc
In file included from test.cc:1:0:
/usr/include/c++/4.9/tuple: In instantiation of ‘struct std::_Head_base<0ul, void, false>’:
/usr/include/c++/4.9/tuple:231:12:   required from ‘struct std::_Tuple_impl<0ul, void>’
/usr/include/c++/4.9/tuple:390:11:   required from ‘class std::tuple<void>’
test.cc:5:54:   required from here
/usr/include/c++/4.9/tuple:134:17: error: forming reference to void
       constexpr _Head_base(const _Head& __h)
                 ^
```

Explanation
-----------

`std::is_same` is a proper class template, not a typedef template, so `!` will
use ADL to find candidate functions implementing it.  The set of namespaces
for ADL includes the set of namespaces associated with `std::tuple<void>`,
which in turn includes the set of namespaces associated with any base classes
of `std::tuple<void>`.  To determine the base classes, `std::tuple<void>` must
be completed, but you can't have a complete class with a `void` data member or
(which triggers the error here) a constructor taking a `const void &`
parameter.

Occurance
---------

I last came across this when writing a unit test for `Dune::TypeList` and
associated utilites.  `Dune::IsEmptyTypeList<T>` is a type predicate that is
supposed to tell you for any type `T` whether it is and empty typelist.
Except that if `T` is a class type, it needlessly tries to complete it, so
```
static_assert(!Dune::IsEmptyTypeList<std::tuple<void> >::value, "...");
```
does not work as intended.  Even though I use `::value` here, because
`IsEmptyTypeList` uses the implicit conversion to `bool` internally.


Regards,
Jö.

-- 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 811 bytes
Desc: Digital signature
URL: <https://lists.dune-project.org/pipermail/dune-devel/attachments/20170220/37bb606e/attachment.sig>