Maybe a bug of concept and vector

[scarsty]

These codes cannot be complied with clang 19, but of no problem with MSVC and GCC.

#include <iostream>
#include <string>
#include <vector>

class A
{
    std::string a;
    std::vector<A> v;

public:
    A(int i) { a = std::to_string(i); }

    int toInt() const { return atoi(a.c_str()); }

    void pushBack(const A& a) { v.push_back(a); }    // line 1

    template <typename T>
        requires requires(const A& a, T& t) { A2T(a, t); }    // line 2
    operator T() const
    {
        T t;
        A2T(*this, t);
        return t;
    }
};

inline void A2T(const A& a, int& f) { f = a.toInt(); }

int main()
{
    A a(20);
    int f = a;
    std::cout << f << std::endl;
    return 0;
}

The error is

<source>:43:9: error: no viable conversion from 'A' to 'int'
   43 |     int f = a;
      |         ^   ~
<source>:23:5: note: candidate template ignored: constraints not satisfied [with T = int]
   23 |     operator T() const
      |     ^
<source>:22:47: note: because 'A2T(a, t)' would be invalid: use of undeclared identifier 'A2T'
   22 |         requires requires(const A& a, T& t) { A2T(a, t); }
      |                                               ^
1 error generated.

But if one of the line 1 or 2 is removed, things become OK. I guess it might be a bug of concept, does anyone have other idea?

[llvmbot]

@llvm/issue-subscribers-clang-frontend

Author: SunTY (scarsty)

These codes cannot be complied with clang 19, but of no problem with MSVC and GCC.

#include <iostream>
#include <string>
#include <vector>

class A
{
    std::string a;
    std::vector<A> v;

public:
    A(int i) { a = std::to_string(i); }

    int toInt() const { return atoi(a.c_str()); }

    void pushBack(const A& a) { v.push_back(a); }    // line 1

    template <typename T>
        requires requires(const A& a, T& t) { A2T(a, t); }    // line 2
    operator T() const
    {
        T t;
        A2T(*this, t);
        return t;
    }
};

inline void A2T(const A& a, int& f) { f = a.toInt(); }

int main()
{
    A a(20);
    int f = a;
    std::cout << f << std::endl;
    return 0;
}

The error is

<source>:43:9: error: no viable conversion from 'A' to 'int'
   43 |     int f = a;
      |         ^   ~
<source>:23:5: note: candidate template ignored: constraints not satisfied [with T = int]
   23 |     operator T() const
      |     ^
<source>:22:47: note: because 'A2T(a, t)' would be invalid: use of undeclared identifier 'A2T'
   22 |         requires requires(const A& a, T& t) { A2T(a, t); }
      |                                               ^
1 error generated.

But if one of the line 1 or 2 is removed, things become OK. I guess it might be a bug of concept, does anyone have other idea?

[shafik]

edg agrees w/ clang: https://godbolt.org/z/9oM5sdfj8

Using --stdlib=libc++ does not change the results.

[cor3ntin]

It looks like lookup in requires expression doesn't happen in the correct context

[zyn0217]

I reduced it to which GCC also rejects-valid:

https://godbolt.org/z/T16PcMn44

struct A {
  A(int i) {}

  int toInt() const { return 42; }

  void pushBack(const A &a) { A value(*this); }

  template <typename T>
    requires requires(const A &a, T &t) { A2T(a, t); }
  operator T() const {
    return T();
  }
};

inline void A2T(const A &a, int &f) { f = 42; }

int main() {
  A a(20);
  int f = a;
}

Note the user-defined constructor is somehow important to trigger the bug. If we removed it, both clang and gcc would compile.

https://godbolt.org/z/MaaM6xdro

[zyn0217]

So this is related to the cache: when parsing pushBack(), it somehow tried to instantiate the conversion function but failed because A2T was not seen at that point. Soon while parsing main(), it tried to instantiate the conversion function again, but this time it hits the cached result, which indicates that A2T was unavailable!

[cor3ntin]

Gosh, is that a core issue? 😱

[zyn0217]

https://eel.is/c++draft/temp.constr.atomic#3.sentence-5

@cor3ntin

[shafik]

https://eel.is/c++draft/temp.constr.atomic#3.sentence-5

@cor3ntin

Am I missing something or should C be used someplace in the example?

[zygoloid]

Am I missing something or should C be used someplace in the example?

The C part and the S part are two separate examples, neither of which is about caching.

It looks like the issue is that A value(*this) considers not only the A(int) constructor but also the A copy constructor. The copy constructor would be viable if we could call A::operator int(). So we check the requirements for A::operator T with T = int, and we do that before A2T is declared, at which point it's not callable.

So Clang is right; this code is ill-formed, no diagnostic required, because the constraint satisfaction check for the requires expression on operator T is performed twice for T = int and gives different results in the two cases.

This might be fixed by @cor3ntin's P3606, or perhaps a small modification to it. I think this doesn't quite fit the current approach to P3606, which says to not consider function templates if we have an exact match among the functions, but I think it'd be consistent to also say that we don't try to form implicit conversion sequences for the non-template candidates if there's an exact match non-template candidate. @cor3ntin What do you think?

I think we also have a diagnostic quality issue here -- when using a cached concept failure from a different evaluation, it'd be good to note that, eg:

<source>:43:9: error: no viable conversion from 'A' to 'int'
   43 |     int f = a;
      |         ^   ~
<source>:23:5: note: candidate template ignored: constraints not satisfied [with T = int]
   23 |     operator T() const
      |     ^
<source>:22:47: note: because 'A2T(a, t)' would be invalid: use of undeclared identifier 'A2T'
   22 |         requires requires(const A& a, T& t) { A2T(a, t); }
      |                                               ^
<source>:19:36: note: computed when evaluating constraint from here
   36 |    void pushBack(const A& a) { v.push_back(a); }    // line 1
      |                                  ^
1 error generated.

... though maybe it'd be better still to say that

• If we're diagnosing no viable conversion outside of a SFINAE context
• And a candidate is not viable because constraints weren't satisfied
• And the constraints weren't satisfied due to a cached "unsatisfied" result
• And re-evaluating the constraint at the point of the query finds they are now satisfied

... then we produce a different diagnostic saying that the result of the constraint evaluation changed between the two points of evaluation.