[Bug c++/99686] New: ICE when concepts on C++17 when providing both T&& and const T& specialization

[Bug c++/99686] New: ICE when concepts on C++17 when providing both T&& and const T& specialization

[StevenSun2021 at hotmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99686

            Bug ID: 99686
           Summary: ICE when concepts on C++17 when providing both T&& and
                    const T& specialization
           Product: gcc
           Version: 10.2.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: StevenSun2021 at hotmail dot com
  Target Milestone: ---

This problem occurs only when using std=c++17 and -fconcepts. It compiles when
using std=c++20.


I also have an online compiler link for this. https://godbolt.org/z/W9r6Wx

-------------------------------------------------------------------------
template <typename T>
void func(T &&arg)
{
}

template <typename T>
requires requires(T t) { ++t; }
void func(T &&arg)
{
}

template <>
void func(int &&arg)
{
}

template <>
void func(const int &arg)
{
}

int main()
{
    func(1); // I am expecting func(int&&)
}

[Bug c++/99686] ICE when concepts on C++17 when providing both T&& and const T& specialization

[StevenSun2021 at hotmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99686

--- Comment #1 from Steven Sun <StevenSun2021 at hotmail dot com> ---
The compiler args are only "g++ -fconcepts -std=c++17"

It should choose the full specialization func(int &&), while the compiler
aborts during the instatiation of the partial specialization (using concepts).

If either the partial or the full specialzation is removed, it compiles.


New online compiler link: https://godbolt.org/z/fK5G61
-------------------------------------------------------------------------
template <typename T>
void func(T &&arg)
{
}

template <typename T>
requires requires(T t) { ++t; }
void func(T &&arg)
{
}

template <>
void func(int &&arg)
{
}

int main()
{
    func(1); // I am expecting func(int&&)
}
-------------------------------------------------------------------------

Error:
In instantiation of ‘void func(T&&) [with T = int]’:
internal compiler error: Segmentation fault
   11 | }
      | ^
Please submit a full bug report,

[Bug c++/99686] ICE when using both concepts and full specialization

[webrown.cpp at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99686

W E Brown <webrown.cpp at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |webrown.cpp at gmail dot com

--- Comment #2 from W E Brown <webrown.cpp at gmail dot com> ---
Since C++ has no partial specializations of function templates, the test case
in Comment 1 is somewhat mischaracterized there.

Instead, what we have in that test case are, first, two distinct primary
templates that happen to share a common name within a single scope, and hence
overload that name.  Note that these primary templates are distinguishable
because one is more constrained (via its requires-clause) than the other (which
is unconstrained):  whenever these primary templates produce equally viable
candidates during overload resolution, C++ specifies that the more constrained
declaration should be selected.

Finally, the test case defines a complete specialization, but which of the
overloaded primary templates is being specialized?  That's the real question
for the compiler to determine in its role of language lawyer.  Note that if one
of the two primary templates were removed from the test case, the compiler's
decision would become near-trivial to make, as there would remain only a single
candidate.  If, instead, the explicit specialization were removed from the test
case, such a compiler determination would be no longer needed at all.

[Bug c++/99686] ICE when using both concepts and full specialization

[StevenSun2021 at hotmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99686

--- Comment #3 from Steven Sun <StevenSun2021 at hotmail dot com> ---
@W E Brown: I got your idea. So are all uses like this ill-formed? This seems
unexpected for me. I would expect the complete specialization is full
specialization for both primary templates.

I also find facts that support your idea:

If I modify the full specialization to 
--------------------
template <>
void func<int&&> (int&& arg){}
--------------------
It compiles. See https://godbolt.org/z/fh9Mx7Krr


If I modify the full specialization to 
--------------------
template <>
void func<int> (int&& arg){}
--------------------
It produce the same ICE output as comment 1. See
https://godbolt.org/z/eMa5YcrrE

It looks like the compiler doesn't knows which primary template to specialize.

I look up in the C++20 standard, and did not find anything about 2 primary
templates exists.

The code in Comment 1 compiles for g++ 6.1 to 9.3, then breaks since 10.1 (all
under -std=c++17 -fconcepts).

In conclusion, this makes sences but I didn't see that coming. Anyway, I think
a possible improvement is make ICE to an error of "ambigous full
specialization". Or even better, a change in C++23 standard.

[Bug c++/99686] ICE when using both concepts and full specialization

[webrown.cpp at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99686

--- Comment #4 from W E Brown <webrown.cpp at gmail dot com> ---
(In reply to Steven Sun from comment #3)
> .... I would expect the complete specialization is full
> specialization for both primary templates.

No, any given explicit or partial specialization can be a specialization of
only one primary template.  (Suppose that two overloaded primary templates had
different signatures.  How then, in general, could both be specialized by any
single explicit or partial specialization?)

> ... 
> In conclusion, this makes sences but I didn't see that coming. Anyway, I
> think a possible improvement is make ICE to an error of "ambigous full
> specialization". Or even better, a change in C++23 standard.

Given two or more very similar primary templates, C++ already specifies an
algorithm (known as "partial* ordering") to determine which one is being
specialized by a given explicit or partial specialization.  Therefore, unless
you can find some issue with that algorithm, I see no reason to propose any
language change in this area; partial ordering has been part of C++ for several
decades.

(However, I speculate that your test cases may have exposed a bug in GCC's
implementation of partial ordering, possibly just a failure to diagnose an
ambiguous situation.  I'm sure the GCC internals experts will provide proper
diagnosis and remediation in due course.)


* The algorithn is termed "partial" because there are cases that can't be
decided and therefore result in an ill-formed program.  If you'd like some
further details on this topic, I can recommend my video 😊
https://www.youtube.com/watch?v=nfIX8yWlByY.

[Bug c++/99686] ICE when using both concepts and full specialization

[StevenSun2021 at hotmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99686

--- Comment #5 from Steven Sun <StevenSun2021 at hotmail dot com> ---
I learn a little about gcc recently. I think I got a vague idea of what's going
on inside.

In c++17 mode with concepts, and with my code in comment 1.

The compiler decides to instantiate from the concept constrained template.

--------------------------------------------------------------------

Usually the compiler should generate a new function on the AST, but this time
there's already one function (fully specialized) on the AST.

So, the function `start_preparsed_function` in the below link returns at the
quoted line. It's thinking "Ah! There's already a function, no need to allocate
a new one."

https://gcc.gnu.org/git?p=gcc.git;a=blob;f=gcc/cp/decl.c;h=316ad4c1426940bd4f51197a6297eefc24064fec;hb=HEAD#l16696

--------------------------------------------------------------------

Then the flow goes back to the function in the link below. It's thinking "Since
I'm gonna add a new function, new function is allocated on AST. Everything
should be prepared for me!"

More specifially, it will assume the `cfun` in the correct state set by the
`start_preparsed_function`.

But `cfun` is null, since the `start_preparsed_function` returns early. This is
the direct cause of segfault, which triggered at the line below.

https://gcc.gnu.org/git?p=gcc.git;a=blob;f=gcc/cp/pt.c;h=36a8cb5df5d36337c18e1547e775b747f59a087f;hb=HEAD#l25932

--------------------------------------------------------------------

In conclusion, The compiler doesn't consider there would be a situation that an
instatiation is still needed when full specialzation is provided. This break
the invariants.

[Bug c++/99686] ICE when using both concepts and full specialization

[StevenSun2021 at hotmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99686

--- Comment #6 from Steven Sun <StevenSun2021 at hotmail dot com> ---
While in C++20, the complier thinks it's unnecessary to instatiate a new
template. Just use the full specialization! Thus, this bug wouldn't exist at
first place.

Intuitively, I am in favor of the compiler's C++20 behavior -- use full
specialization anyway.

Clang 12 and MSVC latest also have this behavior (choose full specialization).


I don't know how to contribute to gcc. If anyone would like to fix this bug, my
recommendation would be:

Make the compiler in C++17 mode think it's unnecessary to instantiate a new
function when full specialzation is provided, which resulting the same behavior
as C++20 mode and as other compilers.

[Bug c++/99686] ICE when using both concepts and full specialization

[StevenSun2021 at hotmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99686

--- Comment #7 from Steven Sun <StevenSun2021 at hotmail dot com> ---
After digging for two days, I think I may know what's happening inside. 

Under std=c++20 and my code in comment 1, the flow when parsing a full function
specialization would be

1. Found a full specialization of function.
2. Instantiate all possible primary function templates. 
3. Find the most suitable instantiation (here, the concept constrained is
chosen)
4. Then replace it with full specialization. (this is done in
`reregister_specialization` in `gcc/cp/pt.c`)

So when performing overloading in the main function, there would be one
instantiation and one full specialization to choose. (To be specific, those two
are in the hash table `decl_specialization` defined in `gcc/cp/pt.c`)


While in the c++17 mode, step 4 is broken. So there will be 2 implicit
instantiation to choose when overloading.

Step 4 is performed in the function `duplicate_decls` defined in
`gcc/cp/decl.c`.

Certain conditions must be satisfied to replace the instantiation.

The one failed here is:
"the compiler thinks the full specialization has a different concepts
constraint with primary template (because the primary template has contraints
but the full specialization doesn't)" 

So the reregistration is never triggered. This is the source of the bug.

After getting the whole picture, I simplify the test case as
https://godbolt.org/z/9MM6rEf77

----------------------- std=c++17 -fconcepts

template <typename T>
requires requires(T t) { ++t; }
void func(T &&arg)
{
}

template <>
void func(int&& arg)
{
}

int main()
{
    func(1);
}

-----------------------

I'll give more details in the next comment in case any developer would like to
fix it.

[Bug c++/99686] ICE when using both concepts and full specialization

[StevenSun2021 at hotmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99686

--- Comment #8 from Steven Sun <StevenSun2021 at hotmail dot com> ---
under c++17

Step 4 needs `types_match == 1` [at 1] but, its value is zero, which is caused
by `function_requirements_equivalent_p` [at 3] returns 0 [at 2] .

[1]
https://gcc.gnu.org/git?p=gcc.git;a=blob;f=gcc/cp/decl.c;h=316ad4c1426940bd4f51197a6297eefc24064fec;hb=HEAD#l1481
[2]
https://gcc.gnu.org/git?p=gcc.git;a=blob;f=gcc/cp/decl.c;h=316ad4c1426940bd4f51197a6297eefc24064fec;hb=HEAD#l1052
[3]
https://gcc.gnu.org/git?p=gcc.git;a=blob;f=gcc/cp/decl.c;h=316ad4c1426940bd4f51197a6297eefc24064fec;hb=HEAD#l944
[4]
https://gcc.gnu.org/git?p=gcc.git;a=blob;f=gcc/cp/pt.c;h=36a8cb5df5d36337c18e1547e775b747f59a087f;hb=HEAD#l3510

In `function_requirements_equivalent_p` [at 3], the comparison is different for
cxx20 and before.

According to the comments, before c++20, only the combined constraints are
compaired.

--------------------------------------------------------------
By the way, for those two primary function templates in comment 1. Before
cxx20, they are regard as 

equivalent heads [at 4]
inequivalent requirements [at 3]

Since cxx20, they are regard as 

inequivalent heads [at 4]
equivalent requirements [at 3]

If I change both [3], [4], forcing cxx17 using cxx20 standard, everything
works.
-------------------------------------------------------------
Many invariants are broken here. They cause this bug together:

A failure in requirements comparison caused no reregistration, which leads to
overload between implicit instatntiation. The full specialzation is already
parsed, leaving a null `cfun` state. Then it tries to instatiate the function
body and everything crashes,

They are all `by design` in a sense. How to fix it is not only a technical
issue, but a design choice.

If any senior gcc developer sees this, help me.
-------------------------------------------------------------
By the way, I don't think this bug would matter too much, since no one really
uses concepts before c++20. I am just curious.

[Bug c++/99686] ICE when using concepts on function template before c++20 (Reason already found)

[ppalka at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99686

Patrick Palka <ppalka at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |DUPLICATE
             Status|UNCONFIRMED                 |RESOLVED
                 CC|                            |ppalka at gcc dot gnu.org

--- Comment #9 from Patrick Palka <ppalka at gcc dot gnu.org> ---
Fixed by r12-2230

*** This bug has been marked as a duplicate of bug 101098 ***