This is an archive of the discontinued LLVM Phabricator instance.

[libc++] Try and prevent evaluation of `is_default_constructible` on tuples default constructor if it is not needed.
ClosedPublic

Authored by EricWF on Feb 11 2015, 3:07 PM.

Details

Summary

Currently parts of the SFINAE on tuples default constructor always gets evaluated even when the default constructor is never called or instantiated. This can cause a hard compile error when a tuple is created with types that do not have a default constructor. Below is a self contained example using a pair like class. This code will not compile but probably should.

#include <type_traits>

template <class T>
struct IllFormedDefaultImp {
    IllFormedDefaultImp(T x) : value(x) {}
    constexpr IllFormedDefaultImp() {}
    T value;
};

typedef IllFormedDefaultImp<int &> IllFormedDefault;

template <class T, class U>
struct pair
{
  template <bool Dummy = true,
    class = typename std::enable_if<
         std::is_default_constructible<T>::value
      && std::is_default_constructible<U>::value
      && Dummy>::type
    >
  constexpr pair() : first(), second() {}

  pair(T const & t, U const & u) : first(t), second(u) {}

  T first;
  U second;
};

int main()
{
  int x = 1;
  IllFormedDefault v(x);
  pair<IllFormedDefault, IllFormedDefault> p(v, v);
}

One way to fix this is to use Dummy in a more involved way in the constructor SFINAE. The following patch fixes these sorts of hard compile errors for tuple.

Diff Detail

Event Timeline

EricWF updated this revision to Diff 19785.Feb 11 2015, 3:07 PM
EricWF retitled this revision from to [libc++] Try and prevent evaluation of `is_default_constructible` on tuples default constructor if it is not needed..
EricWF updated this object.
EricWF edited the test plan for this revision. (Show Details)
EricWF added reviewers: mclow.lists, rsmith, K-ballo.
EricWF added a subscriber: Unknown Object (MLST).

The faulty code was mine. With the pair example, it is clear that all those is_default_constructible<T> are instantiated even when the default constructor is not. I think my intention when writing Dummy && is_default_constructible<_Tp>::value was to delay the instantiation of is_default_constructible<_Tp> as a compile-time optimization (which it fails to do), but not for correctness purposes.

However, looking at http://en.cppreference.com/w/cpp/types/is_default_constructible and then http://en.cppreference.com/w/cpp/concept/DefaultConstructible, I am unsure whether instantiating is_default_constructible<_Tp> should cause a hard error at all. If instantiating the default constructor in the expression

_Tp x{}

causes a compile-time error, should is_default_constructible<_Tp>::value be false or trigger a compile-time error?

Summary: At a glance, the fix seems correct in that it will properly delay the instantiation of the is_default_constructible<_Tp>s. However, is there a bug in is_default_constructible?

rsmith added inline comments.Feb 11 2015, 4:12 PM
include/type_traits
2649–2652

Is it worth generalizing / simpilfying this:

template <class _Tp, bool> struct __dependent : public _Tp {};

(Usage: __dependent<is_default_constructible<_Tp>, Dummy>::value)

It's not yet completely clear whether Clang's (and GCC's) behavior is right here; there's an active core issue on how to handle this case.

GCC 4.8 and clang actually do the exact same thing on the example
code. What behavior are you referring to exactly?

Could you explain why the dependent type trick actually works? I'm
unsure as to why clang thinks it can evaluate one set of template
arguments but not the other even though it has essentially the same
information when evaluating tuple().

Also is clang ever going to outsmart this trick and realize
dependent<is_default_constructible<_Tp>, Dummy> is not actually
dependent on anything in this context? (ie on a default constructor)

Thanks for your input

/Eric

EricWF updated this revision to Diff 19803.Feb 11 2015, 6:54 PM

Address Richard's comments. Create a generic __dependent_type template instead of using __dependent_is_default_constructible.

Could you explain why the dependent type trick actually works? I'm
unsure as to why clang thinks it can evaluate one set of template
arguments but not the other even though it has essentially the same
information when evaluating tuple().

Let's take the pair example:

template <class T, class U>
struct pair {
  template <bool Dummy = true,
    class = typename std::enable_if<
         std::is_default_constructible<T>::value
      && std::is_default_constructible<U>::value
      && Dummy>::type
    >
  constexpr pair() {}
};

Because T and U are known when pair<T, U> is instantiated, regardless of
whether the default constructor is instantiated too, the above is equivalent to

template <class T, class U>
struct pair {
  static constexpr bool T_is_default_constr = std::is_default_constructible<T>::value;
  static constexpr bool U_is_default_constr = std::is_default_constructible<U>::value;

  template <bool Dummy = true,
    class = typename std::enable_if<
         T_is_default_constr
      && U_is_default_constr
      && Dummy>::type
    >
  constexpr pair() {}
};

So Clang can (and does) instantiate the is_default_constructible<T> as soon
as it can, which is as soon as pair<T, U> is instantiated. What I wrote for
std::tuple would then just be equivalent to:

template <class ..._Tp>
struct tuple {
  static constexpr bool _Tp1_is_default_constr = std::is_default_constructible<_Tp1>::value;
                            ...
  static constexpr bool _Tpn_is_default_constr = std::is_default_constructible<_Tpn>::value;

  template <bool Dummy = true,
    class = typename std::enable_if<
         _Tp1_is_default_constr && Dummy
      &&            ...
      && _Tpn_is_default_constr && Dummy
    >::type
    >
  constexpr pair() {}
};

At least, that's my analysis but I could be wrong.

Also is clang ever going to outsmart this trick and realize
dependent<is_default_constructible<_Tp>, Dummy> is not actually
dependent on anything in this context? (ie on a default constructor)

That's a really good question. I think it could, in theory. That's because
a constructor does not have a name, so it is impossible to call e.g.

tuple<int, char>::tuple</* Dummy = */false>()

Hence, I think in theory the compiler could say "hey you have template
parameters but it is _impossible_ to specify something else than their
default value, so let's substitute the parameters now". I'm not sure
what is said about that in the standard though; perhaps someone can
shed light on this?

Louis

K-ballo added inline comments.Feb 12 2015, 4:39 PM
include/tuple
514

Nitpick, no need to name _Up here.

test/std/utilities/tuple/tuple.tuple/tuple.cnstr/default.pass.cpp
39

Could this constructor be made ill-formed in a more obvious way? For instance, would a static_assert dependent on T achieve the same effect?

EricWF updated this revision to Diff 20415.Feb 20 2015, 9:23 AM

Address @K-ballo's comments.

EricWF accepted this revision.Feb 20 2015, 6:31 PM
EricWF added a reviewer: EricWF.

Accepting. I think this is a candidate for post commit review.

include/type_traits
2649–2652

Probably.

This revision is now accepted and ready to land.Feb 20 2015, 6:31 PM
EricWF closed this revision.Feb 20 2015, 6:32 PM