rsmith added a comment.

Ouch. This testcase is horrible. Note that we find two different ways to 
convert `Optional<std::tuple<dynamic>>` to itself: the obvious way, and 
`Optional<std::tuple<dynamic>> -> dynamic -> std::tuple<dynamic> -> 
Optional<std::tuple<dynamic>>` (because `tuple`'s conversion happens inside its 
converting constructor, this sidesteps the "at most one user-defined 
conversion" restriction). The reason for the problem is that when overload 
resolution explores the second path, libc++'s SFINAE checks enter a cycle, and 
`is_convertible`'s result for this conversion ends up defined in terms of 
itself.

Now, there *is* a Clang bug here -- a `constexpr` function template 
specialization can trigger its own recursive instantiation. Reduced reproducer:

  template<typename T> constexpr int f(T t) { return g(t); }
  template<typename T> constexpr int g(T t) { return f(t); }
  struct X {};
  constexpr int k = f(X());

With that fixed, you'll get an arbitrary answer from `is_convertible` here 
rather than a compile-time error, but the root cause of the problem in this 
testcase (the cycle) will remain and will result in weird misbehavior in some 
cases. Consider the following:

  template <typename T, typename U> struct is_convertible { static const bool 
value = __is_convertible_to(T, U); };
  template<bool> struct enable_if; template<> struct enable_if<true> { using 
type = void; };
  
  template<typename T> struct tuple {
    // tuple::tuple(U&&...) requires each T is constructible from the 
corresponding U
    template<typename U, typename = typename enable_if<is_convertible<U, 
T>::value>::type> tuple(U);
  };
  template<typename T> struct optional {
    // optional(const T&) requires T is copy-constructible, but that doesn't 
affect our example
    optional(T);
  };
  struct any {
    // any::any(ValueType&&) requires decay_t<ValueType> is CopyConstructible.
    template<typename T, typename = typename enable_if<is_convertible<T, 
T>::value>::type> any(T);
  };
  
  #ifdef WRONG
  static_assert(is_convertible<optional<tuple<any>>, 
optional<tuple<any>>>::value == true);
  static_assert(is_convertible<optional<tuple<any>>, any>::value == false);
  #else
  static_assert(is_convertible<optional<tuple<any>>, any>::value == true);
  static_assert(is_convertible<optional<tuple<any>>, 
optional<tuple<any>>>::value == true);
  #endif

This testcase has essentially the same character as the original one, except 
that I cut out a lot of irrelevant complexity and explicitly added a use of 
`is_convertible` to `any` to make the problem easier to observe (and renamed 
the classes to match the names in the standard library). Note that:

- instantiating `is_convertible<o<t<a>>, o<t<a>>>` triggers instantiation of 
`is_convertible<o<t<a>>, a>`, because it tries to construct a `t<a>` from an 
`o<t<a>>`, which tries to use the `tuple(U)` constructor with `U = o<t<a>>`
- instantiating `is_convertible<o<t<a>>, a>` triggers instantiation of 
`is_convertible<o<t<a>>, o<t<a>>>`, because it tries to construct a `a` from an 
`o<t<a>>`

Now, depending on where we enter this cycle, we get different answers for the 
type traits, because when we get back to our starting point in the cycle, we 
get a SFINAE failure (because we've not instantiated `::value` yet) and the 
previous `is_convertible` step will evaluate to `false` if there's no other way 
to perform the conversion. (In the `-UWRONG` case, there's another way to 
convert `o<t<a>>` to `o<t<a>>`, but in the `-DWRONG` case there is no other way 
to convert `o<t<a>>` to `a`.)

Arguably the bug here is in the definition of class `optional`, which triggered 
overload resolution from `optional` to `T` when constructing from an argument 
of type `optional`... but it seems unreasonable to expect `optional` to avoid 
that (and likewise for user-defined types like `Optional` in the testcase for 
this patch).


https://reviews.llvm.org/D23999



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to