[PATCH] D133052: [clang] Avoid crash when expanding conversion templates in concepts.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

It seems wrong to change semantics of initialization when instantiating concept requirements. It implies that semantic checking may behave differently inside requires expressions, this is a red flag.
Clang has a mechanism <https://github.com/llvm/llvm-project/blob/84d07d021333f7b5716f0444d5c09105557272e0/clang/lib/Sema/SemaOverload.cpp#L7429> to prevent recursion when considering user-defined conversion for copy initialization.

Currently the intention of the Clang code path that handles this case is to:

1. Deduce `TO` to be `Deferred`
2. Try to check `Numeric<Deferred>`,
3. Check conversions during overload resolution for `foo(a)`. Go to step 1, <infinite recursion, so we never get to the next step>
4. <never reached> Check <https://github.com/llvm/llvm-project/blob/84d07d021333f7b5716f0444d5c09105557272e0/clang/lib/Sema/SemaOverload.cpp#L7429> that conversion operator converts the type to itself and mark the candidate as failed.

If move the check in step 4 to happen before step 3 (even if we need to duplicate the check), we will get the desired semantics.

Does that look reasonable?

I also agree there is a more general issue with recursive concept instantiations at play here, e.g. for code like

  template <class T> concept A = requires (T a) { foo(a); };
  template <class T> concept B = requires (T a) { bar(a); };
  
  struct X {};
  template <A T> void bar(T);
  template <B T> void foo(T);
  
  void test() {
      foo(X());
  }

Clang will currently cut this out because the template instantiation depth is too high, whereas GCC will provide a useful diagnostics that says concept satisfaction computation is recursive.
We probably want a more informative error message too. Probably worth investigating separately from that particular change.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D133052/new/

https://reviews.llvm.org/D133052