[Bug c++/88146] ice in tsubst_copy, at cp/pt.c:16014
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88146 --- Comment #14 from CVS Commits --- The releases/gcc-10 branch has been updated by Jakub Jelinek : https://gcc.gnu.org/g:01edf2031461b558f630afea382a813e41b631e6 commit r10-9481-g01edf2031461b558f630afea382a813e41b631e6 Author: Jakub Jelinek Date: Thu Mar 4 16:04:48 2021 +0100 c++: Fix up [[nodiscard]] on ctors on targetm.cxx.cdtor_returns_this targets [PR99362] In the P1771R1 changes JeanHeyd reverted part of Alex' PR88146 fix, but that seems to be incorrect to me. Where P1771R1 suggests warnings for [[nodiscard]] on constructors is handled in a different place - in particular the TARGET_EXPR handling of convert_to_void. When we have CALL_EXPR of a ctor, on most arches that call has void return type and so returns early, and on arm where the ctor returns the this pointer it is undesirable to warn as it warns about all ctor calls, not just the ones where it should warn. The P1771R1 changes added a test for this, but as it was given *.c extension rather than *.C, the test was never run and so this didn't get spotted immediately. The test also had a bug, (?n) can't be used in dg-warning/dg-error because those are implemented by prepending some regexp before the user provided one and (?n) must come at the start of the regexp. Furthermore, while -ftrack-macro-expansion=0 is useful in one nodiscard test which uses macros, I don't see how it would be relevant to all the other cpp2a/nodiscard* tests which don't use any macros. 2021-03-04 Jakub Jelinek PR c++/88146 PR c++/99362 gcc/cp/ * cvt.c (convert_to_void): Revert 2019-10-17 changes. Clarify comment. gcc/testsuite/ * g++.dg/cpp2a/nodiscard-constructor.c: Renamed to ... * g++.dg/cpp2a/nodiscard-constructor1.C: ... this. Remove -ftrack-macro-expansion=0 from dg-options. Don't use (?n) in dg-warning regexps, instead replace .* with \[^\n\r]*. * g++.dg/cpp2a/nodiscard-constructor2.C: New test. * g++.dg/cpp2a/nodiscard-reason-only-one.C: Remove -ftrack-macro-expansion=0 from dg-options. * g++.dg/cpp2a/nodiscard-reason-nonstring.C: Likewise. * g++.dg/cpp2a/nodiscard-once.C: Likewise. (cherry picked from commit c9816196328a4f4b927f08cf2f66cf255849da0b)
[Bug c++/88146] ice in tsubst_copy, at cp/pt.c:16014
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88146 --- Comment #13 from CVS Commits --- The master branch has been updated by Jakub Jelinek : https://gcc.gnu.org/g:c9816196328a4f4b927f08cf2f66cf255849da0b commit r11-7512-gc9816196328a4f4b927f08cf2f66cf255849da0b Author: Jakub Jelinek Date: Thu Mar 4 16:04:48 2021 +0100 c++: Fix up [[nodiscard]] on ctors on targetm.cxx.cdtor_returns_this targets [PR99362] In the P1771R1 changes JeanHeyd reverted part of Alex' PR88146 fix, but that seems to be incorrect to me. Where P1771R1 suggests warnings for [[nodiscard]] on constructors is handled in a different place - in particular the TARGET_EXPR handling of convert_to_void. When we have CALL_EXPR of a ctor, on most arches that call has void return type and so returns early, and on arm where the ctor returns the this pointer it is undesirable to warn as it warns about all ctor calls, not just the ones where it should warn. The P1771R1 changes added a test for this, but as it was given *.c extension rather than *.C, the test was never run and so this didn't get spotted immediately. The test also had a bug, (?n) can't be used in dg-warning/dg-error because those are implemented by prepending some regexp before the user provided one and (?n) must come at the start of the regexp. Furthermore, while -ftrack-macro-expansion=0 is useful in one nodiscard test which uses macros, I don't see how it would be relevant to all the other cpp2a/nodiscard* tests which don't use any macros. 2021-03-04 Jakub Jelinek PR c++/88146 PR c++/99362 gcc/cp/ * cvt.c (convert_to_void): Revert 2019-10-17 changes. Clarify comment. gcc/testsuite/ * g++.dg/cpp2a/nodiscard-constructor.c: Renamed to ... * g++.dg/cpp2a/nodiscard-constructor1.C: ... this. Remove -ftrack-macro-expansion=0 from dg-options. Don't use (?n) in dg-warning regexps, instead replace .* with \[^\n\r]*. * g++.dg/cpp2a/nodiscard-constructor2.C: New test. * g++.dg/cpp2a/nodiscard-reason-only-one.C: Remove -ftrack-macro-expansion=0 from dg-options. * g++.dg/cpp2a/nodiscard-reason-nonstring.C: Likewise. * g++.dg/cpp2a/nodiscard-once.C: Likewise.
[Bug c++/88146] ice in tsubst_copy, at cp/pt.c:16014
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88146 --- Comment #12 from Alexandre Oliva --- Author: aoliva Date: Thu Jan 17 04:49:55 2019 New Revision: 268004 URL: https://gcc.gnu.org/viewcvs?rev=268004=gcc=rev Log: [PR88146] avoid diagnostics diffs if cdtor_returns_this Diagnostics for testsuite/g++.dg/cpp0x/inh-ctor32.C varied across platforms. Specifically, on ARM, the diagnostics within the subtest derived_ctor::inherited_derived_ctor::constexpr_noninherited_ctor did not match those displayed on other platforms, and the test failed. The difference seemed to have to do with locations assigned to ctors, but it was more subtle: on ARM, the instantiation of bor's template ctor was nested within the instantiation of bar's template ctor inherited from bor. The reason turned out to be related with the internal return type of ctors: arm_cxx_cdtor_returns_this is enabled for because of AAPCS, while cxx.cdtor_returns_this is disabled on most other platforms. While convert_to_void returns early with a VOID expr, the non-VOID return type of the base ctor CALL_EXPR causes convert_to_void to inspect the called decl for nodiscard attributes: maybe_warn_nodiscard -> cp_get_fndecl_from_callee -> maybe_constant_init -> cxx_eval_outermost_constant_expr -> instantiate_constexpr_fns -> nested instantiation. The internal return type assigned to a cdtor should not affect instantiation (constexpr or template) decisions, IMHO. We know it affects diagnostics, but I have a hunch this might bring deeper issues with it, so I've arranged for the CALL_EXPR handler in convert_to_void to disregard cdtors, regardless of the ABI. for gcc/cp/ChangeLog PR c++/88146 * cvt.c (convert_to_void): Handle all cdtor calls as if returning void. Modified: trunk/gcc/cp/ChangeLog trunk/gcc/cp/cvt.c
[Bug c++/88146] ice in tsubst_copy, at cp/pt.c:16014
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88146 Bug 88146 depends on bug 87814, which changed state. Bug 87814 Summary: [9 Regression] ICE in in tsubst_copy, at cp/pt.c:15962 with range-v3 https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87814 What|Removed |Added Status|ASSIGNED|RESOLVED Resolution|--- |FIXED
[Bug c++/88146] ice in tsubst_copy, at cp/pt.c:16014
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88146 Alexandre Oliva changed: What|Removed |Added Status|ASSIGNED|RESOLVED Resolution|--- |FIXED --- Comment #11 from Alexandre Oliva --- Fixed in the trunk
[Bug c++/88146] ice in tsubst_copy, at cp/pt.c:16014
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88146 --- Comment #10 from Alexandre Oliva --- Author: aoliva Date: Wed Dec 19 06:51:19 2018 New Revision: 267250 URL: https://gcc.gnu.org/viewcvs?rev=267250=gcc=rev Log: [PR c++/88146] do not crash synthesizing inherited ctor(...) This patch started out from the testcase in PR88146, that attempted to synthesize an inherited ctor without any args before a varargs ellipsis and crashed while at that, because of the unguarded dereferencing of the parm type list, that usually contains a terminator. The terminator is not there for varargs functions, however, and without any other args, we ended up dereferencing a NULL pointer. Oops. Guarding accesses to parm would be easy, but not necessary. In do_build_copy_constructor, non-inherited ctors are copy-ctors, that always have at least one parm, so parm needs not be guarded when we know the access will only take place when we're dealing with an inherited ctor. The only other problematic use was in the cvquals initializer, a variable only used in a loop over fields, that we skipped individually in inherited ctors. I've guarded the cvquals initialization and the entire loop over fields so they only run for copy-ctors. Avoiding the crash from unguarded accesses was easy, but I thought we should still produce the sorry message we got in other testcases that passed arguments through the ellipsis in inherited ctors. I put a check in, and noticed the inherited ctors were synthesized with the location assigned to the class name, although they were initially assigned the location of the using declaration. I decided the latter was better, and arranged for the better location to be retained. Further investigation revealed the lack of a sorry message had to do with the call being in a non-evaluated context, in this case, a noexcept expression. The sorry would be correctly reported in other contexts, so I rolled back the check I'd added, but retained the source location improvement. I was still concerned about issuing sorry messages while instantiating template ctors even in non-evaluated contexts, e.g., if a template ctor had a base initializer that used an inherited ctor with enough arguments that they'd go through an ellipsis. I wanted to defer the instantiation of such template ctors, but that would have been wrong for constexpr template ctors, and already done for non-constexpr ones. So, I just consolidated multiple test variants into a single testcase that explores and explains various of the possibilities I thought of. for gcc/cp/ChangeLog PR c++/88146 * method.c (do_build_copy_constructor): Guard cvquals init and loop over fields to run for non-inherited ctors only. (synthesize_method): Retain location of inherited ctor. for gcc/testsuite/ChangeLog PR c++/88146 * g++.dg/cpp0x/inh-ctor32.C: New. Added: trunk/gcc/testsuite/g++.dg/cpp0x/inh-ctor32.C Modified: trunk/gcc/cp/ChangeLog trunk/gcc/cp/method.c trunk/gcc/testsuite/ChangeLog
[Bug c++/88146] ice in tsubst_copy, at cp/pt.c:16014
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88146 Alexandre Oliva changed: What|Removed |Added Depends on||87814 --- Comment #9 from Alexandre Oliva --- https://gcc.gnu.org/ml/gcc-patches/2018-12/msg00424.html Referenced Bugs: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87814 [Bug 87814] [9 Regression] ICE in in tsubst_copy, at cp/pt.c:15962 with range-v3
[Bug c++/88146] ice in tsubst_copy, at cp/pt.c:16014
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88146 --- Comment #8 from Alexandre Oliva --- So, no, it's not a copy ctor, but apparently we're reusing the logic that synthesizes them for other non-default ctors.
[Bug c++/88146] ice in tsubst_copy, at cp/pt.c:16014
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88146 Alexandre Oliva changed: What|Removed |Added Status|NEW |ASSIGNED Assignee|unassigned at gcc dot gnu.org |aoliva at gcc dot gnu.org --- Comment #7 from Alexandre Oliva --- Created attachment 45162 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=45162=edit candidate patch This patch fixes the additional problem. We have a (...) ctor in a base class, reexported with an access declaration. We try to do_build_copy_constructor from that, but then FUNCTION_FIRST_USER_PARM is NULL, and we attempt to dereference it unconditionally, since copy ctors normally have at least one user parm. Should this one even match as a copy ctor? Checking...
[Bug c++/88146] ice in tsubst_copy, at cp/pt.c:16014
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88146 Alexandre Oliva changed: What|Removed |Added CC||aoliva at gcc dot gnu.org --- Comment #6 from Alexandre Oliva --- The patch I've just attached to bug 87814 affects this as well, but although it allows the testcase to avoid this specific crash, it still fails later, while synthesizing a ctor for dn.
[Bug c++/88146] ice in tsubst_copy, at cp/pt.c:16014
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88146 Marek Polacek changed: What|Removed |Added Status|WAITING |NEW CC||mpolacek at gcc dot gnu.org --- Comment #5 from Marek Polacek --- This also started with r261084 (like bug 87814), though the backtrace is not the same.
[Bug c++/88146] ice in tsubst_copy, at cp/pt.c:16014
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88146 ensadc at mailnesia dot com changed: What|Removed |Added CC||ensadc at mailnesia dot com --- Comment #4 from ensadc at mailnesia dot com --- Related to bug 87814?
[Bug c++/88146] ice in tsubst_copy, at cp/pt.c:16014
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88146 --- Comment #3 from David Binderman --- Created attachment 45065 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=45065=edit C++ source code
[Bug c++/88146] ice in tsubst_copy, at cp/pt.c:16014
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88146 Richard Biener changed: What|Removed |Added Status|UNCONFIRMED |WAITING Last reconfirmed||2018-11-22 Ever confirmed|0 |1 --- Comment #2 from Richard Biener --- Please attach, the inline copy is mangled.
[Bug c++/88146] ice in tsubst_copy, at cp/pt.c:16014
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88146 --- Comment #1 from David Binderman --- svn blame says 11362mrs default: 229756 miyuki /* We shouldn't get here, but keep going if !flag_checking. */ 229756 miyuki if (flag_checking) 229756 miyuki gcc_unreachable (); 170677 jason return t; 6613mrs }
