We have now two ways to associate types to integers: Int2Type in misc and integral_constant in typetraits.hh. Can we maybe settle for one and get rid of the other?
I vote for getting rid of Int2Type because integral_constant is more flexible and part of the c++0x standard.
Designs
Child items ...
Show closed items
Linked items 0
Link issues together to show that they're related.
Learn more.
Int2Type was introduced in revision 1583 (2005-03-01)
integral_constant was introduced in revision (2010-05-28)
While I can see that the "new" integral_constant concept is more general (and includes a lot of syntactic sugar), it is also very much longer, since, e.g., Int2Type<2> becomes integral_constant<int,2>. Moreover, since integral_constant is pretty new, Int2Type will be used whereever this construct was previously needed. So, if I understand this correctly, we are talking about some code beautification (or uglification).
Before we vote on this, I would like to have someone responsible for the transition (and I don't mean just replacing the old construct by the new one but extensive testing as well as quick bug fixing).
integral_constant is more flexible and part of the c++0x standard
I would mark Int2Type as deprecated first, before removing anything. We are using this a lot.
I don't see any benefit for a change right now. Furthermore, since when is gcc support c++0x.
We might get in trouble with wolder compiler (or non-gnu) version.
So, I support to use integral_constant but I would also like to kepp Int2Type for a little bit longer.
[ I'm on holiday currently, so don't expect quick responses for the next 3 ]
[ weeks ]
I did look for a template with a behaviour like Int2Type in Dune before
introducing integral_constant -- apparently I didn't look hard enough, so
sorry for the confusion, I guess.
@christi: it is used in PDELab, once in MultiTypeTree (which can be easily
replaced by Int2Type) and several times in WeightedSumLocalOperator
(weightedsum.hh) and InstationarySumLocalOperator (sum.hh).
@Robert: When introducing integral_constant, I did it the same way as Dune
already does for several other components from C++0x: if the compiler supports
C++0x (e.g. "g++ -std=c++0x") introduce the class provided by the compiler
into the Dune namespace by a "using" declaration, otherwise ("g++ -ansi")
implement it ourselves.
I have used integral_constant mostly for TMP: where I had an integral template
parameter, bu I needed to instanciate a template metaprogram with a type
template parameter. As far as I can see, Martin has used Int2Type mostly for
differenciating different overloads of a function, probably because partial
specialization (or even total specialization) was impossible. Martins code is
all over the place (generic geometries, generic reference elements,
albertagrid, parts of localfunctions).
If we want to migrate from Int2Type to integral_constant we have to take these
overloads into account. As far as I can quickly see, the calls to the
overloaded functions always happen inside some set of cooperating classes and
are thus implementation details. So it should be relatively uncomplicated to
migrate to integral_constant. Of course it will still be tedious due to the
number of occurances of Int2Type. And I suppose that Martin and possibly
others are using Int2Type in their private modules, but I can't say anything
about that.
Migrating back to Int2Type is relatively easy. It would require doing boolean
operations with "int"s though, which makes it slightly ugly in my opinion. In
the end I would possibly opt to copy the definition integral_constant to
PDELab instead.
The third possibility is to keep both Int2Type and integral_constant. Since
both are really small classes this should not create too much overhead. It
has the two benefits that we do have a standard-conforming integral_constant,
while at the same time we don't have to migrate existing code.
So I guess my list of favorite options is currently like this (most favorite
first):
keep both (best of both worlds)
remove integral_constant (removing Int2Type more trouble than it's worth)
deprecate Int2Type
My favours are not very strong though, I really can live with any of the three
options.
I am against keeping both unless Int2Type really does something that integral_constant cannot do. Even though the classes are small and simple this kind of cruft accumulates and makes Dune more difficult to understand and maintain.
If we are to remove one class I vote for the removal of Int2Type, because it is less flexible, less well documented, and because integral_constant is part of an upcoming standard. Of course 'removal' means keeping the old class as deprecated for a grace period of a suitable duration.
In my opinion, the additional work needed to replace occurrences of Int2Type by integral_constant is not a strong argument. I volunteer to help.
Here is a patch against dune-common rev6272 to deprecate Int2Type. Not checking it in yet since it increases the number of warnings by an insane amount (see attached logfiles). It appears to be otherwise harmless, though.
As long as deprecating Int2Type yields warnings within the core modules, I am strictly against applying the patch. The main part of the work consists in finding all Int2Type uses and replacing them by the new construct (including testing!!!). The other part is trivial.
Since this seems to be a longer issue, I suggest using a branch for this.
No worry, I'm almost done with dune-grid. I think there are no uses of Int2Type in the core modules besides dune-grid, but I'll check again. As to dune-fem, Robert agreed to do the transition there (by copying the definition from dune-common to dune-fem)
OK, here is the patch against dune-grid rev7150. It is actually a collection of patches in an mbox. Compilation of dune-common and dune-grid yields pretty much the same output as before the transition, except for some line numbers and similiar stuff (compare pre.log.bz2 from three comments before with final.log.bz2. There are some more instances of Int2Type in localfunctions, so I won't commit yet.
OK, here is the patch for dune-localfunctions (in mbox format). Also included is another patch for dune-grid, which applies on top of the previous stack. The logs of building common, grid, and localfunctions without any patches and with all patches, including running make -k check in localfunctions are also attached; they are identical except for buildsystem noise. I' going to commit now.