[Bug c++/105289] New: ICE on partial specialization

[Bug c++/105289] New: ICE on partial specialization

[michsteinb at gmail dot com]

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

            Bug ID: 105289
           Summary: ICE on partial specialization
           Product: gcc
           Version: 11.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: michsteinb at gmail dot com
  Target Milestone: ---

Created attachment 52818
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=52818&action=edit
Self-contained reproduction of the ICE, compile with -std=c++20

arm-none-eabi-g++ 11.2 crashes on attached self-contained code with following
prompt on win64:
```
PS H:\projects\install\arm-embedded-gcc\11.2-2022.02\bin>
.\arm-none-eabi-g++.exe -save-temps -std=c++20 -c repro.cxx
repro.cxx:14:8: internal compiler error: Segmentation fault
   14 | struct push_front_vlist<XList<T, Vs...>, Arg>
      |        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Please submit a full bug report,
with preprocessed source if appropriate.
See <https://bugs.linaro.org/> for instructions.
```

The 10.x release cycle compilers accepted this code, so does clang, so this is
a regression introduced with 11.0.

Further checks on godbolt showed the same behaviour on all 11.x compilers I
tried.

The problem stems from the primary template using a meta-function of its first
parameter as type for its second. If I do not constrain the type of Arg, the
crash disappears.

[Bug c++/105289] [11/12 Regression] ICE on partial specialization

[ppalka at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
      Known to fail|                            |11.2.0, 12.0
           Assignee|unassigned at gcc dot gnu.org      |ppalka at gcc dot gnu.org
   Target Milestone|---                         |11.4
   Last reconfirmed|                            |2022-04-18
     Ever confirmed|0                           |1
            Summary|[11 Regression] ICE on      |[11/12 Regression] ICE on
                   |partial specialization      |partial specialization
             Status|UNCONFIRMED                 |ASSIGNED
           Keywords|                            |ice-on-valid-code
                 CC|                            |ppalka at gcc dot gnu.org
      Known to work|                            |10.3.0, 9.4.0

--- Comment #1 from Patrick Palka <ppalka at gcc dot gnu.org> ---
Confirmed, started with r11-6483-ge2e2f3f2c9400f.

[Bug c++/105289] [11/12 Regression] ICE on partial specialization

[rguenth at gcc dot gnu.org]

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

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P3                          |P2

[Bug c++/105289] [11/12 Regression] ICE on partial specialization

[michsteinb at gmail dot com]

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

--- Comment #2 from Michael Steinberg <michsteinb at gmail dot com> ---
Created attachment 52851
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=52851&action=edit
Working modified partial specialization

After the related issue was pointed out, it too made me curious whether my code
is valid, that is whether the partial specialization is truly more specialized
than the primary template.
So I modified the specialization so that Arg is at least as constrained as in
the primary template - et voila, the ICE is gone and the code is accepted.
This makes me believe that the classification 'ice-on-valid-code' may not be
true after all?

[Bug c++/105289] [11/12 Regression] ICE on partial specialization

[cvs-commit at gcc dot gnu.org]

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

--- Comment #3 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Patrick Palka <ppalka@gcc.gnu.org>:

https://gcc.gnu.org/g:288e4c64f6b4806358aabc9b99b2fba72bf04bf6

commit r12-8256-g288e4c64f6b4806358aabc9b99b2fba72bf04bf6
Author: Patrick Palka <ppalka@redhat.com>
Date:   Mon Apr 25 21:46:56 2022 -0400

    c++: partial ordering with dependent NTTP type [PR105289]

    Here ever since r11-6483-ge2e2f3f2c9400f we're rejecting and crashing
    on (respectively) two testcases that we used to accept in C++17 mode
    since r8-1437-g3da557ec145823.  Both testcases declare a partial
    specialization of a primary template that has an NTTP with dependent
    type, and the validity of these partial specializations is unclear and
    the subject of PR86193 / CWG 455.

    So for now, this minimal patch just aims to fix the crash in the second
    testcase.  During deduction, when checking whether the type of an NTTP
    uses still-undeduced parameters, we were incorrectly substituting into
    the previously substituted type instead of into its original type.

    In passing this patch also downgrades the "not more specialized"
    diagnostic from a permerror to a pedwarn.

            PR c++/105289
            PR c++/86193

    gcc/cp/ChangeLog:

            * pt.cc (process_partial_specialization): Downgrade "partial
            specialization isn't more specialized" diagnostic from permerror
            to an on-by-default pedwarn.
            (unify) <case TEMPLATE_PARM_INDEX>: When substituting into the
            NTTP type a second time, use the original type not the
            substituted type.

    gcc/testsuite/ChangeLog:

            * g++.dg/template/partial-specialization11.C: New test.
            * g++.dg/template/partial-specialization12.C: New test.

[Bug c++/105289] [11/12 Regression] ICE on partial specialization

[ppalka at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|ice-on-valid-code           |ice-on-invalid-code

--- Comment #4 from Patrick Palka <ppalka at gcc dot gnu.org> ---
(In reply to Michael Steinberg from comment #2)
> Created attachment 52851 [details]
> Working modified partial specialization
> 
> After the related issue was pointed out, it too made me curious whether my
> code is valid, that is whether the partial specialization is truly more
> specialized than the primary template.
> So I modified the specialization so that Arg is at least as constrained as
> in the primary template - et voila, the ICE is gone and the code is accepted.
> This makes me believe that the classification 'ice-on-valid-code' may not be
> true after all?

I suppose ice-on-invalid-code might be the more convenient classification since
after the above patch we now just reject the original testcase instead of
crashing.  This way the question of validity is left entirely to PR86193.

[Bug c++/105289] [11 Regression] ICE on partial specialization

[ppalka at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|[11/12 Regression] ICE on   |[11 Regression] ICE on
                   |partial specialization      |partial specialization

--- Comment #5 from Patrick Palka <ppalka at gcc dot gnu.org> ---
Fixed for GCC 12 so far

[Bug c++/105289] [11 Regression] ICE on partial specialization

[michsteinb at gmail dot com]

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

--- Comment #6 from Michael Steinberg <michsteinb at gmail dot com> ---
(In reply to Patrick Palka from comment #4)
> (In reply to Michael Steinberg from comment #2)
> > Created attachment 52851 [details]
> > Working modified partial specialization
> > <snip>
> 
> I suppose ice-on-invalid-code might be the more convenient classification
> since after the above patch we now just reject the original testcase instead
> of crashing.  This way the question of validity is left entirely to PR86193.

Even though this is my code that worked before, I would lean towards calling it
invalid given the standard. ;)

But on the other hand, one would need to take into consideration, if deducing
parameters by use of a single specialization with an empty primary template is
a usage common enough for a special rule allowing for the base template to set
an implicit lower bar to the specializations. I don't mind repeating the
constraints though.

[Bug c++/105289] [11 Regression] ICE on partial specialization

[cvs-commit at gcc dot gnu.org]

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

--- Comment #7 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-11 branch has been updated by Patrick Palka
<ppalka@gcc.gnu.org>:

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

commit r11-9945-gc4332c785c8e993104a4d76d86c737b51a636aee
Author: Patrick Palka <ppalka@redhat.com>
Date:   Mon Apr 25 21:46:56 2022 -0400

    c++: partial ordering with dependent NTTP type [PR105289]

    Here ever since r11-6483-ge2e2f3f2c9400f we're rejecting and crashing
    on (respectively) two testcases that we used to accept in C++17 mode
    since r8-1437-g3da557ec145823.  Both testcases declare a partial
    specialization of a primary template that has an NTTP with dependent
    type, and the validity of these partial specializations is unclear and
    the subject of PR86193 / CWG 455.

    So for now, this minimal patch just aims to fix the crash in the second
    testcase.  During deduction, when checking whether the type of an NTTP
    uses still-undeduced parameters, we were incorrectly substituting into
    the previously substituted type instead of into its original type.

    In passing this patch also downgrades the "not more specialized"
    diagnostic from a permerror to a pedwarn.

            PR c++/105289
            PR c++/86193

    gcc/cp/ChangeLog:

            * pt.c (process_partial_specialization): Downgrade "partial
            specialization isn't more specialized" diagnostic from permerror
            to an on-by-default pedwarn.
            (unify) <case TEMPLATE_PARM_INDEX>: When substituting into the
            NTTP type a second time, use the original type not the
            substituted type.

    gcc/testsuite/ChangeLog:

            * g++.dg/template/partial-specialization11.C: New test.
            * g++.dg/template/partial-specialization12.C: New test.

    (cherry picked from commit 288e4c64f6b4806358aabc9b99b2fba72bf04bf6)

[Bug c++/105289] [11 Regression] ICE on partial specialization

[ppalka at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|---                         |FIXED

--- Comment #8 from Patrick Palka <ppalka at gcc dot gnu.org> ---
Fixed -- PR86193 remains open to deal with the question of validity of such
partial specializations.