[Dune-devel] convert `enum` to `static const int`

[Jö Fahlke]

Am Mi, 20. Sep 2017, 15:37:29 +0200 schrieb Steffen Müthing:
> Date: Wed, 20 Sep 2017 15:37:29 +0200
> From: Steffen Müthing <steffen.muething at iwr.uni-heidelberg.de>
> To: Jö Fahlke <jorrit at jorrit.de>
> Cc: Ansgar Burchardt <Ansgar.Burchardt at tu-dresden.de>,
>  dune-devel at lists.dune-project.org
> Subject: Re: [Dune-devel] convert `enum` to `static const int`
> X-Mailer: Apple Mail (2.3273)
> 
> 
> > Am 20.09.2017 um 15:23 schrieb Jö Fahlke <jorrit at jorrit.de>:
> > 
> > Am Mi, 20. Sep 2017, 13:58:41 +0200 schrieb Ansgar Burchardt:
> >> it looks like GCC has new warnings:
> >> 
> >>    ../dune/grid/test/checkintersectionit.hh:599:58: warning:
> >>    comparison between ‘enum Dune::Grid<[...]>::<unnamed>’
> >>    and ‘enum Dune::Grid<[...]>::<unnamed>’ [-Wenum-compare]
> >> 
> >> for the expression
> >> 
> >>    GridType::dimension == GridType::dimensionworld
> >> 
> >> Is there any downside to changing every constant that is currently a
> >> `enum` to a `static const int`?  (Minus deciding on the signedness, but
> >> that is unrelated to the `enum` -> `static const` conversion.)
> > 
> > It should probably be `constexpr`, not `const`, but other than that using
> > either `int` or `unsigned` is probably preferrable now.  Of course, then it is
> > _all_ about the signedness, because that is a problem that you did not have
> > with pure enums (or at least, it wasn't aggressively warned about).
> > 
> > Oh, and: `static constexpr` (or `static const`) variables are lvalues, while
> > enumerators were prvalues.  That means we need to define them (in addition to
> > initializing them) without violating the ODR.  Otherwise expressions like
> > `std::min(GridType::dimension, 2)` can produce linker errors.
> 
> That’s not a problem with "static constexpr" members, at least up to C++17. I’m
> not entirely sure about the situation with C++17, but there adding an "inline" should
> fix the problem (there’s a macro DUNE_INLINE_VARIABLE for that purpose).

It is a problem with c++14: http://coliru.stacked-crooked.com/a/ece51fe315f24c0e
It looks like it is not a problem if you compile with -O2.

I'm not sure whether it indeed standard-conforming in c++17.  g++-7.2 (coliru)
accepts it in c++17-mode, g++-5.4.1 (coliru) does not, neither does g++-6.3.0
(Debian stretch).

Regards,
Jö.


For reference http://coliru.stacked-crooked.com/a/ece51fe315f24c0e is:
======================================================================
#include <iostream>

struct C {
    static constexpr int i = 2;
};

template<class T>
T foo(const T &t)
{
    return t;
}

int main()
{
    std::cout << foo(C::i) << std::endl;
}
======================================================================
set -ex; g++ --version; g++ -std=c++14 -Wall -pedantic -pthread main.cpp; ./a.out

+ g++ --version

g++ (GCC) 7.2.0

Copyright (C) 2017 Free Software Foundation, Inc.

This is free software; see the source for copying conditions.  There is NO

warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.


+ g++ -std=c++14 -Wall -pedantic -pthread main.cpp

/tmp/ccqvkRgX.o: In function `main':

main.cpp:(.text+0x5): undefined reference to `C::i'

collect2: error: ld returned 1 exit status
======================================================================


-- 
Jorrit (Jö) Fahlke, Institute for Computational und Applied Mathematics,
University of Münster, Orleans-Ring 10, D-48149 Münster
Tel: +49 251 83 35146 Fax: +49 251 83 32729

A mathematician is a device to turn coffee into theorems.
-- Paul Erdős
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.dune-project.org/pipermail/dune-devel/attachments/20170920/f6877600/attachment.sig>