[Bug c++/107126] New: GCC accepts invalid out of class definition for destructor with C++17

[Bug c++/107126] New: GCC accepts invalid out of class definition for destructor with C++17

[jlame646 at gmail dot com]

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

            Bug ID: 107126
           Summary: GCC accepts invalid out of class definition for
                    destructor with C++17
           Product: gcc
           Version: 12.1.1
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: jlame646 at gmail dot com
  Target Milestone: ---

The following invalid program is accepted by gcc with C++17. Demo:
https://godbolt.org/z/TYjEzss6q

```
template<typename T>
struct C
{
    ~C();
};
template<typename T>
C<T>::~C<T>()      //this is accepted by gcc with c++17
{

}
int main()
{
    C<int> c;;
}
```

This has been discussed here:
https://stackoverflow.com/questions/73928601/clang-accepts-out-of-class-destructor-definition-while-gcc-does-not

[Bug c++/107126] GCC accepts invalid out of class definition for destructor with C++17

[jakub at gcc dot gnu.org]

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

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jakub at gcc dot gnu.org

--- Comment #1 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
This is rejected with -std=c++20 since
r11-532-g4b38d56dbac6742b038551a36ec80200313123a1
and the commit log states that it is intentional to apply it only for C++20
mode because the DR wasn't against C++17.
So why do you think otherwise?
https://www.open-std.org/jtc1/sc22/wg21/docs/cwg_defects.html#2237
also states:
(Note that this resolution is a change for C++20, NOT a defect report against
C++17 and earlier versions.)

[Bug c++/107126] GCC accepts invalid out of class definition for destructor with C++17

[jlame646 at gmail dot com]

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

--- Comment #2 from Jason Liam <jlame646 at gmail dot com> ---
(In reply to Jakub Jelinek from comment #1)
> This is rejected with -std=c++20 since
> r11-532-g4b38d56dbac6742b038551a36ec80200313123a1
> and the commit log states that it is intentional to apply it only for C++20
> mode because the DR wasn't against C++17.
> So why do you think otherwise?
> https://www.open-std.org/jtc1/sc22/wg21/docs/cwg_defects.html#2237
> also states:
> (Note that this resolution is a change for C++20, NOT a defect report
> against C++17 and earlier versions.)

First, you've clearly missed something here because this is not
https://www.open-std.org/jtc1/sc22/wg21/docs/cwg_defects.html#2237 . Note
carefully that the above link that you provided was for declarations of ctors
and dtors inside the class definition itself and not for declarations at
namespace scope. 


Second, i'm already aware that this is rejected by gcc with c++20 as i provided
a demo link https://godbolt.org/z/TYjEzss6q at the first line of my bug report.

[Bug c++/107126] GCC accepts invalid out of class definition for destructor with C++17

[jlame646 at gmail dot com]

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

--- Comment #3 from Jason Liam <jlame646 at gmail dot com> ---
(In reply to Jakub Jelinek from comment #1)
> This is rejected with -std=c++20 since
> r11-532-g4b38d56dbac6742b038551a36ec80200313123a1
> and the commit log states that it is intentional to apply it only for C++20
> mode because the DR wasn't against C++17.
> So why do you think otherwise?
> https://www.open-std.org/jtc1/sc22/wg21/docs/cwg_defects.html#2237
> also states:
> (Note that this resolution is a change for C++20, NOT a defect report
> against C++17 and earlier versions.)

Additionally note that i'm already aware of the cwg defect link that you
provided because i once used it here:
https://stackoverflow.com/questions/71972000/is-having-a-declaration-stackt-for-the-default-ctor-valid-inside-a-class-te/71981884#71981884

But that cwg defect does not apply here. This is a different gcc bug.

[Bug c++/107126] GCC accepts invalid out of class definition for destructor with C++17

[jakub at gcc dot gnu.org]

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

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |mpolacek at gcc dot gnu.org

--- Comment #4 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
(In reply to Jason Liam from comment #2)
> Second, i'm already aware that this is rejected by gcc with c++20 as i
> provided a demo link https://godbolt.org/z/TYjEzss6q at the first line of my
> bug report.

One thing is to know that some versions of g++ do that, but the above provides
details on what change actually changed the behavior, which is useful
information for any further changes.
Then it is https://cplusplus.github.io/CWG/issues/1435.html which we perhaps
don't implement or don't implement fully.

[Bug c++/107126] GCC accepts invalid out of class definition for destructor with C++17

[jlame646 at gmail dot com]

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

--- Comment #5 from Jason Liam <jlame646 at gmail dot com> ---
(In reply to Jakub Jelinek from comment #4)
> (In reply to Jason Liam from comment #2)
> > Second, i'm already aware that this is rejected by gcc with c++20 as i
> > provided a demo link https://godbolt.org/z/TYjEzss6q at the first line of my
> > bug report.
> 
> One thing is to know that some versions of g++ do that, but the above
> provides details on what change actually changed the behavior, which is
> useful information for any further changes.
> Then it is https://cplusplus.github.io/CWG/issues/1435.html which we perhaps
> don't implement or don't implement fully.

Note that the program shown in this bug is ill-formed in all c++ versions
including c++17. That is, it is different from
https://www.open-std.org/jtc1/sc22/wg21/docs/cwg_defects.html#2237 because that
cwg defect report only talks about declarations that are in the member
specification of the class and not in the namespace scope.

This means that the program shown in that cwg defect report is ill-formed only
from c++20 and onwards. But the program shown in my bug is ill-formed with all
modern c++ version including c++17. So this is a different bug.

[Bug c++/107126] GCC accepts invalid out of class definition for destructor with C++17

[jlame646 at gmail dot com]

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

--- Comment #6 from Jason Liam <jlame646 at gmail dot com> ---
This means that the code `template<class T> struct A {~A<T>(); };` is
ill-formed only with c++20 and onwards while the code `template<class T> struct
A {~A(); }; template<typename T> A<T>::~A<T>(){} ;` is ill-formed in all c++
versions including c++17.

[Bug c++/107126] GCC accepts invalid out of class definition for destructor with C++17

[redi at gcc dot gnu.org]

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

--- Comment #7 from Jonathan Wakely <redi at gcc dot gnu.org> ---
https://wg21.link/cwg1435 introduced the wording that allows this (which can
still be seen in the context for the [class.dtor] changes in CWG 2337):

"in a declaration at namespace scope or in a friend declaration, the
id-expression is nested-name-specifier ~class-name and the class-name names the
same class as the nested-name-specifier."

https://wg21.link/p1787r6 changed that to:

"otherwise, the id-expression is nested-name-specifier ~class-name and the
class-name is the injected-class-name of the class nominated by the
nested-name-specifier."

This seems like a breaking change that was not obvious from the revision
history of the r5 paper:

- Required destructor declarations to use the injected-class-name, avoiding
name lookup
- Simplified lookup for destructors
- Specified that < begins a template argument list in a destructor name

All compilers accept the program in C++17 mode, and I don't think it's at all
clear that the code was always invalid in older standards, as you claim.

[Bug c++/107126] -Wc++20-compat should complain about template-id in out of class definition for destructor

[jason at gcc dot gnu.org]

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

Jason Merrill <jason at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
     Ever confirmed|0                           |1
                 CC|                            |jason at gcc dot gnu.org
            Summary|GCC accepts invalid out of  |-Wc++20-compat should
                   |class definition for        |complain about template-id
                   |destructor with C++17       |in out of class definition
                   |                            |for destructor
   Last reconfirmed|                            |2022-10-03

--- Comment #8 from Jason Merrill <jason at gcc dot gnu.org> ---
As Jonathan documents, this was explicitly well-formed from DR1435 in C++11
until P1787 in C++23.  It seems reasonable to also apply the P1787 change to
C++20, treating it as a missing piece of the DR2237 resolution.  It seems
undesirable to apply it to C++17 as well.

But it would be helpful for -Wc++20-compat to flag this compatibility issue

Incidentally, we should probably also reduce the C++20 error to a pedwarn.

[Bug c++/107126] -Wc++20-compat should complain about template-id in out of class definition for destructor

[mpolacek at gcc dot gnu.org]

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

Marek Polacek <mpolacek at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
           Assignee|unassigned at gcc dot gnu.org      |mpolacek at gcc dot gnu.org

[Bug c++/107126] -Wc++20-compat should complain about template-id in out of class definition for destructor

[cvs-commit at gcc dot gnu.org]

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

--- Comment #9 from GCC Commits <cvs-commit at gcc dot gnu.org> ---
The trunk branch has been updated by Marek Polacek <mpolacek@gcc.gnu.org>:

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

commit r14-8915-gcff174fabd6c980c09aee95db1d9d5c22421761f
Author: Marek Polacek <polacek@redhat.com>
Date:   Fri Feb 2 14:53:01 2024 -0500

    c++: DR2237, cdtor and template-id tweaks [PR107126]

    Since my r11-532 changes to implement DR2237, for this test:

      template<typename T>
      struct S {
        S<T>();
      };

    in C++20 we emit the ugly:

    q.C:3:8: error: expected unqualified-id before ')' token
        3 |   S<T>();

    which doesn't explain what the problem is.  This patch improves that
    diagnostic, reduces the error to a pedwarn, and adds a -Wc++20-compat
    diagnostic.  We now say:

    q.C:3:7: warning: template-id not allowed for constructor in C++20
[-Wtemplate-id-cdtor]
        3 |   S<T>();
    q.C:3:7: note: remove the '< >'

    This patch also fixes
    <https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97202#c8>
    where the C++20 diagnostic was missing altogether:  The problem was that I
checked
    for CPP_TEMPLATE_ID too early, at a point at which cp_parser_template_id
may not
    have been called yet.  So let's check for it at the end of the function,
after
    the tentative parse and rollback.

    -Wc++20-compat triggered in libitm/; I sent a patch for that.

            DR 2237
            PR c++/107126
            PR c++/97202

    gcc/c-family/ChangeLog:

            * c-opts.cc (c_common_post_options): In C++20 or with
-Wc++20-compat,
            turn on -Wtemplate-id-cdtor.
            * c.opt (Wtemplate-id-cdtor): New.

    gcc/cp/ChangeLog:

            * parser.cc (cp_parser_unqualified_id): Downgrade the DR2237 error
to
            a pedwarn.
            (cp_parser_constructor_declarator_p): Likewise.

    gcc/ChangeLog:

            * doc/invoke.texi: Document -Wtemplate-id-cdtor.

    gcc/testsuite/ChangeLog:

            * g++.dg/DRs/dr2237.C: Adjust dg-error.
            * g++.dg/parse/constructor2.C: Likewise.
            * g++.dg/template/error34.C: Likewise.
            * g++.old-deja/g++.pt/ctor2.C: Likewise.
            * g++.dg/DRs/dr2237-2.C: New test.
            * g++.dg/DRs/dr2237-3.C: New test.
            * g++.dg/DRs/dr2237-4.C: New test.
            * g++.dg/DRs/dr2237-5.C: New test.
            * g++.dg/warn/Wtemplate-id-cdtor-1.C: New test.
            * g++.dg/warn/Wtemplate-id-cdtor-2.C: New test.
            * g++.dg/warn/Wtemplate-id-cdtor-3.C: New test.
            * g++.dg/warn/Wtemplate-id-cdtor-4.C: New test.

[Bug c++/107126] -Wc++20-compat should complain about template-id in out of class definition for destructor

[mpolacek at gcc dot gnu.org]

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

Marek Polacek <mpolacek at gcc dot gnu.org> changed:

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

--- Comment #10 from Marek Polacek <mpolacek at gcc dot gnu.org> ---
Should be fixed.