Re: check_qualified_type
On Mon, 17 Jun 2024 at 07:02, Richard Biener via Gcc wrote:
>
> On Sun, 16 Jun 2024, Martin Uecker wrote:
>
> >
> >
> > I am trying to understand what check_qualified_type
> > does exactly. The direct comparison of TYPE_NAMES seems incorrect
> > for C and its use is c_update_type_canonical then causes
> > PR114930 and PR115502. In the later function I think
> > it is not really needed and I guess one could simply remove
> > it, but I wonder if it works incorrectly in other cases
> > too?
>
> TYPE_NAMES is compared because IIRC typedefs are recorded as variants
> and 'const T' isn't the same as 'const int' with typedef int T.
Presumably it's also relevant for this example:
typedef signed int sint;
struct S {
int i : 2;
sint s : 2;
};
Here it's impl-defined whether i is signed, but s must be signed.
Re: check_qualified_type
On Mon, Jun 17, 2024 at 03:53:41PM +0200, Martin Uecker wrote: > > If c_update_type_canonical is only ever called for the main variants of the > > type and they always have !TYPE_QUALS (t), then yes. > > But if we rely on that, perhaps we should gcc_checking_assert that. > > So > > gcc_checking_assert (t == TYPE_MAIN_VARIANT (t) && !TYPE_QUALS (t)); > > or something similar at the start of the function. > > It calls itself recursively on pointers to the type. But to > me the third branch looks dead in any case, because the first > two cover all possibilities. Yes, but the pointers built by build_pointer_type should still be hopefully the unqualified versions, if one wants a qualified pointer, that would be build_qualified_type (build_pointer_type (...), ...) The checks cover all the possibilities only if the canonical type has the same quals as t. > > Then we could also change the > > for (tree x = TYPE_MAIN_VARIANT (t); x; x = TYPE_NEXT_VARIANT (x)) > > to > > for (tree x = t; x; x = TYPE_NEXT_VARIANT (x)) > > and > > if (TYPE_QUALS (x) == TYPE_QUALS (t)) > > ... > > to > > if (!TYPE_QUALS (x)) > > TYPE_CANONICAL (x) = TYPE_CANONICAL (t); > > else > > build_qualified_type (TYPE_CANONICAL (t), TYPE_QUALS (x)); > > Jakub
Re: check_qualified_type
Am Montag, dem 17.06.2024 um 15:40 +0200 schrieb Jakub Jelinek: > On Mon, Jun 17, 2024 at 03:33:05PM +0200, Martin Uecker wrote: > > > I've done that and that was because build_qualified_type uses that > > > predicate, where qualified types created by build_qualified_type have > > > as TYPE_CANONICAL the qualified type of the main variant of the canonical > > > type, while in all other cases TYPE_CANONICAL is just the main variant of > > > the type. > > > Guess we could also just do > > > if (TYPE_QUALS (x) == TYPE_QUALS (t)) > > > TYPE_CANONICAL (x) = TYPE_CANONICAL (t); > > > else if (TYPE_CANONICAL (t) != t > > > || TYPE_QUALS (x) != TYPE_QUALS (TYPE_CANONICAL (t))) > > > TYPE_CANONICAL (x) > > > = build_qualified_type (TYPE_CANONICAL (t), TYPE_QUALS (x)); > > > else > > > TYPE_CANONICAL (x) = x; > > > > > > > Ok, that works. I think the final "else" is then then impossible to reach > > and can be eliminated as well, because if TYPE_CANONICAL (t) == t then > > TYPE_QUALS (x) == TYPE_QUALS (TYPE_CANONICAL (t)) is identical to > > TYPE_QUALS (x) == TYPE_QUALS (t) which is the first case. > > If c_update_type_canonical is only ever called for the main variants of the > type and they always have !TYPE_QUALS (t), then yes. > But if we rely on that, perhaps we should gcc_checking_assert that. > So > gcc_checking_assert (t == TYPE_MAIN_VARIANT (t) && !TYPE_QUALS (t)); > or something similar at the start of the function. It calls itself recursively on pointers to the type. But to me the third branch looks dead in any case, because the first two cover all possibilities. Martin > Then we could also change the > for (tree x = TYPE_MAIN_VARIANT (t); x; x = TYPE_NEXT_VARIANT (x)) > to > for (tree x = t; x; x = TYPE_NEXT_VARIANT (x)) > and > if (TYPE_QUALS (x) == TYPE_QUALS (t)) > ... > to > if (!TYPE_QUALS (x)) > TYPE_CANONICAL (x) = TYPE_CANONICAL (t); > else > build_qualified_type (TYPE_CANONICAL (t), TYPE_QUALS (x)); > >
Re: check_qualified_type
On Mon, Jun 17, 2024 at 03:33:05PM +0200, Martin Uecker wrote: > > I've done that and that was because build_qualified_type uses that > > predicate, where qualified types created by build_qualified_type have > > as TYPE_CANONICAL the qualified type of the main variant of the canonical > > type, while in all other cases TYPE_CANONICAL is just the main variant of > > the type. > > Guess we could also just do > > if (TYPE_QUALS (x) == TYPE_QUALS (t)) > > TYPE_CANONICAL (x) = TYPE_CANONICAL (t); > > else if (TYPE_CANONICAL (t) != t > >|| TYPE_QUALS (x) != TYPE_QUALS (TYPE_CANONICAL (t))) > > TYPE_CANONICAL (x) > > = build_qualified_type (TYPE_CANONICAL (t), TYPE_QUALS (x)); > > else > > TYPE_CANONICAL (x) = x; > > > > Ok, that works. I think the final "else" is then then impossible to reach > and can be eliminated as well, because if TYPE_CANONICAL (t) == t then > TYPE_QUALS (x) == TYPE_QUALS (TYPE_CANONICAL (t)) is identical to > TYPE_QUALS (x) == TYPE_QUALS (t) which is the first case. If c_update_type_canonical is only ever called for the main variants of the type and they always have !TYPE_QUALS (t), then yes. But if we rely on that, perhaps we should gcc_checking_assert that. So gcc_checking_assert (t == TYPE_MAIN_VARIANT (t) && !TYPE_QUALS (t)); or something similar at the start of the function. Then we could also change the for (tree x = TYPE_MAIN_VARIANT (t); x; x = TYPE_NEXT_VARIANT (x)) to for (tree x = t; x; x = TYPE_NEXT_VARIANT (x)) and if (TYPE_QUALS (x) == TYPE_QUALS (t)) ... to if (!TYPE_QUALS (x)) TYPE_CANONICAL (x) = TYPE_CANONICAL (t); else build_qualified_type (TYPE_CANONICAL (t), TYPE_QUALS (x)); Jakub
Re: check_qualified_type
Am Montag, dem 17.06.2024 um 14:57 +0200 schrieb Jakub Jelinek: > On Mon, Jun 17, 2024 at 02:42:05PM +0200, Richard Biener wrote: > > > > > I am trying to understand what check_qualified_type > > > > > does exactly. The direct comparison of TYPE_NAMES seems incorrect > > > > > for C and its use is c_update_type_canonical then causes > > > > > PR114930 and PR115502. In the later function I think > > > > > it is not really needed and I guess one could simply remove > > > > > it, but I wonder if it works incorrectly in other cases > > > > > too? > > > > > > > > TYPE_NAMES is compared because IIRC typedefs are recorded as variants > > > > and 'const T' isn't the same as 'const int' with typedef int T. > > > > > > so if it is intentional that it differentiates between > > > > > > struct foo > > > > > > and > > > > > > typedef struct foo bar > > > > > > then I will change c_update_type_canonical to not use it, > > > because both types should have the same TYPE_CANONICAL > > > > The check is supposed to differentiate between variants and all variants > > have the same TYPE_CANONICAL so I'm not sure why you considered using > > this function for canonical type compute? > > I've done that and that was because build_qualified_type uses that > predicate, where qualified types created by build_qualified_type have > as TYPE_CANONICAL the qualified type of the main variant of the canonical > type, while in all other cases TYPE_CANONICAL is just the main variant of > the type. > Guess we could also just do > if (TYPE_QUALS (x) == TYPE_QUALS (t)) > TYPE_CANONICAL (x) = TYPE_CANONICAL (t); > else if (TYPE_CANONICAL (t) != t > || TYPE_QUALS (x) != TYPE_QUALS (TYPE_CANONICAL (t))) > TYPE_CANONICAL (x) > = build_qualified_type (TYPE_CANONICAL (t), TYPE_QUALS (x)); > else > TYPE_CANONICAL (x) = x; > Ok, that works. I think the final "else" is then then impossible to reach and can be eliminated as well, because if TYPE_CANONICAL (t) == t then TYPE_QUALS (x) == TYPE_QUALS (TYPE_CANONICAL (t)) is identical to TYPE_QUALS (x) == TYPE_QUALS (t) which is the first case. Martin
Re: check_qualified_type
On Mon, Jun 17, 2024 at 02:42:05PM +0200, Richard Biener wrote: > > > > I am trying to understand what check_qualified_type > > > > does exactly. The direct comparison of TYPE_NAMES seems incorrect > > > > for C and its use is c_update_type_canonical then causes > > > > PR114930 and PR115502. In the later function I think > > > > it is not really needed and I guess one could simply remove > > > > it, but I wonder if it works incorrectly in other cases > > > > too? > > > > > > TYPE_NAMES is compared because IIRC typedefs are recorded as variants > > > and 'const T' isn't the same as 'const int' with typedef int T. > > > > so if it is intentional that it differentiates between > > > > struct foo > > > > and > > > > typedef struct foo bar > > > > then I will change c_update_type_canonical to not use it, > > because both types should have the same TYPE_CANONICAL > > The check is supposed to differentiate between variants and all variants > have the same TYPE_CANONICAL so I'm not sure why you considered using > this function for canonical type compute? I've done that and that was because build_qualified_type uses that predicate, where qualified types created by build_qualified_type have as TYPE_CANONICAL the qualified type of the main variant of the canonical type, while in all other cases TYPE_CANONICAL is just the main variant of the type. Guess we could also just do if (TYPE_QUALS (x) == TYPE_QUALS (t)) TYPE_CANONICAL (x) = TYPE_CANONICAL (t); else if (TYPE_CANONICAL (t) != t || TYPE_QUALS (x) != TYPE_QUALS (TYPE_CANONICAL (t))) TYPE_CANONICAL (x) = build_qualified_type (TYPE_CANONICAL (t), TYPE_QUALS (x)); else TYPE_CANONICAL (x) = x; or so. Jakub
Re: check_qualified_type
On Mon, 17 Jun 2024, Martin Uecker wrote: > Am Montag, dem 17.06.2024 um 08:01 +0200 schrieb Richard Biener via Gcc: > > On Sun, 16 Jun 2024, Martin Uecker wrote: > > > > > > > > > > > I am trying to understand what check_qualified_type > > > does exactly. The direct comparison of TYPE_NAMES seems incorrect > > > for C and its use is c_update_type_canonical then causes > > > PR114930 and PR115502. In the later function I think > > > it is not really needed and I guess one could simply remove > > > it, but I wonder if it works incorrectly in other cases > > > too? > > > > TYPE_NAMES is compared because IIRC typedefs are recorded as variants > > and 'const T' isn't the same as 'const int' with typedef int T. > > so if it is intentional that it differentiates between > > struct foo > > and > > typedef struct foo bar > > then I will change c_update_type_canonical to not use it, > because both types should have the same TYPE_CANONICAL The check is supposed to differentiate between variants and all variants have the same TYPE_CANONICAL so I'm not sure why you considered using this function for canonical type compute? Richard.
Re: check_qualified_type
Am Montag, dem 17.06.2024 um 08:01 +0200 schrieb Richard Biener via Gcc: > On Sun, 16 Jun 2024, Martin Uecker wrote: > > > > > > > I am trying to understand what check_qualified_type > > does exactly. The direct comparison of TYPE_NAMES seems incorrect > > for C and its use is c_update_type_canonical then causes > > PR114930 and PR115502. In the later function I think > > it is not really needed and I guess one could simply remove > > it, but I wonder if it works incorrectly in other cases > > too? > > TYPE_NAMES is compared because IIRC typedefs are recorded as variants > and 'const T' isn't the same as 'const int' with typedef int T. so if it is intentional that it differentiates between struct foo and typedef struct foo bar then I will change c_update_type_canonical to not use it, because both types should have the same TYPE_CANONICAL Martin
Re: check_qualified_type
On Sun, 16 Jun 2024, Martin Uecker wrote: > > > I am trying to understand what check_qualified_type > does exactly. The direct comparison of TYPE_NAMES seems incorrect > for C and its use is c_update_type_canonical then causes > PR114930 and PR115502. In the later function I think > it is not really needed and I guess one could simply remove > it, but I wonder if it works incorrectly in other cases > too? TYPE_NAMES is compared because IIRC typedefs are recorded as variants and 'const T' isn't the same as 'const int' with typedef int T. Richard.
check_qualified_type
I am trying to understand what check_qualified_type does exactly. The direct comparison of TYPE_NAMES seems incorrect for C and its use is c_update_type_canonical then causes PR114930 and PR115502. In the later function I think it is not really needed and I guess one could simply remove it, but I wonder if it works incorrectly in other cases too? Martin
[Bug c++/98333] [10 Regression] ICE in check_qualified_type, at tree.c:6593 since r10-1280-g78f7607db4c53f8c
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98333 Marek Polacek changed: What|Removed |Added Status|ASSIGNED|RESOLVED Resolution|--- |FIXED --- Comment #7 from Marek Polacek --- Fixed for GCC 10.4/11.
[Bug c++/98333] [10 Regression] ICE in check_qualified_type, at tree.c:6593 since r10-1280-g78f7607db4c53f8c
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98333
--- Comment #6 from CVS Commits ---
The releases/gcc-10 branch has been updated by Marek Polacek
:
https://gcc.gnu.org/g:3bb551c6751304fb2d51cc8569f066dd8a9090e2
commit r10-9682-g3bb551c6751304fb2d51cc8569f066dd8a9090e2
Author: Marek Polacek
Date: Fri Jan 8 15:48:41 2021 -0500
c++: ICE when late parsing noexcept/NSDMI [PR98333]
Since certain members of a class are a complete-class context
[class.mem.general]p7, we delay their parsing untile the whole class has
been parsed. For instance, NSDMIs and noexcept-specifiers. The order
in which we perform this delayed parsing matters; we were first parsing
NSDMIs and only they did we parse noexcept-specifiers. That turns out
to be wrong: since NSDMIs may use noexcept-specifiers, we must process
noexcept-specifiers first. Otherwise we'll ICE in code that doesn't
expect to see DEFERRED_PARSE.
This doesn't just shift the problem, noexcept-specifiers can use members
with a NSDMI just fine, and I've also tested a similar test with this
member function:
bool f() { return __has_nothrow_constructor (S); }
and that compiled fine too.
gcc/cp/ChangeLog:
PR c++/98333
* parser.c (cp_parser_class_specifier_1): Perform late-parsing
of NSDMIs before late-parsing of noexcept-specifiers.
gcc/testsuite/ChangeLog:
PR c++/98333
* g++.dg/cpp0x/noexcept62.C: New test.
(cherry picked from commit c37f1d4081f5a19e39192d13e2a3acea13662e5a)
[Bug c++/98333] [10 Regression] ICE in check_qualified_type, at tree.c:6593 since r10-1280-g78f7607db4c53f8c
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98333 Richard Biener changed: What|Removed |Added Target Milestone|10.3|10.4 --- Comment #5 from Richard Biener --- GCC 10.3 is being released, retargeting bugs to GCC 10.4.
[Bug c++/98333] [10 Regression] ICE in check_qualified_type, at tree.c:6593 since r10-1280-g78f7607db4c53f8c
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98333 Marek Polacek changed: What|Removed |Added Summary|[10/11 Regression] ICE in |[10 Regression] ICE in |check_qualified_type, at|check_qualified_type, at |tree.c:6593 since |tree.c:6593 since |r10-1280-g78f7607db4c53f8c |r10-1280-g78f7607db4c53f8c --- Comment #4 from Marek Polacek --- Fixed on trunk so far.
[Bug c++/98333] [10/11 Regression] ICE in check_qualified_type, at tree.c:6593 since r10-1280-g78f7607db4c53f8c
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98333
--- Comment #3 from CVS Commits ---
The master branch has been updated by Marek Polacek :
https://gcc.gnu.org/g:c37f1d4081f5a19e39192d13e2a3acea13662e5a
commit r11-6799-gc37f1d4081f5a19e39192d13e2a3acea13662e5a
Author: Marek Polacek
Date: Fri Jan 8 15:48:41 2021 -0500
c++: ICE when late parsing noexcept/NSDMI [PR98333]
Since certain members of a class are a complete-class context
[class.mem.general]p7, we delay their parsing untile the whole class has
been parsed. For instance, NSDMIs and noexcept-specifiers. The order
in which we perform this delayed parsing matters; we were first parsing
NSDMIs and only they did we parse noexcept-specifiers. That turns out
to be wrong: since NSDMIs may use noexcept-specifiers, we must process
noexcept-specifiers first. Otherwise we'll ICE in code that doesn't
expect to see DEFERRED_PARSE.
This doesn't just shift the problem, noexcept-specifiers can use members
with a NSDMI just fine, and I've also tested a similar test with this
member function:
bool f() { return __has_nothrow_constructor (S); }
and that compiled fine too.
gcc/cp/ChangeLog:
PR c++/98333
* parser.c (cp_parser_class_specifier_1): Perform late-parsing
of NSDMIs before late-parsing of noexcept-specifiers.
gcc/testsuite/ChangeLog:
PR c++/98333
* g++.dg/cpp0x/noexcept62.C: New test.
[Bug c++/98333] [10/11 Regression] ICE in check_qualified_type, at tree.c:6593 since r10-1280-g78f7607db4c53f8c
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98333
Marek Polacek changed:
What|Removed |Added
Keywords|ice-on-invalid-code |ice-on-valid-code
--- Comment #2 from Marek Polacek ---
Modified valid code:
constexpr int b = false;
struct T {
template struct S
{ S () noexcept (b) {} };
int a = __has_nothrow_constructor (S);
};
[Bug c++/98333] [10/11 Regression] ICE in check_qualified_type, at tree.c:6593 since r10-1280-g78f7607db4c53f8c
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98333 Richard Biener changed: What|Removed |Added Priority|P3 |P2
[Bug c++/98333] [10/11 Regression] ICE in check_qualified_type, at tree.c:6593 since r10-1280-g78f7607db4c53f8c
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98333 Marek Polacek changed: What|Removed |Added Assignee|unassigned at gcc dot gnu.org |mpolacek at gcc dot gnu.org Target Milestone|--- |10.3 Status|NEW |ASSIGNED
[Bug c++/98333] [10/11 Regression] ICE in check_qualified_type, at tree.c:6593 since r10-1280-g78f7607db4c53f8c
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98333 Martin Liška changed: What|Removed |Added Last reconfirmed||2020-12-17 Status|UNCONFIRMED |NEW Summary|[10/11 Regression] ICE in |[10/11 Regression] ICE in |check_qualified_type, at|check_qualified_type, at |tree.c:6593 |tree.c:6593 since ||r10-1280-g78f7607db4c53f8c Ever confirmed|0 |1 CC||marxin at gcc dot gnu.org, ||mpolacek at gcc dot gnu.org --- Comment #1 from Martin Liška --- Started with r10-1280-g78f7607db4c53f8c.
[Bug c++/98333] New: [10/11 Regression] ICE in check_qualified_type, at tree.c:6593
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98333
Bug ID: 98333
Summary: [10/11 Regression] ICE in check_qualified_type, at
tree.c:6593
Product: gcc
Version: 11.0
Status: UNCONFIRMED
Severity: normal
Priority: P3
Component: c++
Assignee: unassigned at gcc dot gnu.org
Reporter: gs...@t-online.de
Target Milestone: ---
Changed between 20190616 and 20190623, with an extra "()" :
$ cat z1.cc
struct T
{
template struct S
{ S () noexcept () {} };
int a = __has_nothrow_constructor (S);
};
$ g++-11-20201213 -c z1.cc
z1.cc: In instantiation of 'T::S< >::S() [with
= int]':
z1.cc:5:44: required from here
z1.cc:4:5: internal compiler error: Segmentation fault
4 | { S () noexcept () {} };
| ^
0xc8f5ff crash_signal
../../gcc/toplev.c:327
0xf00280 check_qualified_type(tree_node const*, tree_node const*, int)
../../gcc/tree.c:6593
0xf00384 get_qualified_type(tree_node*, int)
../../gcc/tree.c:6625
0xf0d6b3 build_qualified_type(tree_node*, int)
../../gcc/tree.c:6656
0x654566 strip_top_quals(tree_node*)
../../gcc/cp/call.c:1177
0x654566 standard_conversion
../../gcc/cp/call.c:1202
0x656c7a implicit_conversion_1
../../gcc/cp/call.c:2008
0x656c7a implicit_conversion
../../gcc/cp/call.c:2108
0x6669df build_converted_constant_expr_internal
../../gcc/cp/call.c:4369
0x6e5289 build_noexcept_spec(tree_node*, int)
../../gcc/cp/except.c:1234
0x775ce9 maybe_instantiate_noexcept(tree_node*, int)
../../gcc/cp/pt.c:25545
0x7759f8 maybe_instantiate_noexcept(tree_node*, int)
../../gcc/cp/pt.c:25481
0x79d235 trait_expr_value
../../gcc/cp/semantics.c:10246
0x7a7c6a finish_trait_expr(unsigned int, cp_trait_kind, tree_node*, tree_node*)
../../gcc/cp/semantics.c:10449
0x748dd7 cp_parser_trait_expr
../../gcc/cp/parser.c:10749
0x73f58c cp_parser_primary_expression
../../gcc/cp/parser.c:5777
0x741b75 cp_parser_postfix_expression
../../gcc/cp/parser.c:7491
0x751a35 cp_parser_unary_expression
../../gcc/cp/parser.c:8808
0x72b84f cp_parser_cast_expression
../../gcc/cp/parser.c:9712
0x72c082 cp_parser_binary_expression
../../gcc/cp/parser.c:9814
