llvm / llvm-project

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

[Clang][Sema][Concepts] Clang can't choose from a member and a non member operator #82509

Closed HoBoIs closed 8 months ago

HoBoIs commented 8 months ago

Consider the following code:

struct s{
    template<class T>
    int operator+(const T&)const{return 0;}
};

template<class T>
concept AlwaysTrue=true;

template<AlwaysTrue T>
int operator+(const s&,const T&){return 1;}

int main(){
    return s{}+1;
}

godbolt example: https://godbolt.org/z/vG31vjxbn Since the 2nd operator+ is more constrained clang should choose that. However clang reports an ambiguity. (GCC chooses the 2nd one as expected)
Note that the ambiguity remains if the only 1st, or both declaration have a concept, (even if one of them is more constrained). If we define both operators inside the class or both operators outside the class clang chooses the correct overload.

llvmbot commented 8 months ago

@llvm/issue-subscribers-clang-frontend

Author: Botond István Horváth (HoBoIs)

Consider the following code: ``` struct s{ template<class T> int operator+(const T&)const{return 0;} }; template<class T> concept AlwaysTrue=true; template<AlwaysTrue T> int operator+(const s&,const T&){return 1;} int main(){ return s{}+1; } ``` godbolt example: https://godbolt.org/z/vG31vjxbn Since the 2nd `operator+` is more constrained clang should choose that. However clang reports an ambiguity. (GCC chooses the 2nd one as expected) Note that the ambiguity remains if the only 1st, or both declaration have a concept, (even if one of them is more constrained). If we define both operators inside the class or both operators outside the class clang chooses the correct overload.
llvmbot commented 8 months ago

@llvm/issue-subscribers-c-20

Author: Botond István Horváth (HoBoIs)

Consider the following code: ``` struct s{ template<class T> int operator+(const T&)const{return 0;} }; template<class T> concept AlwaysTrue=true; template<AlwaysTrue T> int operator+(const s&,const T&){return 1;} int main(){ return s{}+1; } ``` godbolt example: https://godbolt.org/z/vG31vjxbn Since the 2nd `operator+` is more constrained clang should choose that. However clang reports an ambiguity. (GCC chooses the 2nd one as expected) Note that the ambiguity remains if the only 1st, or both declaration have a concept, (even if one of them is more constrained). If we define both operators inside the class or both operators outside the class clang chooses the correct overload.
shafik commented 8 months ago

CC @erichkeane wdyt?

erichkeane commented 8 months ago

I suspect our comparison of constraints isn't right here, I don't think we're actually comparing them at all, see: https://godbolt.org/z/WqddPoc7M

We are checking for 'equivalent', but we are probably missing a tie-breaker rule in overload resolution for some reason.

zygoloid commented 8 months ago

It looks to me like Sema::getMoreSpecializedTemplate is not properly handling the case of comparing a member against a non-member -- the X(M) handling from [temp.func.order]/3 is missing here, and parameters are incorrectly being compared simply by index. The Reversed flag is also mishandled.

isAtLeastAsSpecializedAs handles this; maybe that computation can be factored out and shared.

HoBoIs commented 8 months ago

It seems like the current handling is for cpp17, patched a bit so cpp20 reversed operators are working. I'm working on a patch (I'm almost ready), and there is an other part of the problem. Since we started from the Function member of the OverloadCandidate -s we lost the info on the type of the object parameter, provided we have a using like this:

struct base{
  template<AlwaysTrue T>
  void foo(T);
};
struct derived:base{
  using base::foo;
  template<class T>
  void foo(T);
};
int f(){
  derived d;
  d.foo(1);
}

the object parameter will still be base in the function template. Also we have to be careful if there is an explicit object parameter. The standard does not mention it, but it makes no sense to add it again to the parameter list even thou it is already in the list. eg.:

struct S{
  template<class T>
  void foo(T);//#a
  template<AlwaysTrue T>
  void foo(this S self, T);//#b
};
...
S s;
s.foo(0);

If we follow the standard to the letter we would have 'S,int' parameters for function #a and 'S,S,int' for function #b. I think it is intended to have only 'S,int' parameters.