| Message ID | orzhu0r8a3.fsf@lxoliva.fsfla.org |
|---|---|
| State | New |
| Headers | show |
| Series | [PR86397] set p_t_decl while canonicalizing eh specs for mangling | expand |
On 11/22/18 6:40 PM, Alexandre Oliva wrote: > Mangling visits the base template function type, prior to template > resolution, and on such types, exception specifications may contain > unresolved noexcept expressions. nothrow_spec_p is called on them > even when exception specifications are not part of function types, and > it rejects unresolved noexcept expressions if processing_template_decl > is not set. The problem here is that the noexcept expression is unresolved even though it isn't dependent; it should have been resolved to 'true'. Ideally with a warning, since using the name of a function as a boolean value is pretty strange. nothrow_spec_p is right to reject this. Jason
On Nov 27, 2018, Jason Merrill <jason@redhat.com> wrote: > On 11/22/18 6:40 PM, Alexandre Oliva wrote: >> Mangling visits the base template function type, prior to template >> resolution, and on such types, exception specifications may contain >> unresolved noexcept expressions. nothrow_spec_p is called on them >> even when exception specifications are not part of function types, and >> it rejects unresolved noexcept expressions if processing_template_decl >> is not set. > The problem here is that the noexcept expression is unresolved even > though it isn't dependent Yeah, but that seems to be on purpose, according to these comments, that precede the hunk below. /* This isn't part of the signature, so don't bother trying to evaluate it until instantiation. */ Taking out the 'flag_noexcept_type && ' subexpr fixes the problem, but defeats the intended deferral of unnecessary computation: diff --git a/gcc/cp/except.c b/gcc/cp/except.c index 3449b59b3cc0..dbd233c94c3a 100644 --- a/gcc/cp/except.c +++ b/gcc/cp/except.c @@ -1193,7 +1193,7 @@ build_noexcept_spec (tree expr, tsubst_flags_t complain) it until instantiation. */ if (TREE_CODE (expr) != DEFERRED_NOEXCEPT && (!processing_template_decl - || (flag_noexcept_type && !value_dependent_expression_p (expr)))) + || !value_dependent_expression_p (expr))) { expr = perform_implicit_conversion_flags (boolean_type_node, expr, complain, In order to retain that deferral, we could change the mangling logic to also refrain from canonicalizing the EH spec when it's not part of the type: diff --git a/gcc/cp/mangle.c b/gcc/cp/mangle.c index 64415894bc57..4c8086c9f9bd 100644 --- a/gcc/cp/mangle.c +++ b/gcc/cp/mangle.c @@ -418,9 +418,12 @@ canonicalize_for_substitution (tree node) || TREE_CODE (node) == METHOD_TYPE) { node = build_ref_qualified_type (node, type_memfn_rqual (orig)); - tree r = canonical_eh_spec (TYPE_RAISES_EXCEPTIONS (orig)); + tree r = TYPE_RAISES_EXCEPTIONS (orig); if (flag_noexcept_type) - node = build_exception_variant (node, r); + { + r = canonical_eh_spec (r); + node = build_exception_variant (node, r); + } else /* Set the warning flag if appropriate. */ write_exception_spec (r); This would bypass the nothrow_spec_p call in canonical_eh_spec at C++1[14], but it might produce unintended -Wnoexcept-type warnings when the noexcept expression would resolve to false. The canonical_eh_spec call wouldn't have avoided it anyway. Which one?
On 11/27/18 11:54 PM, Alexandre Oliva wrote: > On Nov 27, 2018, Jason Merrill <jason@redhat.com> wrote: > >> On 11/22/18 6:40 PM, Alexandre Oliva wrote: >>> Mangling visits the base template function type, prior to template >>> resolution, and on such types, exception specifications may contain >>> unresolved noexcept expressions. nothrow_spec_p is called on them >>> even when exception specifications are not part of function types, and >>> it rejects unresolved noexcept expressions if processing_template_decl >>> is not set. > >> The problem here is that the noexcept expression is unresolved even >> though it isn't dependent > > Yeah, but that seems to be on purpose, according to these comments, that > precede the hunk below. > > /* This isn't part of the signature, so don't bother trying to evaluate > it until instantiation. */ > Taking out the 'flag_noexcept_type && ' subexpr fixes the problem, but > defeats the intended deferral of unnecessary computation: > > diff --git a/gcc/cp/except.c b/gcc/cp/except.c > index 3449b59b3cc0..dbd233c94c3a 100644 > --- a/gcc/cp/except.c > +++ b/gcc/cp/except.c > @@ -1193,7 +1193,7 @@ build_noexcept_spec (tree expr, tsubst_flags_t complain) > it until instantiation. */ > if (TREE_CODE (expr) != DEFERRED_NOEXCEPT > && (!processing_template_decl > - || (flag_noexcept_type && !value_dependent_expression_p (expr)))) > + || !value_dependent_expression_p (expr))) > { > expr = perform_implicit_conversion_flags (boolean_type_node, expr, > complain, Let's go with this. And remove the comment. And the !processing_template_decl is also redundant, since that's checked at the top of value_dependent_expression_p. Jason
On Nov 29, 2018, Jason Merrill <jason@redhat.com> wrote: > Let's go with this. And remove the comment. > And the !processing_template_decl is also redundant, since that's > checked at the top of value_dependent_expression_p. I've tested this on i686- and x86_64-linux-gnu. Ok to install? [PR86397] resolve nondependent noexcept specs early in C++1[14] From: Alexandre Oliva <aoliva@redhat.com> build_noexcept_spec refrained from resolving nondependent noexcept expressions when they were not part of the function types (C++ 11 and 14). This caused problems during mangling: canonical_eh_spec, when called on the template function type, would find an unresolved but not explicitly deferred expression, and nothrow_spec_p would reject it. We could relax the mangling logic to skip canonical_eh_spec, but since -Wnoexcept-type warns when mangling function names that change as noexcept specs become part of types and of mangling in C++17, and the test at mangling time may give incorrect results if the spec is not resolved, we might as well keep things simple and resolve nondependent noexcept specs sooner rather than later. This is what this patch does. for gcc/cp/ChangeLog PR c++/86397 * except.c (build_noexcept_spec): Resolve nondependent expressions. for gcc/testsuite/ChangeLog PR c++/86397 * g++.dg/cpp0x/pr86397-1.C: New. * g++.dg/cpp0x/pr86397-2.C: New. --- gcc/cp/except.c | 5 +---- gcc/testsuite/g++.dg/cpp0x/pr86397-1.C | 4 ++++ gcc/testsuite/g++.dg/cpp0x/pr86397-2.C | 4 ++++ 3 files changed, 9 insertions(+), 4 deletions(-) create mode 100644 gcc/testsuite/g++.dg/cpp0x/pr86397-1.C create mode 100644 gcc/testsuite/g++.dg/cpp0x/pr86397-2.C diff --git a/gcc/cp/except.c b/gcc/cp/except.c index 3449b59b3cc0..a6951baa35c6 100644 --- a/gcc/cp/except.c +++ b/gcc/cp/except.c @@ -1189,11 +1189,8 @@ type_throw_all_p (const_tree type) tree build_noexcept_spec (tree expr, tsubst_flags_t complain) { - /* This isn't part of the signature, so don't bother trying to evaluate - it until instantiation. */ if (TREE_CODE (expr) != DEFERRED_NOEXCEPT - && (!processing_template_decl - || (flag_noexcept_type && !value_dependent_expression_p (expr)))) + && !value_dependent_expression_p (expr)) { expr = perform_implicit_conversion_flags (boolean_type_node, expr, complain, diff --git a/gcc/testsuite/g++.dg/cpp0x/pr86397-1.C b/gcc/testsuite/g++.dg/cpp0x/pr86397-1.C new file mode 100644 index 000000000000..4f9f5fa7e4c8 --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp0x/pr86397-1.C @@ -0,0 +1,4 @@ +// { dg-do compile { target c++11 } } +void e(); +template <bool> void f(int() noexcept(e)) {} +template void f<false>(int()); // { dg-error "does not match" "" { target c++17 } } diff --git a/gcc/testsuite/g++.dg/cpp0x/pr86397-2.C b/gcc/testsuite/g++.dg/cpp0x/pr86397-2.C new file mode 100644 index 000000000000..fb43499526e8 --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp0x/pr86397-2.C @@ -0,0 +1,4 @@ +// { dg-do compile { target c++11 } } +void e(); +template <bool> void f(int() noexcept(e)) {} +template void f<false>(int() noexcept);
OK, thanks. On Wed, Dec 5, 2018 at 1:32 AM Alexandre Oliva <aoliva@redhat.com> wrote: > > On Nov 29, 2018, Jason Merrill <jason@redhat.com> wrote: > > > Let's go with this. And remove the comment. > > > And the !processing_template_decl is also redundant, since that's > > checked at the top of value_dependent_expression_p. > > I've tested this on i686- and x86_64-linux-gnu. Ok to install? > > > [PR86397] resolve nondependent noexcept specs early in C++1[14] > > From: Alexandre Oliva <aoliva@redhat.com> > > build_noexcept_spec refrained from resolving nondependent noexcept > expressions when they were not part of the function types (C++ 11 and > 14). This caused problems during mangling: canonical_eh_spec, when > called on the template function type, would find an unresolved but not > explicitly deferred expression, and nothrow_spec_p would reject it. > > We could relax the mangling logic to skip canonical_eh_spec, but since > -Wnoexcept-type warns when mangling function names that change as > noexcept specs become part of types and of mangling in C++17, and the > test at mangling time may give incorrect results if the spec is not > resolved, we might as well keep things simple and resolve nondependent > noexcept specs sooner rather than later. This is what this patch does. > > > for gcc/cp/ChangeLog > > PR c++/86397 > * except.c (build_noexcept_spec): Resolve nondependent > expressions. > > for gcc/testsuite/ChangeLog > > PR c++/86397 > * g++.dg/cpp0x/pr86397-1.C: New. > * g++.dg/cpp0x/pr86397-2.C: New. > --- > gcc/cp/except.c | 5 +---- > gcc/testsuite/g++.dg/cpp0x/pr86397-1.C | 4 ++++ > gcc/testsuite/g++.dg/cpp0x/pr86397-2.C | 4 ++++ > 3 files changed, 9 insertions(+), 4 deletions(-) > create mode 100644 gcc/testsuite/g++.dg/cpp0x/pr86397-1.C > create mode 100644 gcc/testsuite/g++.dg/cpp0x/pr86397-2.C > > diff --git a/gcc/cp/except.c b/gcc/cp/except.c > index 3449b59b3cc0..a6951baa35c6 100644 > --- a/gcc/cp/except.c > +++ b/gcc/cp/except.c > @@ -1189,11 +1189,8 @@ type_throw_all_p (const_tree type) > tree > build_noexcept_spec (tree expr, tsubst_flags_t complain) > { > - /* This isn't part of the signature, so don't bother trying to evaluate > - it until instantiation. */ > if (TREE_CODE (expr) != DEFERRED_NOEXCEPT > - && (!processing_template_decl > - || (flag_noexcept_type && !value_dependent_expression_p (expr)))) > + && !value_dependent_expression_p (expr)) > { > expr = perform_implicit_conversion_flags (boolean_type_node, expr, > complain, > diff --git a/gcc/testsuite/g++.dg/cpp0x/pr86397-1.C b/gcc/testsuite/g++.dg/cpp0x/pr86397-1.C > new file mode 100644 > index 000000000000..4f9f5fa7e4c8 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/cpp0x/pr86397-1.C > @@ -0,0 +1,4 @@ > +// { dg-do compile { target c++11 } } > +void e(); > +template <bool> void f(int() noexcept(e)) {} > +template void f<false>(int()); // { dg-error "does not match" "" { target c++17 } } > diff --git a/gcc/testsuite/g++.dg/cpp0x/pr86397-2.C b/gcc/testsuite/g++.dg/cpp0x/pr86397-2.C > new file mode 100644 > index 000000000000..fb43499526e8 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/cpp0x/pr86397-2.C > @@ -0,0 +1,4 @@ > +// { dg-do compile { target c++11 } } > +void e(); > +template <bool> void f(int() noexcept(e)) {} > +template void f<false>(int() noexcept); > > > -- > Alexandre Oliva, freedom fighter https://FSFLA.org/blogs/lxo > Be the change, be Free! FSF Latin America board member > GNU Toolchain Engineer Free Software Evangelist > Hay que enGNUrecerse, pero sin perder la terGNUra jamás-GNUChe
diff --git a/gcc/cp/mangle.c b/gcc/cp/mangle.c index 64415894bc57..a499ef97b3c6 100644 --- a/gcc/cp/mangle.c +++ b/gcc/cp/mangle.c @@ -170,6 +170,17 @@ integer_type_codes[itk_none] = '\0', '\0', '\0', '\0', '\0', '\0', '\0', '\0' }; +/* Exception specifications may carry unresolved noexcept expressions + before template substitution, but nothrow_spec_p will fail an + assert check if it comes across such a spec while + !processing_template_decl. So, when we start processing the + signature of a template function, that may contain a such + unresolved expressions, we set this variable. Then, as we process + the exception spec, we set processing_template_decl. We don't want + to set processing_template_decl throughout, becuase this affects + other relevant tests, such as no_linkage_check. */ +static int processing_template_function_signature; + static int decl_is_template_id (const tree, tree* const); /* Functions for handling substitutions. */ @@ -418,7 +429,11 @@ canonicalize_for_substitution (tree node) || TREE_CODE (node) == METHOD_TYPE) { node = build_ref_qualified_type (node, type_memfn_rqual (orig)); + if (processing_template_function_signature) + processing_template_decl++; tree r = canonical_eh_spec (TYPE_RAISES_EXCEPTIONS (orig)); + if (processing_template_function_signature) + processing_template_decl--; if (flag_noexcept_type) node = build_exception_variant (node, r); else @@ -836,6 +851,7 @@ write_encoding (const tree decl) write_bare_function_type -- otherwise, it will get confused about which artificial parameters to skip. */ d = NULL_TREE; + ++processing_template_function_signature; } else { @@ -846,6 +862,9 @@ write_encoding (const tree decl) write_bare_function_type (fn_type, mangle_return_type_p (decl), d); + + if (tmpl) + --processing_template_function_signature; } } diff --git a/gcc/testsuite/g++.dg/cpp0x/pr86397-1.C b/gcc/testsuite/g++.dg/cpp0x/pr86397-1.C new file mode 100644 index 000000000000..4f9f5fa7e4c8 --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp0x/pr86397-1.C @@ -0,0 +1,4 @@ +// { dg-do compile { target c++11 } } +void e(); +template <bool> void f(int() noexcept(e)) {} +template void f<false>(int()); // { dg-error "does not match" "" { target c++17 } } diff --git a/gcc/testsuite/g++.dg/cpp0x/pr86397-2.C b/gcc/testsuite/g++.dg/cpp0x/pr86397-2.C new file mode 100644 index 000000000000..fb43499526e8 --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp0x/pr86397-2.C @@ -0,0 +1,4 @@ +// { dg-do compile { target c++11 } } +void e(); +template <bool> void f(int() noexcept(e)) {} +template void f<false>(int() noexcept);