Bug 107461 - [12 Regression] ambiguity error for friend with templated constexpr argument
Summary: [12 Regression] ambiguity error for friend with templated constexpr argument
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: c++ (show other bugs)
Version: 13.0
: P1 normal
Target Milestone: 12.3
Assignee: Patrick Palka
URL:
Keywords: rejects-valid
: 109001 (view as bug list)
Depends on:
Blocks:
 
Reported: 2022-10-30 08:17 UTC by Jason Liam
Modified: 2023-03-03 14:12 UTC (History)
6 users (show)

See Also:
Host:
Target:
Build:
Known to work: 11.3.0
Known to fail: 12.2.0, 13.0
Last reconfirmed: 2022-10-30 00:00:00


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Jason Liam 2022-10-30 08:17:51 UTC
The following well-formed program(afaik) is rejected by gcc but accepted by clang and msbc. Demo: https://godbolt.org/z/bMWxd8bEa

```
#include <iostream>
#include <type_traits>
#include <vector>
#include <string>
#include <functional>
#include <map>
#include <algorithm>
using namespace std;
#include <algorithm>
#include <array>
#include <type_traits>
#include <vector>

namespace MLL{
  template<typename data_t, std::size_t n_rows, std::size_t n_cols, std::size_t MAX=256>
    class Matrix;    //this forward declaration not needed but you can have this if you want and if you do then make sure that you only provide the default arg 256 in declaration and not in definition
//--------------------------------------------------------------------------------vvvvvvv---->added this default arg here instead of in forward declaration
    template<typename data_t, std::size_t n_rows, std::size_t n_cols, std::size_t MAX>
    class Matrix{
        static constexpr bool IS_STATIC = n_rows * n_cols <= MAX;
        using container_t = typename std::conditional<IS_STATIC, std::array<data_t, n_rows * n_cols>, std::vector<data_t>>::type;

        container_t m_data_list;

    public:
        Matrix(){
            if constexpr( !IS_STATIC ){
                m_data_list.resize(n_rows * n_cols);
            }
        }

        explicit Matrix(data_t default_value){
            if constexpr( IS_STATIC ){
                m_data_list.fill(default_value);
            }else{
                m_data_list.resize(n_rows * n_cols, default_value);
            }
        }

        explicit Matrix(std::initializer_list<data_t>&& value_list){
            std::copy(value_list.begin(), value_list.end(), m_data_list.begin());
        }

        Matrix(Matrix const& other)
                : m_data_list(other.m_data_list){
        }

        Matrix(Matrix&& other) noexcept
                : m_data_list(std::move(other.m_data_list)){
        }

        Matrix& operator=(Matrix const& other){
            m_data_list = other.m_data_list;
            return *this;
        }

        Matrix& operator=(Matrix&& other) noexcept{
            m_data_list = std::move(other.m_data_list);
            return *this;
        }

        //renamed all the arguments by prefexing them with OP for better readibility
        template<typename data_tOP, typename TOP, std::size_t n_rowst, std::size_t n_colsOP, std::size_t MAXOP, std::size_t other_MAXOP>
    friend Matrix<decltype(std::declval<data_tOP>() + std::declval<TOP>()), n_rowst, n_colsOP, std::min(MAXOP, other_MAXOP)>
    operator+(Matrix<data_tOP, n_rowst, n_colsOP, MAXOP> const& lhs, Matrix<TOP, n_rowst, n_colsOP, other_MAXOP> const& rhs);
    };

     template<typename data_tOP, typename TOP, std::size_t n_rowst, std::size_t n_colsOP, std::size_t MAXOP, std::size_t other_MAXOP>
    Matrix<decltype(std::declval<data_tOP>() + std::declval<TOP>()), n_rowst, n_colsOP, std::min(MAXOP, other_MAXOP)>
    operator+(Matrix<data_tOP, n_rowst, n_colsOP, MAXOP> const& lhs, Matrix<TOP, n_rowst, n_colsOP, other_MAXOP> const& rhs){
         const std::size_t n = n_rowst * n_colsOP;
        for( std::size_t i = 0; i < n; ++i ){
           // lhs.m_data_list[i] += rhs.m_data_list[i];   //can't assing using const lvalue reference
        }
        return lhs;
    }
}
int main()
{
    
    MLL::Matrix<int, 4, 4> a({1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16});
    a+a;
    return 0;;
}

```

GCC rejects this with the error:

```
<source>:82:6: error: ambiguous overload for 'operator+' (operand types are 'MLL::Matrix<int, 4, 4>' and 'MLL::Matrix<int, 4, 4>')
   82 |     a+a;
      |     ~^~
      |     | |
      |     | Matrix<[...],[...],[...]>
      |     Matrix<[...],[...],[...]>
<source>:70:5: note: candidate: 'MLL::Matrix<decltype ((declval<data_tOP>() + declval<TOP>())), n_rowst, n_colsOP, std::min<long unsigned int>(MAXOP, other_MAXOP)> MLL::operator+(const Matrix<data_tOP, n_rowst, n_colsOP, MAXOP>&, const Matrix<TOP, n_rowst, n_colsOP, other_MAXOP>&) [with data_tOP = int; TOP = int; long unsigned int n_rowst = 4; long unsigned int n_colsOP = 4; long unsigned int MAXOP = 256; long unsigned int other_MAXOP = 256; decltype ((declval<data_tOP>() + declval<TOP>())) = int]'
   70 |     operator+(Matrix<data_tOP, n_rowst, n_colsOP, MAXOP> const& lhs, Matrix<TOP, n_rowst, n_colsOP, other_MAXOP> const& rhs){
      |     ^~~~~~~~
<source>:65:5: note: candidate: 'MLL::Matrix<decltype ((declval<data_tOP>() + declval<TOP>())), n_rowst, n_colsOP, std::min<long unsigned int>(MAXOP, other_MAXOP)> MLL::operator+(const Matrix<data_tOP, n_rowst, n_colsOP, MAXOP>&, const Matrix<TOP, n_rowst, n_colsOP, other_MAXOP>&) [with data_tOP = int; TOP = int; long unsigned int n_rowst = 4; long unsigned int n_colsOP = 4; long unsigned int MAXOP = 256; long unsigned int other_MAXOP = 256; data_t = int; long unsigned int n_rows = 4; long unsigned int n_cols = 4; long unsigned int MAX = 256; decltype ((declval<data_tOP>() + declval<TOP>())) = int]'
   65 |     operator+(Matrix<data_tOP, n_rowst, n_colsOP, MAXOP> const& lhs, Matrix<TOP, n_rowst, n_colsOP, other_MAXOP> const& rhs);
      |     ^~~~~~~~
```
Comment 1 Andrew Pinski 2022-10-30 19:13:18 UTC
Confirmed. reduced testcase:
template <class T> constexpr const T min(const T t0, const T t1) { return 0; }
template<int MAX>
struct Matrix{
    template<int MAXOP, int other_MAXOP>
friend Matrix<min(MAXOP, other_MAXOP)>
operator+(Matrix<MAXOP> const& lhs, Matrix<other_MAXOP> const& rhs);
};

template<int MAXOP, int other_MAXOP>
Matrix<min(MAXOP, other_MAXOP)>
operator+(Matrix<MAXOP> const& lhs, Matrix<other_MAXOP> const& rhs);

void f()
{
    Matrix<1> a;
    a+a;
}

---- CUT ----
Note the min function inside the return type is important and is looking like is what is causing issues to not match the two decls to be the same.
Having min as a non-template function allows it to be matched.
Comment 2 Patrick Palka 2022-11-08 14:19:08 UTC
Started with r12-6075-g2decd2cabe5a4f
Comment 3 GCC Commits 2023-02-03 14:41:20 UTC
The master branch has been updated by Patrick Palka <ppalka@gcc.gnu.org>:

https://gcc.gnu.org/g:59e0376f607805ef9b67fd7b0a4a3084ab3571a5

commit r13-5684-g59e0376f607805ef9b67fd7b0a4a3084ab3571a5
Author: Patrick Palka <ppalka@redhat.com>
Date:   Fri Feb 3 09:41:10 2023 -0500

    c++: unexpected ADDR_EXPR after overload set pruning [PR107461]
    
    Here the ahead-of-time overload set pruning in finish_call_expr is
    unintentionally returning a CALL_EXPR whose (pruned) callee is wrapped
    in an ADDR_EXPR, despite the original callee not being wrapped in an
    ADDR_EXPR.  This ends up causing a bogus declaration mismatch error in
    the below testcase because the call to min in #1 gets expressed as a
    CALL_EXPR of ADDR_EXPR of FUNCTION_DECL, whereas the level-lowered call
    to min in #2 gets expressed instead as a CALL_EXPR of FUNCTION_DECL.
    
    This patch fixes this by stripping the spurious ADDR_EXPR appropriately.
    Thus the first call to min now also gets expressed as a CALL_EXPR of
    FUNCTION_DECL, matching the behavior before r12-6075-g2decd2cabe5a4f.
    
            PR c++/107461
    
    gcc/cp/ChangeLog:
    
            * semantics.cc (finish_call_expr): Strip ADDR_EXPR from
            the selected callee during overload set pruning.
    
    gcc/testsuite/ChangeLog:
    
            * g++.dg/template/call9.C: New test.
Comment 4 GCC Commits 2023-02-03 15:07:20 UTC
The releases/gcc-12 branch has been updated by Patrick Palka <ppalka@gcc.gnu.org>:

https://gcc.gnu.org/g:534aea1ca7e7538dc6af3eac3cd2ec6c7343fdee

commit r12-9103-g534aea1ca7e7538dc6af3eac3cd2ec6c7343fdee
Author: Patrick Palka <ppalka@redhat.com>
Date:   Fri Feb 3 09:41:10 2023 -0500

    c++: unexpected ADDR_EXPR after overload set pruning [PR107461]
    
    Here the ahead-of-time overload set pruning in finish_call_expr is
    unintentionally returning a CALL_EXPR whose (pruned) callee is wrapped
    in an ADDR_EXPR, despite the original callee not being wrapped in an
    ADDR_EXPR.  This ends up causing a bogus declaration mismatch error in
    the below testcase because the call to min in #1 gets expressed as a
    CALL_EXPR of ADDR_EXPR of FUNCTION_DECL, whereas the level-lowered call
    to min in #2 gets expressed instead as a CALL_EXPR of FUNCTION_DECL.
    
    This patch fixes this by stripping the spurious ADDR_EXPR appropriately.
    Thus the first call to min now also gets expressed as a CALL_EXPR of
    FUNCTION_DECL, matching the behavior before r12-6075-g2decd2cabe5a4f.
    
            PR c++/107461
    
    gcc/cp/ChangeLog:
    
            * semantics.cc (finish_call_expr): Strip ADDR_EXPR from
            the selected callee during overload set pruning.
    
    gcc/testsuite/ChangeLog:
    
            * g++.dg/template/call9.C: New test.
    
    (cherry picked from commit 59e0376f607805ef9b67fd7b0a4a3084ab3571a5)
Comment 5 Patrick Palka 2023-02-03 15:08:03 UTC
Fixed for GCC 12.3/13, thanks for the bug report.
Comment 6 Khem Raj 2023-02-04 07:26:02 UTC
This is now resulting in build error in harfbuzz on aarch64 which did not happen before, the testcase is here

https://uclibc.org/~kraj/hb-aat-layout.cc.i

Compile cmd used is

/mnt/b/cross/aarch64-linux-musl/tools/bin/aarch64-linux-musl-g++ --sysroot=/mnt/b/yoe/master/build/tmp/work/cortexa72-yoe-linux/harfbuzz/6.0.0-r0/recipe-sysroot -fstack-protector-strong -O2 -D_FORTIFY_SOURCE=2 -Wformat -Wformat-security -Werror=format-security -Isrc/libharfbuzz.so.0.60600.0.p -Isrc -I../harfbuzz-6.0.0/src -I. -I../harfbuzz-6.0.0 -fdiagnostics-color=always -D_FILE_OFFSET_BITS=64 -Wall -Winvalid-pch -std=c++11 -fno-rtti -fno-exceptions -fno-rtti -fno-threadsafe-statics -fvisibility-inlines-hidden -DHAVE_CONFIG_H -O2 -pipe -g -feliminate-unused-debug-types -fmacro-prefix-map=/mnt/b/yoe/master/build/tmp/work/cortexa72-yoe-linux/harfbuzz/6.0.0-r0/harfbuzz-6.0.0=/usr/src/debug/harfbuzz/6.0.0-r0 -fdebug-prefix-map=/mnt/b/yoe/master/build/tmp/work/cortexa72-yoe-linux/harfbuzz/6.0.0-r0/harfbuzz-6.0.0=/usr/src/debug/harfbuzz/6.0.0-r0 -fmacro-prefix-map=/mnt/b/yoe/master/build/tmp/work/cortexa72-yoe-linux/harfbuzz/6.0.0-r0/build=/usr/src/debug/harfbuzz/6.0.0-r0 -fdebug-prefix-map=/mnt/b/yoe/master/build/tmp/work/cortexa72-yoe-linux/harfbuzz/6.0.0-r0/build=/usr/src/debug/harfbuzz/6.0.0-r0 -fdebug-prefix-map=/mnt/b/yoe/master/build/tmp/work/cortexa72-yoe-linux/harfbuzz/6.0.0-r0/recipe-sysroot= -fmacro-prefix-map=/mnt/b/yoe/master/build/tmp/work/cortexa72-yoe-linux/harfbuzz/6.0.0-r0/recipe-sysroot= -fdebug-prefix-map=/mnt/b/yoe/master/build/tmp/work/cortexa72-yoe-linux/harfbuzz/6.0.0-r0/recipe-sysroot-native= -fvisibility-inlines-hidden -fPIC -pthread -Wno-non-virtual-dtor -MD -MQ src/libharfbuzz.so.0.60600.0.p/hb-aat-layout.cc.o -MF src/libharfbuzz.so.0.60600.0.p/hb-aat-layout.cc.o.d -o src/libharfbuzz.so.0.60600.0.p/hb-aat-layout.cc.o -c ../harfbuzz-6.0.0/src/hb-aat-layout.cc
Comment 7 Jakub Jelinek 2023-02-04 10:37:51 UTC
Reduced testcase:
template <typename T> T foo ();
template <typename> using A = int;
template <typename T, typename U>
auto operator| (T, U) -> decltype (U() (T()));
template <typename T> struct B {
  template <typename U, typename = A<decltype (foo<T> () (0, foo<typename U::E> ()))>>
  void operator() (U);
};
struct {
  template <typename T, typename U>
  B<T> operator() (T, U) { return B<T> (); }
} c;
struct D {
  D() {
    c([] {}, 0);
  }
  struct E {
  };
  void bar ()
  {
    E f;
    f | c ([] (int, E) {}, 0);
  }
};
Comment 8 Jakub Jelinek 2023-02-04 10:39:14 UTC
.
Comment 9 Patrick Palka 2023-02-04 17:02:57 UTC
(In reply to Jakub Jelinek from comment #7)
> Reduced testcase:

Interestingly Clang also rejects this testcase, so I'm not sure if we were correct to accept it previously.

Here's a more reduced testcase that seems clearly valid:

template <typename T> T foo ();

template <typename> struct A { };

template <typename T> struct B {
  template <typename U, typename = A<decltype (foo<T>() (U()))>>
  static void bar(U);
};

int main() {
  B<int> b;
  B<void(*)(int)>::bar(0);
}

<stdin>: In function ‘int main()’:
<stdin>:12:23: error: no matching function for call to ‘B<void (*)(int)>::bar(int)’
<stdin>:7:15: note: candidate: ‘template<class U, class> static void B<T>::bar(U) [with <template-parameter-2-2> = U; T = void (*)(int)]’
<stdin>:7:15: note:   template argument deduction/substitution failed:
<stdin>:6:54: error: expression cannot be used as a function

If we remove the line #1 then this bogus error disappears.
Comment 10 Patrick Palka 2023-02-04 17:04:12 UTC
(In reply to Patrick Palka from comment #9)
> If we remove the line #1 then this bogus error disappears.

The line 'B<int> b;' rather.
Comment 11 Jakub Jelinek 2023-02-04 17:14:40 UTC
(In reply to Patrick Palka from comment #9)
> (In reply to Jakub Jelinek from comment #7)
> > Reduced testcase:
> 
> Interestingly Clang also rejects this testcase, so I'm not sure if we were
> correct to accept it previously.

Note, the reduction was done purely with a test that it compiles without errors before your commit and with 1-3 errors "no matching function for call to" after it.
Another thing is whether harfbuzz full source is valid or not, but that might be more difficult to find out because clang doesn't support the same builtins/traits etc.
Comment 12 GCC Commits 2023-02-06 02:35:49 UTC
The master branch has been updated by Patrick Palka <ppalka@gcc.gnu.org>:

https://gcc.gnu.org/g:31924665c86d47af6b1f22a74f594f2e1dc0ed2d

commit r13-5707-g31924665c86d47af6b1f22a74f594f2e1dc0ed2d
Author: Patrick Palka <ppalka@redhat.com>
Date:   Sun Feb 5 21:35:33 2023 -0500

    c++: equivalence of non-dependent calls [PR107461]
    
    After r13-5684-g59e0376f607805 the (pruned) callee of a non-dependent
    CALL_EXPR is a bare FUNCTION_DECL rather than ADDR_EXPR of FUNCTION_DECL.
    This innocent change revealed that cp_tree_equal doesn't first check
    dependence of a CALL_EXPR before treating a FUNCTION_DECL callee as a
    dependent name, which leads to us incorrectly accepting the first two
    testcases below and rejecting the third:
    
     * In the first testcase, cp_tree_equal incorrectly returns true for
       the two non-dependent CALL_EXPRs f(0) and f(0) (whose CALL_EXPR_FN
       are different FUNCTION_DECLs) which causes us to treat #2 as a
       redeclaration of #1.
    
     * Same issue in the second testcase, for f<int*>() and f<char>().
    
     * In the third testcase, cp_tree_equal incorrectly returns true for
       f<int>() and f<void(*)(int)>() which causes us to conflate the two
       dependent specializations A<decltype(f<int>()(U()))> and
       A<decltype(f<void(*)(int)>()(U()))>.
    
    This patch fixes this by making called_fns_equal treat two callees as
    dependent names only if the overall CALL_EXPRs are dependent, via a new
    convenience function call_expr_dependent_name that is like dependent_name
    but also checks dependence of the overall CALL_EXPR.
    
            PR c++/107461
    
    gcc/cp/ChangeLog:
    
            * cp-tree.h (call_expr_dependent_name): Declare.
            * pt.cc (iterative_hash_template_arg) <case CALL_EXPR>: Use
            call_expr_dependent_name instead of dependent_name.
            * tree.cc (call_expr_dependent_name): Define.
            (called_fns_equal): Adjust to take two CALL_EXPRs instead of
            CALL_EXPR_FNs thereof.  Use call_expr_dependent_name instead
            of dependent_name.
            (cp_tree_equal) <case CALL_EXPR>: Adjust call to called_fns_equal.
    
    gcc/testsuite/ChangeLog:
    
            * g++.dg/cpp0x/overload5.C: New test.
            * g++.dg/cpp0x/overload5a.C: New test.
            * g++.dg/cpp0x/overload6.C: New test.
Comment 13 Patrick Palka 2023-02-06 03:01:56 UTC
hb-aat-layout.cc.i and the comment #9 test should be accepted on trunk again now, backport to the 12 branch to follow.

The comment #7 testcase I think is invalid because GCC incorrectly accepts the substitution 'typename U::E [with U=E]', which seems to be PR34810.
Comment 14 GCC Commits 2023-02-06 16:33:23 UTC
The releases/gcc-12 branch has been updated by Patrick Palka <ppalka@gcc.gnu.org>:

https://gcc.gnu.org/g:eda24f6c12b6d3777ff3bf3656187e695a3e8dc2

commit r12-9110-geda24f6c12b6d3777ff3bf3656187e695a3e8dc2
Author: Patrick Palka <ppalka@redhat.com>
Date:   Sun Feb 5 21:35:33 2023 -0500

    c++: equivalence of non-dependent calls [PR107461]
    
    After r13-5684-g59e0376f607805 the (pruned) callee of a non-dependent
    CALL_EXPR is a bare FUNCTION_DECL rather than ADDR_EXPR of FUNCTION_DECL.
    This innocent change revealed that cp_tree_equal doesn't first check
    dependence of a CALL_EXPR before treating a FUNCTION_DECL callee as a
    dependent name, which leads to us incorrectly accepting the first two
    testcases below and rejecting the third:
    
     * In the first testcase, cp_tree_equal incorrectly returns true for
       the two non-dependent CALL_EXPRs f(0) and f(0) (whose CALL_EXPR_FN
       are different FUNCTION_DECLs) which causes us to treat #2 as a
       redeclaration of #1.
    
     * Same issue in the second testcase, for f<int*>() and f<char>().
    
     * In the third testcase, cp_tree_equal incorrectly returns true for
       f<int>() and f<void(*)(int)>() which causes us to conflate the two
       dependent specializations A<decltype(f<int>()(U()))> and
       A<decltype(f<void(*)(int)>()(U()))>.
    
    This patch fixes this by making called_fns_equal treat two callees as
    dependent names only if the overall CALL_EXPRs are dependent, via a new
    convenience function call_expr_dependent_name that is like dependent_name
    but also checks dependence of the overall CALL_EXPR.
    
            PR c++/107461
    
    gcc/cp/ChangeLog:
    
            * cp-tree.h (call_expr_dependent_name): Declare.
            * pt.cc (iterative_hash_template_arg) <case CALL_EXPR>: Use
            call_expr_dependent_name instead of dependent_name.
            * tree.cc (call_expr_dependent_name): Define.
            (called_fns_equal): Adjust to take two CALL_EXPRs instead of
            CALL_EXPR_FNs thereof.  Use call_expr_dependent_name instead
            of dependent_name.
            (cp_tree_equal) <case CALL_EXPR>: Adjust call to called_fns_equal.
    
    gcc/testsuite/ChangeLog:
    
            * g++.dg/cpp0x/overload5.C: New test.
            * g++.dg/cpp0x/overload5a.C: New test.
            * g++.dg/cpp0x/overload6.C: New test.
    
    (cherry picked from commit 31924665c86d47af6b1f22a74f594f2e1dc0ed2d)
Comment 15 Patrick Palka 2023-02-06 16:36:48 UTC
Fixed, hopefully
Comment 16 Patrick Palka 2023-03-03 14:12:58 UTC
*** Bug 109001 has been marked as a duplicate of this bug. ***