llvm / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies.
http://llvm.org
Other
28.03k stars 11.57k forks source link

[clang] The timing of checking whether a constraint is self-dependent is unclear #104592

Open zyn0217 opened 1 month ago

zyn0217 commented 1 month ago

This was uncovered while I was working on the implementation of CWG2369 in https://github.com/llvm/llvm-project/issues/54440; I ran into a libcxx test regression, and that test failure turned out to be a discrepancy in the implementation.

The pre-existing issue is as follows (thanks to @cor3ntin who helped to find out the nuance across GCC and Clang):


template <class Tp>
concept C = requires(Tp t) { f(t); };

struct S {};

// template <typename T = void>
auto f(S) { return 1; } // #1

template <class T>
  requires C<T>
auto f(T); // #2

S p;
int i = f(p);

(https://gcc.godbolt.org/z/vM61xxbKz)

Despite whether # 1 is a template, Clang would reject it, saying the evaluation of C<T> depends on itself. GCC would reject it if # 1 is a template, while it would accept it if # 1 is a non-template.

It appears that two compilers disagree on when the circular dependency check will happen. For GCC, non-templates seem to have a higher priority over the template argument deduction and hence it would accept the case speculatively.

The other issue exposed by my approach of CWG2369 is, for a similar case below:

template <class Tp>
concept partially_ordered_with = requires(Tp t, Tp u) { t <= u; };

struct time_point {};
struct leap_second {};

auto operator<=>(time_point, time_point) { return 1; } // #1

template <class Duration>
  requires partially_ordered_with<Duration>
auto operator<=>(leap_second, Duration); // #2

time_point process_end;
bool result = process_end >= process_end;

Clang currently accepts it, while this won't be the case with my patch because we try to pick up the best candidate while evaluating the constraint t <= u, and while checking # 2, it would be a substitution failure rather than a hard error because the function template substitution happens before the constraint evaluation. My patch swapped their orders, and therefore we would evaluate the constraint on # 2 again, hence a self-dependent error.

As exhibited above, it is unclear to us whether we should accept the non-template early so we could avoid some cases that would otherwise result in a self-dependent error.

CC @zygoloid @cor3ntin for some insights.

llvmbot commented 1 month ago

@llvm/issue-subscribers-clang-frontend

Author: Younan Zhang (zyn0217)

This was uncovered while I was working on the implementation of CWG2369 in https://github.com/llvm/llvm-project/issues/54440; I ran into a libcxx test regression, and that test failure turned out to be a discrepancy in the implementation. The pre-existing issue is as follows (thanks to @cor3ntin who helped to find out the nuance across GCC and Clang): ```cpp template <class Tp> concept C = requires(Tp t) { f(t); }; struct S {}; // template <typename T = void> auto f(S) { return 1; } // #1 template <class T> requires C<T> auto f(T); // #2 S p; int i = f(p); ``` (https://gcc.godbolt.org/z/vM61xxbKz) Despite whether # 1 is a template, Clang would reject it, saying the evaluation of `C<T>` depends on itself. GCC would reject it if # 1 is a template, while it would accept it if # 1 is a non-template. It appears that two compilers disagree on when the circular dependency check will happen. For GCC, non-templates seem to have a higher priority over the template argument deduction and hence it would accept the case speculatively. The other issue exposed by my approach of CWG2369 is, for a similar case below: ```cpp template <class Tp> concept partially_ordered_with = requires(Tp t, Tp u) { t <= u; }; struct time_point {}; struct leap_second {}; auto operator<=>(time_point, time_point) { return 1; } // #1 template <class Duration> requires partially_ordered_with<Duration> auto operator<=>(leap_second, Duration); // #2 time_point process_end; bool result = process_end >= process_end; ``` Clang currently accepts it, while this won't be the case with my patch because we try to pick up the best candidate while evaluating the constraint `t <= u`, and while checking # 2, it would be a substitution failure rather than a hard error because the function template substitution happens before the constraint evaluation. My patch swapped their orders, and therefore we would evaluate the constraint on # 2 again, hence a self-dependent error. As exhibited above, it is unclear to us whether we should accept the non-template early so we could avoid some cases that would otherwise result in a self-dependent error. CC @zygoloid @cor3ntin for some insights.
cor3ntin commented 1 month ago

For the chrono issue https://cplusplus.github.io/LWG/issue4139