FS#1512 Replace type traits by their standard versions

mentioned in merge request dune-localfunctions!2 (merged)

mentioned in merge request dune-geometry!3 (merged)

mentioned in merge request dune-grid!23 (merged)

mentioned in merge request dune-istl!19 (closed)

mentioned in merge request pdelab/dune-pdelab!107 (merged)

mentioned in issue flyspray/FS#1512 (closed)

It might be worth mentioning that a reduction from e.g.

  template <class Base, class Derived>
  class DUNE_DEPRECATED_MSG("Use std::is_base_of instead!") IsBaseOf
  {
  public:
    enum {
      /** @brief True if Base is a base class of Derived. */
      value = std::is_base_of<Base, Derived>::value
    };
  };

to the shorter version (which is not entirely identical, but I take it the additional members wouldn't hurt)

  template <class Base, class Derived>
  using IsBaseOf DUNE_DEPRECATED_MSG("Use std::is_base_of instead!") = std::is_base_of<Base, Derived>;

will not work as expected: The latter never generates a deprecation notice for me, even though I think I put it in the right place.

Clang or GCC? For the latter I'd expect it to work, for the former there is a bug open for several years.

mentioned in commit dune-localfunctions@d6ea557c

mentioned in commit felix.gruber/dune-localfunctions@d6ea557c

@gruenich that's with clang.

I take it that's https://llvm.org/bugs/show_bug.cgi?id=17862 which you yourself reported? :)

mentioned in commit dune-geometry@2334a10b

mentioned in commit dune-grid@46c40f23

mentioned in commit felix.gruber/dune-grid@46c40f23

mentioned in commit felix.gruber/dune-typetree@c2c914c6

Added 1 commit:

69fad160 - deprecate all duplicates of std type traits

I wrote some thin wrapper classes inheriting publicly from the std type traits, to deprecate our using statements of them. (see commit 69fad160)

That should work on both GCC and Clang.

Thanks for the cleanup. If nobody objects (or is faster merging), I am going to merge this at the end of the week.

It seems clang is also unable to handle deprecation notes of this type. I wrote the following sample code:

#include <type_traits>

#ifdef USE_CXX14_ATTRIBUTES
#define DUNE_DEPRECATED_MSG(text) [[deprecated(# text)]]
#else
#define DUNE_DEPRECATED_MSG(text) __attribute__((deprecated(# text)))
#endif

namespace Dune {
  template<typename T>
  struct DUNE_DEPRECATED_MSG("Use std::remove_const instead!") remove_const
    : public std::remove_const<T> {};
}

int
main() {
  using TC = const int;
  using T = Dune::remove_const<TC>::type;

  T i = 3;
  ++i;
}

Here's what I get:

% g++ --version                                             
g++ (Debian 4.9.2-10) 4.9.2
[..]
% g++ -std=c++14 -Wall -Wextra foo.cc                       
% g++ -std=c++14 -Wall -Wextra foo.cc -DUSE_CXX14_ATTRIBUTES
%

% g++ --version
g++ (GCC) 5.2.0
[..]
% g++ -std=c++14 -Wall -Wextra foo.cc
foo.cc: In function ‘int main()’:
foo.cc:18:19: warning: ‘template<class T> struct Dune::remove_const’ is deprecated [-Wdeprecated-declarations]
   using T = Dune::remove_const<TC>::type;
                   ^
foo.cc:11:64: note: declared here
   struct DUNE_DEPRECATED_MSG("Use std::remove_const instead!") remove_const
                                                                ^
% g++ -std=c++14 -Wall -Wextra foo.cc -DUSE_CXX14_ATTRIBUTES
foo.cc: In function ‘int main()’:
foo.cc:18:19: warning: ‘template<class T> struct Dune::remove_const’ is deprecated [-Wdeprecated-declarations]
   using T = Dune::remove_const<TC>::type;
                   ^
foo.cc:11:64: note: declared here
   struct DUNE_DEPRECATED_MSG("Use std::remove_const instead!") remove_const
                                                                ^
%

% clang++ --version
clang version 3.7.1 (tags/RELEASE_371/final)
[..]
% clang++ -std=c++14 -Wall -Wextra foo.cc
% clang++ -std=c++14 -Wall -Wextra foo.cc -DUSE_CXX14_ATTRIBUTES
%

Probably we should deprecate the header altogether and just move the bits we need to some different header. Maybe typetraits.hh would be a suitable name…

Hmm, that's very unfortunate. What does however seem to work is the following:

#include <type_traits>

#ifdef USE_CXX14_ATTRIBUTES
#define DUNE_DEPRECATED_MSG(text) [[deprecated(# text)]]
#else
#define DUNE_DEPRECATED_MSG(text) __attribute__((deprecated(# text)))
#endif

namespace Dune {
  template<typename T>
  struct DUNE_DEPRECATED_MSG("Use std::remove_const instead!") remove_const
  {
    typedef DUNE_DEPRECATED_MSG("Use std::remove_const instead!") typename std::remove_const<T>::type type;
  };
}

int
main() {
  using TC = const int;
  using T = Dune::remove_const<TC>::type;

  T i = 3;
  ++i;
}

Then we get

% clang++ --version
clang version 3.5.0 (tags/RELEASE_350/final 216961)
[..]
% clang++ -std=c++14 -Wall -Wextra clang_deprecation.cc 
clang_deprecation.cc:20:37: warning: 'type' is deprecated: "Use
      std::remove_const instead!" [-Wdeprecated-declarations]
  using T = Dune::remove_const<TC>::type;
                                    ^
clang_deprecation.cc:13:103: note: 'type' has been explicitly marked deprecated
      here
  ...std::remove_const instead!") typename std::remove_const<T>::type type;
                                                                      ^
1 warning generated.

So maybe, instead of using inheritance to define the wrapper classes we could simply define a deprecated type or value inside the wrapper class.

As annoying as it is, I guess it'd be nice if you used that approach, @felix.gruber.

Added 1 commit:

30b1fb22 - [cleanup] make deprecation work on clang

OK, I've done it that way. For the value type traits I've used deprecated enums except for integral_constant, which uses a deprecated static constexpr to ensure type correctness. I've run some tests with clang and gcc and it seems to work as expected now.

I would love to accept this merge request but it fails for me when compiling the istl tests. Not compiled with Clang 3.7.0:

[ 70%] Building CXX object dune/istl/test/CMakeFiles/complexmatrixtest.dir/complexmatrixtest.cc.o
In file included from /temp/gruenich/dune/complete/dune-istl/dune/istl/test/complexmatrixtest.cc:15:
In file included from /temp/gruenich/dune/complete/dune-common/dune/common/fvector.hh:15:
/temp/gruenich/dune/complete/dune-common/dune/common/typetraits.hh:141:88: error: no type named 'type' in 'std::enable_if<false, std::complex<double> >'; 'enable_if' cannot be used to disable this declaration
    typedef DUNE_DEPRECATED_MSG("Use std::enable_if instead!") typename std::enable_if<B,T>::type type;
                                                                                       ^
/temp/gruenich/dune/complete/dune-istl/dune/istl/solvers.hh:1384:14: note: in instantiation of template class 'Dune::enable_if<false, std::complex<double> >' requested here
    typename enable_if<is_same<field_type,real_type>::value,T>::type conjugate(const T& t) {
             ^
/temp/gruenich/dune/complete/dune-istl/dune/istl/solvers.hh:1426:13: note: while substituting deduced template arguments into function template 'conjugate' [with T = std::complex<double>]
      dy = -conjugate(sn) * dx + cs * dy;
            ^
/temp/gruenich/dune/complete/dune-istl/dune/istl/solvers.hh:1288:13: note: in instantiation of member function 'Dune::RestartedGMResSolver<Dune::FieldVector<std::complex<double>, 10>, Dune::FieldVector<std::complex<double>, 10>,
      Dune::FieldVector<std::complex<double>, 10> >::applyPlaneRotation' requested here
            applyPlaneRotation(H[k][i],H[k+1][i],cs[k],sn[k]);
            ^
/temp/gruenich/dune/complete/dune-istl/dune/istl/test/complexmatrixtest.cc:61:77: note: in instantiation of member function 'Dune::RestartedGMResSolver<Dune::FieldVector<std::complex<double>, 10>,
      Dune::FieldVector<std::complex<double>, 10>, Dune::FieldVector<std::complex<double>, 10> >::apply' requested here
    Dune::RestartedGMResSolver<Dune::FieldVector<std::complex<double>,10> > realgmrestest(hilbertadapter,noprec,reduction,maxIter,maxIter,2);
                                                                            ^```

Added 1 commit:

52611705 - [bugfix] fix Dune::enable_if

Sorry for this mess. I fixed the deprecated enable_if implementation and the dune-istl tests compile for me with GCC 5.3.1.

Hmm, there is still something wrong with the enable_if as it throws deprecation warnings in dune-istl, but we use only the enable_if from std:: there.

I'll look into it tomorrow.

Added 1 commit:

a036590d - [bugfix] GCC was giving too many deprecation warnings for enable_if

@felix.gruber It seems the problem is that the new version (enabled here via NEW_VERSION) does not compile whereas the previous version does for this piece of code:

#include <iostream>
#include <type_traits>

namespace Dune {
#if NEW_VERSION
  template<bool B, class T = void>
  struct enable_if {
    typedef typename std::enable_if<B,T>::type type;
  };
#else
  template<bool B, class T = void>
  struct enable_if : public std::enable_if<B,T> {};
#endif
};

template <typename T>
typename Dune::enable_if<std::is_integral<T>::value, void>::type
f()
{
  std::cout << "is integral!" << std::endl;
}

template <typename T>
typename Dune::enable_if<!std::is_integral<T>::value, void>::type
f()
{
  std::cout << "is not integral!" << std::endl;
}

int
main()
{
  f<int>();
  f<long>();
  f<float>();
  f<double>();
}

The upside: This appears to make both GCC and Clang generate warnings:

#include <iostream>
#include <type_traits>

namespace Dune {
  template<bool B, class T = void>
  struct [[deprecated]] enable_if {}; // GCC warns                              

  template<class T>
  struct [[deprecated]] enable_if<true, T> { typedef T type; }; // Clang warns  
}

template <typename T>
typename Dune::enable_if<std::is_integral<T>::value, void>::type
f()
{
  std::cout << "is integral!" << std::endl;
}

template <typename T>
typename Dune::enable_if<!std::is_integral<T>::value, void>::type
f()
{
  std::cout << "is not integral!" << std::endl;
}

int
main()
{
  f<int>();
  f<long>();
  f<float>();
  f<double>();
}

If no problems arise, I'll merge this this weekend.

mentioned in commit pdelab/dune-pdelab@cc5cf09e

The last proposal for enable_if from @pipping seems to be the best so far, but still has one large drawback: it sometimes gives deprecation warnings even when using std::enable_if.

Take for example this code:

#include <iostream>
#include <type_traits>

#ifdef USE_CXX14_ATTRIBUTES
#define DUNE_DEPRECATED_MSG(text) [[deprecated(# text)]]
#else
#define DUNE_DEPRECATED_MSG(text) __attribute__((deprecated(# text)))
#endif

namespace Dune {
  template<bool B, class T = void>
  struct DUNE_DEPRECATED_MSG("Use std::enable_if instead!") enable_if
  {};

  template<class T>
  struct DUNE_DEPRECATED_MSG("Use std::enable_if instead!") enable_if<true,T>
  {
    typedef T type;
  };

  template <typename T>
  typename std::enable_if<std::is_integral<T>::value, void>::type
  f()
  {
    std::cout << "is integral!" << std::endl;
  }

  template <typename T>
  typename std::enable_if<!std::is_integral<T>::value, void>::type
  f()
  {
    std::cout << "is not integral!" << std::endl;
  }
}

int
main() {
  Dune::f<int>();
  Dune::f<long>();
  Dune::f<float>();
  Dune::f<double>();
}

Then GCC is spilling deprecation warnings even though we are only using std::enable_if.


$ g++ --version
g++ (GCC) 5.2.0
Copyright (C) 2015 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 -Wextra enable_if.cc 
enable_if.cc:16:61: warning: ‘template<bool B, class T> struct Dune::enable_if’ is deprecated [-Wdeprecated-declarations]
   struct DUNE_DEPRECATED_MSG("Use std::enable_if instead!") enable_if<true,T>
                                                             ^
enable_if.cc:12:61: note: declared here
   struct DUNE_DEPRECATED_MSG("Use std::enable_if instead!") enable_if
                                                             ^
$ g++ --std=c++14 -Wall -Wextra enable_if.cc -DUSE_CXX14_ATTRIBUTES
enable_if.cc:16:61: warning: ‘template<bool B, class T> struct Dune::enable_if’ is deprecated [-Wdeprecated-declarations]
   struct DUNE_DEPRECATED_MSG("Use std::enable_if instead!") enable_if<true,T>
                                                             ^
enable_if.cc:12:61: note: declared here
   struct DUNE_DEPRECATED_MSG("Use std::enable_if instead!") enable_if
                                                             ^

I currently don't know how we could fix this. Thus I would be very glad if anyone had an idea of how to implement this deprecated Dune::enable_if without triggering the deprecation warning when using std::enable_if.

GCC 5 appears to be a tad over-eager here. Even this piece of code triggers a warning (note how enable_if is defined but never used):

#ifdef USE_CXX14_ATTRIBUTES
#define DUNE_DEPRECATED_MSG(text) [[deprecated(# text)]]
#else
#define DUNE_DEPRECATED_MSG(text) __attribute__((deprecated(# text)))
#endif

namespace Dune {
  template<bool B, class T = void>
  struct DUNE_DEPRECATED_MSG("Use std::enable_if instead!") enable_if
  {};

  template<class T>
  struct enable_if<true,T>
  {
    typedef T type;
  };
}

The template specialisation "refers" to a deprecated (template) class in a way here that triggers a warning. This might be related to

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=33911#c16

At the same time, this does not trigger a warning at all:

#include <iostream>
#include <type_traits>

#ifdef USE_CXX14_ATTRIBUTES
#define DUNE_DEPRECATED_MSG(text) [[deprecated(# text)]]
#else
#define DUNE_DEPRECATED_MSG(text) __attribute__((deprecated(# text)))
#endif

namespace Dune {
  template<bool B, class T = void>
  struct enable_if
  {
  };

  template<class T>
  struct DUNE_DEPRECATED_MSG("Use std::enable_if instead!") enable_if<false,T>
  {
  };

  template<class T>
  struct DUNE_DEPRECATED_MSG("Use std::enable_if instead!") enable_if<true,T>
  {
    typedef T type;
  };

  template <typename T>
  typename Dune::enable_if<std::is_integral<T>::value, void>::type
  f()
  {
    std::cout << "is integral!" << std::endl;
  }

  template <typename T>
  typename Dune::enable_if<!std::is_integral<T>::value, void>::type
  f()
  {
    std::cout << "is not integral!" << std::endl;
  }
}

int
main() {
  Dune::f<int>();
  Dune::f<long>();
  Dune::f<float>();
  Dune::f<double>();
}

So it seems both clang and GCC are badly broken here in all kinds of ways. I'm no longer convinced that there's a way to make both emit a warning when one is needed without any false positives :(

In doubt, no false warnings! If somebody is not warned, they will get a compiler error after we removed the stuff. But I would hate to get several warnings for every compilation.

I second that. While it is unfortunate that we do not get deprecation warnings for every use of the current Dune::enable_if it would be by far more annoying to get false positives whenever we include typetraits.hh.

Thus, I would keep the code for Dune::enable_if as it currently is on my branch type_traits.

I agree. Those false positives would be terrible. They would quite possibly make users disable warnings altogether.

I've posted a comment here: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=33911#c18

Merged to master, thanks for replacing Dune's implementation by the C++ ones and taking care for a best-effort deprecation.

Status changed to closed

Is there a particular reason for the fact that ConstantVolatileTraits, Conversion and IsInteroperable are not deprecated? To me, it seems much better to use directly the standard type_traits instead of these wrappers.

By the way, ConstantVolatileTraits and IsInteroperable are not used anywhere while ConstantVolatileTraits is only used in few headers in dune-common and in dune-alugrid.

@marco.agnese:

Conversion already uses std::is_convertible and std::is_same internally. The one thing it adds is ::isTwoWay that's otherwise not available as a single expression. IsInteroperable is similar in that respect.

ConstantVolatileTraits looks like it could be replaced in a heartbeat, though. Update: It's not currently used in any of the dune modules that I have easy access to (in particular, none of the core modules).

@pipping The fact is that all these classes are never used. The only one used is Dune::Conversion<T1,T2>::exists which can be avoided with this merge request !37 (merged). Therefore I would directly deprecated all these methods.

What do you mean by never used? Have you checked third-party grids, PDELab, dune-FEM et al.?

I mean that is never used in the core modules. For what concern dune-fem, I have just replaced everywhere common/typetraits.hh with the standard type_traits. dune-alugrid also seems to use only Dune::Conversion<T1,T2>::exists. I don't know dune-pdelab since I don't use it. I would like to deprecate only the definitions which are directly available in the standard library and I would keep for example ::isTwoWay since it is not directly available.

update: more precisely I would deprecated all ConstantVolatileTraits,Conversion::exists and Conversion::sameType

By the way also AlwaysTrue and AlwaysFalse should be deprecated in favour of true_type and false_type. Also these classes are never used in the core modules/dune-fem/dune-alugrid (the only exception is AlwaysFalse which is used only once in dune-grid/dune/grid/common/partitionset.hh). I can prepare a merge request for this change.

There has been a discussion about AlwaysTrue and AlwaysFalse in flyspray/FS#1435 (closed) (see Oliver's and Martin's comments). Apparently, there is a slight difference in semantics between them and std::true_type, std::false_type.

@felix.gruber Thank you! I didn't remember about it :)

FS#1512 Replace type traits by their standard versions

Merge request reports

Activity