* [PATCH] c++: Fix ICE in check_local_shadow with enum [PR95560]
@ 2020-06-10 21:11 Marek Polacek
2020-06-11 19:32 ` Jason Merrill
0 siblings, 1 reply; 6+ messages in thread
From: Marek Polacek @ 2020-06-10 21:11 UTC (permalink / raw)
To: Jason Merrill, GCC Patches
Another indication that perhaps this warning is emitted too early. We
crash because same_type_p gets a null type: we have an enumerator
without a fixed underlying type and finish_enum_value_list hasn't yet
run. So check if the type is null before calling same_type_p.
(This is a regression and this fix is suitable for backporting. Delaying
the warning would not be suitable to backport.)
Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk/10/9?
gcc/cp/ChangeLog:
PR c++/95560
* name-lookup.c (check_local_shadow): Check if types are
non-null before calling same_type_p.
gcc/testsuite/ChangeLog:
PR c++/95560
* g++.dg/warn/Wshadow-local-3.C: New test.
---
gcc/cp/name-lookup.c | 4 +++-
gcc/testsuite/g++.dg/warn/Wshadow-local-3.C | 7 +++++++
2 files changed, 10 insertions(+), 1 deletion(-)
create mode 100644 gcc/testsuite/g++.dg/warn/Wshadow-local-3.C
diff --git a/gcc/cp/name-lookup.c b/gcc/cp/name-lookup.c
index 2ff85f1cf5e..159c98a67cd 100644
--- a/gcc/cp/name-lookup.c
+++ b/gcc/cp/name-lookup.c
@@ -2762,7 +2762,9 @@ check_local_shadow (tree decl)
enum opt_code warning_code;
if (warn_shadow)
warning_code = OPT_Wshadow;
- else if (same_type_p (TREE_TYPE (old), TREE_TYPE (decl))
+ else if ((TREE_TYPE (old)
+ && TREE_TYPE (decl)
+ && same_type_p (TREE_TYPE (old), TREE_TYPE (decl)))
|| TREE_CODE (decl) == TYPE_DECL
|| TREE_CODE (old) == TYPE_DECL
|| (!dependent_type_p (TREE_TYPE (decl))
diff --git a/gcc/testsuite/g++.dg/warn/Wshadow-local-3.C b/gcc/testsuite/g++.dg/warn/Wshadow-local-3.C
new file mode 100644
index 00000000000..fd743eca347
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/Wshadow-local-3.C
@@ -0,0 +1,7 @@
+// PR c++/95560
+// { dg-do compile { target c++11 } }
+
+template <typename> void fn1() {
+ bool ready;
+ enum class State { ready };
+}
base-commit: 3a391adf7a38780f8d01dbac08a2a143fc80b469
--
Marek Polacek • Red Hat, Inc. • 300 A St, Boston, MA
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] c++: Fix ICE in check_local_shadow with enum [PR95560]
2020-06-10 21:11 [PATCH] c++: Fix ICE in check_local_shadow with enum [PR95560] Marek Polacek
@ 2020-06-11 19:32 ` Jason Merrill
2020-06-11 19:34 ` Jason Merrill
2020-06-16 1:20 ` Marek Polacek
0 siblings, 2 replies; 6+ messages in thread
From: Jason Merrill @ 2020-06-11 19:32 UTC (permalink / raw)
To: Marek Polacek, GCC Patches
On 6/10/20 5:11 PM, Marek Polacek wrote:
> Another indication that perhaps this warning is emitted too early. We
> crash because same_type_p gets a null type: we have an enumerator
> without a fixed underlying type and finish_enum_value_list hasn't yet
> run. So check if the type is null before calling same_type_p.
Hmm, I wonder why we use NULL_TREE for the type of uninitialized
enumerators in a template; why not give them integer_type_node temporarily?
> (This is a regression and this fix is suitable for backporting. Delaying
> the warning would not be suitable to backport.)
>
> Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk/10/9?
>
> gcc/cp/ChangeLog:
>
> PR c++/95560
> * name-lookup.c (check_local_shadow): Check if types are
> non-null before calling same_type_p.
>
> gcc/testsuite/ChangeLog:
>
> PR c++/95560
> * g++.dg/warn/Wshadow-local-3.C: New test.
> ---
> gcc/cp/name-lookup.c | 4 +++-
> gcc/testsuite/g++.dg/warn/Wshadow-local-3.C | 7 +++++++
> 2 files changed, 10 insertions(+), 1 deletion(-)
> create mode 100644 gcc/testsuite/g++.dg/warn/Wshadow-local-3.C
>
> diff --git a/gcc/cp/name-lookup.c b/gcc/cp/name-lookup.c
> index 2ff85f1cf5e..159c98a67cd 100644
> --- a/gcc/cp/name-lookup.c
> +++ b/gcc/cp/name-lookup.c
> @@ -2762,7 +2762,9 @@ check_local_shadow (tree decl)
> enum opt_code warning_code;
> if (warn_shadow)
> warning_code = OPT_Wshadow;
> - else if (same_type_p (TREE_TYPE (old), TREE_TYPE (decl))
> + else if ((TREE_TYPE (old)
> + && TREE_TYPE (decl)
> + && same_type_p (TREE_TYPE (old), TREE_TYPE (decl)))
> || TREE_CODE (decl) == TYPE_DECL
> || TREE_CODE (old) == TYPE_DECL
> || (!dependent_type_p (TREE_TYPE (decl))
> diff --git a/gcc/testsuite/g++.dg/warn/Wshadow-local-3.C b/gcc/testsuite/g++.dg/warn/Wshadow-local-3.C
> new file mode 100644
> index 00000000000..fd743eca347
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/warn/Wshadow-local-3.C
> @@ -0,0 +1,7 @@
> +// PR c++/95560
> +// { dg-do compile { target c++11 } }
> +
> +template <typename> void fn1() {
> + bool ready;
> + enum class State { ready };
> +}
>
> base-commit: 3a391adf7a38780f8d01dbac08a2a143fc80b469
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] c++: Fix ICE in check_local_shadow with enum [PR95560]
2020-06-11 19:32 ` Jason Merrill
@ 2020-06-11 19:34 ` Jason Merrill
2020-06-11 19:34 ` Jason Merrill
2020-06-16 1:20 ` Marek Polacek
1 sibling, 1 reply; 6+ messages in thread
From: Jason Merrill @ 2020-06-11 19:34 UTC (permalink / raw)
To: Marek Polacek, GCC Patches
On 6/11/20 3:32 PM, Jason Merrill wrote:
> On 6/10/20 5:11 PM, Marek Polacek wrote:
>> Another indication that perhaps this warning is emitted too early. We
>> crash because same_type_p gets a null type: we have an enumerator
>> without a fixed underlying type and finish_enum_value_list hasn't yet
>> run. So check if the type is null before calling same_type_p.
>
> Hmm, I wonder why we use NULL_TREE for the type of uninitialized
> enumerators in a template; why not give them integer_type_node temporarily?
But this patch is OK for 10.2.
>> (This is a regression and this fix is suitable for backporting. Delaying
>> the warning would not be suitable to backport.)
>>
>> Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk/10/9?
>>
>> gcc/cp/ChangeLog:
>>
>> PR c++/95560
>> * name-lookup.c (check_local_shadow): Check if types are
>> non-null before calling same_type_p.
>>
>> gcc/testsuite/ChangeLog:
>>
>> PR c++/95560
>> * g++.dg/warn/Wshadow-local-3.C: New test.
>> ---
>> gcc/cp/name-lookup.c | 4 +++-
>> gcc/testsuite/g++.dg/warn/Wshadow-local-3.C | 7 +++++++
>> 2 files changed, 10 insertions(+), 1 deletion(-)
>> create mode 100644 gcc/testsuite/g++.dg/warn/Wshadow-local-3.C
>>
>> diff --git a/gcc/cp/name-lookup.c b/gcc/cp/name-lookup.c
>> index 2ff85f1cf5e..159c98a67cd 100644
>> --- a/gcc/cp/name-lookup.c
>> +++ b/gcc/cp/name-lookup.c
>> @@ -2762,7 +2762,9 @@ check_local_shadow (tree decl)
>> enum opt_code warning_code;
>> if (warn_shadow)
>> warning_code = OPT_Wshadow;
>> - else if (same_type_p (TREE_TYPE (old), TREE_TYPE (decl))
>> + else if ((TREE_TYPE (old)
>> + && TREE_TYPE (decl)
>> + && same_type_p (TREE_TYPE (old), TREE_TYPE (decl)))
>> || TREE_CODE (decl) == TYPE_DECL
>> || TREE_CODE (old) == TYPE_DECL
>> || (!dependent_type_p (TREE_TYPE (decl))
>> diff --git a/gcc/testsuite/g++.dg/warn/Wshadow-local-3.C
>> b/gcc/testsuite/g++.dg/warn/Wshadow-local-3.C
>> new file mode 100644
>> index 00000000000..fd743eca347
>> --- /dev/null
>> +++ b/gcc/testsuite/g++.dg/warn/Wshadow-local-3.C
>> @@ -0,0 +1,7 @@
>> +// PR c++/95560
>> +// { dg-do compile { target c++11 } }
>> +
>> +template <typename> void fn1() {
>> + bool ready;
>> + enum class State { ready };
>> +}
>>
>> base-commit: 3a391adf7a38780f8d01dbac08a2a143fc80b469
>>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] c++: Fix ICE in check_local_shadow with enum [PR95560]
2020-06-11 19:34 ` Jason Merrill
@ 2020-06-11 19:34 ` Jason Merrill
0 siblings, 0 replies; 6+ messages in thread
From: Jason Merrill @ 2020-06-11 19:34 UTC (permalink / raw)
To: Marek Polacek, GCC Patches
On 6/11/20 3:34 PM, Jason Merrill wrote:
> On 6/11/20 3:32 PM, Jason Merrill wrote:
>> On 6/10/20 5:11 PM, Marek Polacek wrote:
>>> Another indication that perhaps this warning is emitted too early. We
>>> crash because same_type_p gets a null type: we have an enumerator
>>> without a fixed underlying type and finish_enum_value_list hasn't yet
>>> run. So check if the type is null before calling same_type_p.
>>
>> Hmm, I wonder why we use NULL_TREE for the type of uninitialized
>> enumerators in a template; why not give them integer_type_node
>> temporarily?
>
> But this patch is OK for 10.2.
And 9.
>
>>> (This is a regression and this fix is suitable for backporting.
>>> Delaying
>>> the warning would not be suitable to backport.)
>>>
>>> Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk/10/9?
>>>
>>> gcc/cp/ChangeLog:
>>>
>>> PR c++/95560
>>> * name-lookup.c (check_local_shadow): Check if types are
>>> non-null before calling same_type_p.
>>>
>>> gcc/testsuite/ChangeLog:
>>>
>>> PR c++/95560
>>> * g++.dg/warn/Wshadow-local-3.C: New test.
>>> ---
>>> gcc/cp/name-lookup.c | 4 +++-
>>> gcc/testsuite/g++.dg/warn/Wshadow-local-3.C | 7 +++++++
>>> 2 files changed, 10 insertions(+), 1 deletion(-)
>>> create mode 100644 gcc/testsuite/g++.dg/warn/Wshadow-local-3.C
>>>
>>> diff --git a/gcc/cp/name-lookup.c b/gcc/cp/name-lookup.c
>>> index 2ff85f1cf5e..159c98a67cd 100644
>>> --- a/gcc/cp/name-lookup.c
>>> +++ b/gcc/cp/name-lookup.c
>>> @@ -2762,7 +2762,9 @@ check_local_shadow (tree decl)
>>> enum opt_code warning_code;
>>> if (warn_shadow)
>>> warning_code = OPT_Wshadow;
>>> - else if (same_type_p (TREE_TYPE (old), TREE_TYPE (decl))
>>> + else if ((TREE_TYPE (old)
>>> + && TREE_TYPE (decl)
>>> + && same_type_p (TREE_TYPE (old), TREE_TYPE (decl)))
>>> || TREE_CODE (decl) == TYPE_DECL
>>> || TREE_CODE (old) == TYPE_DECL
>>> || (!dependent_type_p (TREE_TYPE (decl))
>>> diff --git a/gcc/testsuite/g++.dg/warn/Wshadow-local-3.C
>>> b/gcc/testsuite/g++.dg/warn/Wshadow-local-3.C
>>> new file mode 100644
>>> index 00000000000..fd743eca347
>>> --- /dev/null
>>> +++ b/gcc/testsuite/g++.dg/warn/Wshadow-local-3.C
>>> @@ -0,0 +1,7 @@
>>> +// PR c++/95560
>>> +// { dg-do compile { target c++11 } }
>>> +
>>> +template <typename> void fn1() {
>>> + bool ready;
>>> + enum class State { ready };
>>> +}
>>>
>>> base-commit: 3a391adf7a38780f8d01dbac08a2a143fc80b469
>>>
>>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] c++: Fix ICE in check_local_shadow with enum [PR95560]
2020-06-11 19:32 ` Jason Merrill
2020-06-11 19:34 ` Jason Merrill
@ 2020-06-16 1:20 ` Marek Polacek
2020-06-16 16:06 ` Jason Merrill
1 sibling, 1 reply; 6+ messages in thread
From: Marek Polacek @ 2020-06-16 1:20 UTC (permalink / raw)
To: Jason Merrill; +Cc: GCC Patches
On Thu, Jun 11, 2020 at 03:32:14PM -0400, Jason Merrill wrote:
> On 6/10/20 5:11 PM, Marek Polacek wrote:
> > Another indication that perhaps this warning is emitted too early. We
> > crash because same_type_p gets a null type: we have an enumerator
> > without a fixed underlying type and finish_enum_value_list hasn't yet
> > run. So check if the type is null before calling same_type_p.
>
> Hmm, I wonder why we use NULL_TREE for the type of uninitialized enumerators
> in a template; why not give them integer_type_node temporarily?
That breaks enum22.C:
template <class T>
struct A {
enum e_ : unsigned char { Z_, E_=sizeof(Z_) };
};
static_assert ( A<double>::E_ == 1, "E_ should be 1");
If we give 'Z_' a type, it's no longer instantiation-dependent, so sizeof(Z_)
immediately evaluates to 4. Whereas if it doesn't have a type, in the template
we create a SIZEOF_EXPR and only evaluate when instantiating (to 1).
This sounded like a problem big enough for me not to pursue this any further.
Do you want me to try anything else or is the original patch ok?
Marek
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] c++: Fix ICE in check_local_shadow with enum [PR95560]
2020-06-16 1:20 ` Marek Polacek
@ 2020-06-16 16:06 ` Jason Merrill
0 siblings, 0 replies; 6+ messages in thread
From: Jason Merrill @ 2020-06-16 16:06 UTC (permalink / raw)
To: Marek Polacek; +Cc: GCC Patches
On 6/15/20 9:20 PM, Marek Polacek wrote:
> On Thu, Jun 11, 2020 at 03:32:14PM -0400, Jason Merrill wrote:
>> On 6/10/20 5:11 PM, Marek Polacek wrote:
>>> Another indication that perhaps this warning is emitted too early. We
>>> crash because same_type_p gets a null type: we have an enumerator
>>> without a fixed underlying type and finish_enum_value_list hasn't yet
>>> run. So check if the type is null before calling same_type_p.
>>
>> Hmm, I wonder why we use NULL_TREE for the type of uninitialized enumerators
>> in a template; why not give them integer_type_node temporarily?
>
> That breaks enum22.C:
>
> template <class T>
> struct A {
> enum e_ : unsigned char { Z_, E_=sizeof(Z_) };
> };
>
> static_assert ( A<double>::E_ == 1, "E_ should be 1");
>
> If we give 'Z_' a type, it's no longer instantiation-dependent, so sizeof(Z_)
> immediately evaluates to 4. Whereas if it doesn't have a type, in the template
> we create a SIZEOF_EXPR and only evaluate when instantiating (to 1).
>
> This sounded like a problem big enough for me not to pursue this any further.
> Do you want me to try anything else or is the original patch ok?
The original patch is OK, thanks.
Jason
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-06-16 16:06 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-10 21:11 [PATCH] c++: Fix ICE in check_local_shadow with enum [PR95560] Marek Polacek
2020-06-11 19:32 ` Jason Merrill
2020-06-11 19:34 ` Jason Merrill
2020-06-11 19:34 ` Jason Merrill
2020-06-16 1:20 ` Marek Polacek
2020-06-16 16:06 ` Jason Merrill
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).