Message ID | 65a4fdbe.050a0220.b4d36.7277@mx.google.com |
---|---|
State | New |
Headers | show |
Series | c++: Fix ENABLE_SCOPE_CHECKING printing | expand |
On 1/15/24 04:41, Nathaniel Shead wrote: > While working on another bug, I noticed the ENABLE_SCOPE_CHECKING macro > and thought to try it out. It caused selftest to ICE. This patch is a > minimal fix to get it working again. > > Probably this should use a test to stop this regressing again in the > future the next time new scope-kinds are added, but given it's dependent > on a (almost certainly rarely-used) build-time macro I'm not sure > exactly how you would do that? > > Or alternatively I could add a `sk_count` to the end of the scope kind > list and `static_assert` that the size of the descriptor list matches? That sounds good. > (Also not sure if this would be appropriate for stage 4 or if it should > wait till next stage 1. I suppose this fixes a regression but I suspect > this has been broken for a very long time.) I think it's OK now since it doesn't affect the normal codepath. > -- >8 -- > > The lists of scope kinds used by ENABLE_SCOPE_CHECKING don't seem to > have been updated in a long while, causing ICEs and confusing output. > This patch brings the list into line. > > gcc/cp/ChangeLog: > > * name-lookup.cc (cp_binding_level_descriptor): Add missing > scope kinds. > > Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com> > --- > gcc/cp/name-lookup.cc | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/gcc/cp/name-lookup.cc b/gcc/cp/name-lookup.cc > index d827d337d3b..2e93ed183f1 100644 > --- a/gcc/cp/name-lookup.cc > +++ b/gcc/cp/name-lookup.cc > @@ -4464,11 +4464,16 @@ cp_binding_level_descriptor (cp_binding_level *scope) > "try-scope", > "catch-scope", > "for-scope", > + "cond-init-scope", > + "stmt-expr-scope", > "function-parameter-scope", > "class-scope", > + "enum-scope", > "namespace-scope", > "template-parameter-scope", > - "template-explicit-spec-scope" > + "template-explicit-spec-scope", > + "transaction-scope", > + "openmp-scope" > }; > const scope_kind kind = scope->explicit_spec_p > ? sk_template_spec : scope->kind;
On Mon, Jan 15, 2024 at 04:04:49PM -0500, Jason Merrill wrote: > On 1/15/24 04:41, Nathaniel Shead wrote: > > While working on another bug, I noticed the ENABLE_SCOPE_CHECKING macro > > and thought to try it out. It caused selftest to ICE. This patch is a > > minimal fix to get it working again. > > > > Probably this should use a test to stop this regressing again in the > > future the next time new scope-kinds are added, but given it's dependent > > on a (almost certainly rarely-used) build-time macro I'm not sure > > exactly how you would do that? > > > > Or alternatively I could add a `sk_count` to the end of the scope kind > > list and `static_assert` that the size of the descriptor list matches? > > That sounds good. > > > (Also not sure if this would be appropriate for stage 4 or if it should > > wait till next stage 1. I suppose this fixes a regression but I suspect > > this has been broken for a very long time.) > > I think it's OK now since it doesn't affect the normal codepath. > Thanks. Here's an updated version. Bootstrapped on x86_64-pc-linux-gnu, OK for trunk if regtesting passes? I've also built stage 1 with `-DENABLE_SCOPE_CHECKING` and verified it didn't immediately fail, but regtesting fails due to all the extra output. As an option specifically for developing GCC itself I assume that's OK. -- >8 -- The lists of scope kinds used by ENABLE_SCOPE_CHECKING don't seem to have been updated in a long while, causing ICEs and confusing output. This patch brings the list into line. Additionally, the comment on 'explicit_spec_p' says that the flag is only valid if kind is 'sk_template_parms', so we rewrite the condition to be more obviously correct here. gcc/cp/ChangeLog: * name-lookup.h (enum scope_kind): Add 'sk_count'. * name-lookup.cc (cp_binding_level_descriptor): Add missing scope kinds. Add assertion that the list is up to date. Fix handling of explicit_spec_p. Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com> --- gcc/cp/name-lookup.cc | 15 ++++++++++++--- gcc/cp/name-lookup.h | 3 ++- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/gcc/cp/name-lookup.cc b/gcc/cp/name-lookup.cc index d827d337d3b..15b5fba6297 100644 --- a/gcc/cp/name-lookup.cc +++ b/gcc/cp/name-lookup.cc @@ -4464,14 +4464,23 @@ cp_binding_level_descriptor (cp_binding_level *scope) "try-scope", "catch-scope", "for-scope", + "cond-init-scope", + "stmt-expr-scope", "function-parameter-scope", "class-scope", + "enum-scope", "namespace-scope", "template-parameter-scope", - "template-explicit-spec-scope" + "template-explicit-spec-scope", + "transaction-scope", + "openmp-scope" }; - const scope_kind kind = scope->explicit_spec_p - ? sk_template_spec : scope->kind; + static_assert (ARRAY_SIZE (scope_kind_names) == sk_count, + "must keep names aligned with scope_kind enum"); + + scope_kind kind = scope->kind; + if (kind == sk_template_parms && scope->explicit_spec_p) + kind = sk_template_spec; return scope_kind_names[kind]; } diff --git a/gcc/cp/name-lookup.h b/gcc/cp/name-lookup.h index 4f8454ee35e..d2371323337 100644 --- a/gcc/cp/name-lookup.h +++ b/gcc/cp/name-lookup.h @@ -213,7 +213,8 @@ enum scope_kind { explicit specialization is introduced by "template <>", this scope is always empty. */ sk_transaction, /* A synchronized or atomic statement. */ - sk_omp /* An OpenMP structured block. */ + sk_omp, /* An OpenMP structured block. */ + sk_count /* Number of scope_kind enumerations. */ }; struct GTY(()) cp_class_binding {
On 1/16/24 06:59, Nathaniel Shead wrote: > On Mon, Jan 15, 2024 at 04:04:49PM -0500, Jason Merrill wrote: >> On 1/15/24 04:41, Nathaniel Shead wrote: >>> While working on another bug, I noticed the ENABLE_SCOPE_CHECKING macro >>> and thought to try it out. It caused selftest to ICE. This patch is a >>> minimal fix to get it working again. >>> >>> Probably this should use a test to stop this regressing again in the >>> future the next time new scope-kinds are added, but given it's dependent >>> on a (almost certainly rarely-used) build-time macro I'm not sure >>> exactly how you would do that? >>> >>> Or alternatively I could add a `sk_count` to the end of the scope kind >>> list and `static_assert` that the size of the descriptor list matches? >> >> That sounds good. >> >>> (Also not sure if this would be appropriate for stage 4 or if it should >>> wait till next stage 1. I suppose this fixes a regression but I suspect >>> this has been broken for a very long time.) >> >> I think it's OK now since it doesn't affect the normal codepath. >> > > Thanks. Here's an updated version. > > Bootstrapped on x86_64-pc-linux-gnu, OK for trunk if regtesting passes? OK. > I've also built stage 1 with `-DENABLE_SCOPE_CHECKING` and verified it > didn't immediately fail, but regtesting fails due to all the extra > output. As an option specifically for developing GCC itself I assume > that's OK. > > -- >8 -- > > The lists of scope kinds used by ENABLE_SCOPE_CHECKING don't seem to > have been updated in a long while, causing ICEs and confusing output. > This patch brings the list into line. > > Additionally, the comment on 'explicit_spec_p' says that the flag is > only valid if kind is 'sk_template_parms', so we rewrite the condition > to be more obviously correct here. > > gcc/cp/ChangeLog: > > * name-lookup.h (enum scope_kind): Add 'sk_count'. > * name-lookup.cc (cp_binding_level_descriptor): Add missing > scope kinds. Add assertion that the list is up to date. Fix > handling of explicit_spec_p. > > Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com> > --- > gcc/cp/name-lookup.cc | 15 ++++++++++++--- > gcc/cp/name-lookup.h | 3 ++- > 2 files changed, 14 insertions(+), 4 deletions(-) > > diff --git a/gcc/cp/name-lookup.cc b/gcc/cp/name-lookup.cc > index d827d337d3b..15b5fba6297 100644 > --- a/gcc/cp/name-lookup.cc > +++ b/gcc/cp/name-lookup.cc > @@ -4464,14 +4464,23 @@ cp_binding_level_descriptor (cp_binding_level *scope) > "try-scope", > "catch-scope", > "for-scope", > + "cond-init-scope", > + "stmt-expr-scope", > "function-parameter-scope", > "class-scope", > + "enum-scope", > "namespace-scope", > "template-parameter-scope", > - "template-explicit-spec-scope" > + "template-explicit-spec-scope", > + "transaction-scope", > + "openmp-scope" > }; > - const scope_kind kind = scope->explicit_spec_p > - ? sk_template_spec : scope->kind; > + static_assert (ARRAY_SIZE (scope_kind_names) == sk_count, > + "must keep names aligned with scope_kind enum"); > + > + scope_kind kind = scope->kind; > + if (kind == sk_template_parms && scope->explicit_spec_p) > + kind = sk_template_spec; > > return scope_kind_names[kind]; > } > diff --git a/gcc/cp/name-lookup.h b/gcc/cp/name-lookup.h > index 4f8454ee35e..d2371323337 100644 > --- a/gcc/cp/name-lookup.h > +++ b/gcc/cp/name-lookup.h > @@ -213,7 +213,8 @@ enum scope_kind { > explicit specialization is introduced by > "template <>", this scope is always empty. */ > sk_transaction, /* A synchronized or atomic statement. */ > - sk_omp /* An OpenMP structured block. */ > + sk_omp, /* An OpenMP structured block. */ > + sk_count /* Number of scope_kind enumerations. */ > }; > > struct GTY(()) cp_class_binding {
diff --git a/gcc/cp/name-lookup.cc b/gcc/cp/name-lookup.cc index d827d337d3b..2e93ed183f1 100644 --- a/gcc/cp/name-lookup.cc +++ b/gcc/cp/name-lookup.cc @@ -4464,11 +4464,16 @@ cp_binding_level_descriptor (cp_binding_level *scope) "try-scope", "catch-scope", "for-scope", + "cond-init-scope", + "stmt-expr-scope", "function-parameter-scope", "class-scope", + "enum-scope", "namespace-scope", "template-parameter-scope", - "template-explicit-spec-scope" + "template-explicit-spec-scope", + "transaction-scope", + "openmp-scope" }; const scope_kind kind = scope->explicit_spec_p ? sk_template_spec : scope->kind;
While working on another bug, I noticed the ENABLE_SCOPE_CHECKING macro and thought to try it out. It caused selftest to ICE. This patch is a minimal fix to get it working again. Probably this should use a test to stop this regressing again in the future the next time new scope-kinds are added, but given it's dependent on a (almost certainly rarely-used) build-time macro I'm not sure exactly how you would do that? Or alternatively I could add a `sk_count` to the end of the scope kind list and `static_assert` that the size of the descriptor list matches? (Also not sure if this would be appropriate for stage 4 or if it should wait till next stage 1. I suppose this fixes a regression but I suspect this has been broken for a very long time.) -- >8 -- The lists of scope kinds used by ENABLE_SCOPE_CHECKING don't seem to have been updated in a long while, causing ICEs and confusing output. This patch brings the list into line. gcc/cp/ChangeLog: * name-lookup.cc (cp_binding_level_descriptor): Add missing scope kinds. Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com> --- gcc/cp/name-lookup.cc | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)