| Message ID | 20200511224318.994420-1-ppalka@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | c++: premature requires-expression folding [PR95020] | expand |
On 5/11/20 6:43 PM, Patrick Palka wrote: > In the testcase below we're prematurely folding away the > requires-expression to 'true' after substituting in the function's > template arguments, but before substituting in the lambda's deduced > template arguments. > > This happens because during the first tsubst_requires_expr, > processing_template_decl is 1 but 'args' is just {void} and therefore > non-dependent, so we end up folding away the requires-expression to > boolean_true_node before we could substitute in the lambda's template > arguments and determine that '*v' is ill-formed. > > This patch removes the uses_template_parms check when deciding in > tsubst_requires_expr whether to keep around a new requires-expression. > Regardless of whether the template arguments are dependent, there still > might be more template parameters to later substitute in -- as in the > testcase below -- and even if not, tsubst_expr doesn't perform full > semantic processing unless !processing_template_decl, so it seems we > should wait until then to fold away the requires-expression. > > Passes 'make check-c++', does this look OK to commit after a full > bootstrap/regtest? OK. > gcc/cp/ChangeLog: > > PR c++/95020 > * constraint.c (tsubst_requires_expr): Produce a new > requires-expression when processing_template_decl, even if > template arguments are not dependent. > > gcc/testsuite/ChangeLog: > > PR c++/95020 > * g++/cpp2a/concepts-lambda7.C: New test. > --- > gcc/cp/constraint.cc | 4 +--- > gcc/testsuite/g++.dg/cpp2a/concepts-lambda7.C | 14 ++++++++++++++ > 2 files changed, 15 insertions(+), 3 deletions(-) > create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-lambda7.C > > diff --git a/gcc/cp/constraint.cc b/gcc/cp/constraint.cc > index 4ad17f3b7d8..8ee347cae60 100644 > --- a/gcc/cp/constraint.cc > +++ b/gcc/cp/constraint.cc > @@ -2173,9 +2173,7 @@ tsubst_requires_expr (tree t, tree args, > if (reqs == error_mark_node) > return boolean_false_node; > > - /* In certain cases, produce a new requires-expression. > - Otherwise the value of the expression is true. */ > - if (processing_template_decl && uses_template_parms (args)) > + if (processing_template_decl) > return finish_requires_expr (cp_expr_location (t), parms, reqs); > > return boolean_true_node; > diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-lambda7.C b/gcc/testsuite/g++.dg/cpp2a/concepts-lambda7.C > new file mode 100644 > index 00000000000..50746b777a3 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/cpp2a/concepts-lambda7.C > @@ -0,0 +1,14 @@ > +// PR c++/95020 > +// { dg-do compile { target c++2a } } > + > +template<typename> > +void foo() { > + auto t = [](auto v) { > + static_assert(requires { *v; }); // { dg-error "static assertion failed" } > + }; > + t(0); > +} > + > +void bar() { > + foo<void>(); > +} >
On Wed, 13 May 2020, Jason Merrill wrote: > On 5/11/20 6:43 PM, Patrick Palka wrote: > > In the testcase below we're prematurely folding away the > > requires-expression to 'true' after substituting in the function's > > template arguments, but before substituting in the lambda's deduced > > template arguments. > > > > This happens because during the first tsubst_requires_expr, > > processing_template_decl is 1 but 'args' is just {void} and therefore > > non-dependent, so we end up folding away the requires-expression to > > boolean_true_node before we could substitute in the lambda's template > > arguments and determine that '*v' is ill-formed. > > > > This patch removes the uses_template_parms check when deciding in > > tsubst_requires_expr whether to keep around a new requires-expression. > > Regardless of whether the template arguments are dependent, there still > > might be more template parameters to later substitute in -- as in the > > testcase below -- and even if not, tsubst_expr doesn't perform full > > semantic processing unless !processing_template_decl, so it seems we > > should wait until then to fold away the requires-expression. > > > > Passes 'make check-c++', does this look OK to commit after a full > > bootstrap/regtest? > > OK. Would the same patch be OK to backport to the GCC 10 branch? > > > gcc/cp/ChangeLog: > > > > PR c++/95020 > > * constraint.c (tsubst_requires_expr): Produce a new > > requires-expression when processing_template_decl, even if > > template arguments are not dependent. > > > > gcc/testsuite/ChangeLog: > > > > PR c++/95020 > > * g++/cpp2a/concepts-lambda7.C: New test. > > --- > > gcc/cp/constraint.cc | 4 +--- > > gcc/testsuite/g++.dg/cpp2a/concepts-lambda7.C | 14 ++++++++++++++ > > 2 files changed, 15 insertions(+), 3 deletions(-) > > create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-lambda7.C > > > > diff --git a/gcc/cp/constraint.cc b/gcc/cp/constraint.cc > > index 4ad17f3b7d8..8ee347cae60 100644 > > --- a/gcc/cp/constraint.cc > > +++ b/gcc/cp/constraint.cc > > @@ -2173,9 +2173,7 @@ tsubst_requires_expr (tree t, tree args, > > if (reqs == error_mark_node) > > return boolean_false_node; > > - /* In certain cases, produce a new requires-expression. > > - Otherwise the value of the expression is true. */ > > - if (processing_template_decl && uses_template_parms (args)) > > + if (processing_template_decl) > > return finish_requires_expr (cp_expr_location (t), parms, reqs); > > return boolean_true_node; > > diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-lambda7.C > > b/gcc/testsuite/g++.dg/cpp2a/concepts-lambda7.C > > new file mode 100644 > > index 00000000000..50746b777a3 > > --- /dev/null > > +++ b/gcc/testsuite/g++.dg/cpp2a/concepts-lambda7.C > > @@ -0,0 +1,14 @@ > > +// PR c++/95020 > > +// { dg-do compile { target c++2a } } > > + > > +template<typename> > > +void foo() { > > + auto t = [](auto v) { > > + static_assert(requires { *v; }); // { dg-error "static assertion > > failed" } > > + }; > > + t(0); > > +} > > + > > +void bar() { > > + foo<void>(); > > +} > > > >
On 5/30/20 12:37 AM, Patrick Palka wrote: > On Wed, 13 May 2020, Jason Merrill wrote: > >> On 5/11/20 6:43 PM, Patrick Palka wrote: >>> In the testcase below we're prematurely folding away the >>> requires-expression to 'true' after substituting in the function's >>> template arguments, but before substituting in the lambda's deduced >>> template arguments. >>> >>> This happens because during the first tsubst_requires_expr, >>> processing_template_decl is 1 but 'args' is just {void} and therefore >>> non-dependent, so we end up folding away the requires-expression to >>> boolean_true_node before we could substitute in the lambda's template >>> arguments and determine that '*v' is ill-formed. >>> >>> This patch removes the uses_template_parms check when deciding in >>> tsubst_requires_expr whether to keep around a new requires-expression. >>> Regardless of whether the template arguments are dependent, there still >>> might be more template parameters to later substitute in -- as in the >>> testcase below -- and even if not, tsubst_expr doesn't perform full >>> semantic processing unless !processing_template_decl, so it seems we >>> should wait until then to fold away the requires-expression. >>> >>> Passes 'make check-c++', does this look OK to commit after a full >>> bootstrap/regtest? >> >> OK. > > Would the same patch be OK to backport to the GCC 10 branch? Yes. >> >>> gcc/cp/ChangeLog: >>> >>> PR c++/95020 >>> * constraint.c (tsubst_requires_expr): Produce a new >>> requires-expression when processing_template_decl, even if >>> template arguments are not dependent. >>> >>> gcc/testsuite/ChangeLog: >>> >>> PR c++/95020 >>> * g++/cpp2a/concepts-lambda7.C: New test. >>> --- >>> gcc/cp/constraint.cc | 4 +--- >>> gcc/testsuite/g++.dg/cpp2a/concepts-lambda7.C | 14 ++++++++++++++ >>> 2 files changed, 15 insertions(+), 3 deletions(-) >>> create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-lambda7.C >>> >>> diff --git a/gcc/cp/constraint.cc b/gcc/cp/constraint.cc >>> index 4ad17f3b7d8..8ee347cae60 100644 >>> --- a/gcc/cp/constraint.cc >>> +++ b/gcc/cp/constraint.cc >>> @@ -2173,9 +2173,7 @@ tsubst_requires_expr (tree t, tree args, >>> if (reqs == error_mark_node) >>> return boolean_false_node; >>> - /* In certain cases, produce a new requires-expression. >>> - Otherwise the value of the expression is true. */ >>> - if (processing_template_decl && uses_template_parms (args)) >>> + if (processing_template_decl) >>> return finish_requires_expr (cp_expr_location (t), parms, reqs); >>> return boolean_true_node; >>> diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-lambda7.C >>> b/gcc/testsuite/g++.dg/cpp2a/concepts-lambda7.C >>> new file mode 100644 >>> index 00000000000..50746b777a3 >>> --- /dev/null >>> +++ b/gcc/testsuite/g++.dg/cpp2a/concepts-lambda7.C >>> @@ -0,0 +1,14 @@ >>> +// PR c++/95020 >>> +// { dg-do compile { target c++2a } } >>> + >>> +template<typename> >>> +void foo() { >>> + auto t = [](auto v) { >>> + static_assert(requires { *v; }); // { dg-error "static assertion >>> failed" } >>> + }; >>> + t(0); >>> +} >>> + >>> +void bar() { >>> + foo<void>(); >>> +} >>> >> >> >
diff --git a/gcc/cp/constraint.cc b/gcc/cp/constraint.cc index 4ad17f3b7d8..8ee347cae60 100644 --- a/gcc/cp/constraint.cc +++ b/gcc/cp/constraint.cc @@ -2173,9 +2173,7 @@ tsubst_requires_expr (tree t, tree args, if (reqs == error_mark_node) return boolean_false_node; - /* In certain cases, produce a new requires-expression. - Otherwise the value of the expression is true. */ - if (processing_template_decl && uses_template_parms (args)) + if (processing_template_decl) return finish_requires_expr (cp_expr_location (t), parms, reqs); return boolean_true_node; diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-lambda7.C b/gcc/testsuite/g++.dg/cpp2a/concepts-lambda7.C new file mode 100644 index 00000000000..50746b777a3 --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp2a/concepts-lambda7.C @@ -0,0 +1,14 @@ +// PR c++/95020 +// { dg-do compile { target c++2a } } + +template<typename> +void foo() { + auto t = [](auto v) { + static_assert(requires { *v; }); // { dg-error "static assertion failed" } + }; + t(0); +} + +void bar() { + foo<void>(); +}
In the testcase below we're prematurely folding away the requires-expression to 'true' after substituting in the function's template arguments, but before substituting in the lambda's deduced template arguments. This happens because during the first tsubst_requires_expr, processing_template_decl is 1 but 'args' is just {void} and therefore non-dependent, so we end up folding away the requires-expression to boolean_true_node before we could substitute in the lambda's template arguments and determine that '*v' is ill-formed. This patch removes the uses_template_parms check when deciding in tsubst_requires_expr whether to keep around a new requires-expression. Regardless of whether the template arguments are dependent, there still might be more template parameters to later substitute in -- as in the testcase below -- and even if not, tsubst_expr doesn't perform full semantic processing unless !processing_template_decl, so it seems we should wait until then to fold away the requires-expression. Passes 'make check-c++', does this look OK to commit after a full bootstrap/regtest? gcc/cp/ChangeLog: PR c++/95020 * constraint.c (tsubst_requires_expr): Produce a new requires-expression when processing_template_decl, even if template arguments are not dependent. gcc/testsuite/ChangeLog: PR c++/95020 * g++/cpp2a/concepts-lambda7.C: New test. --- gcc/cp/constraint.cc | 4 +--- gcc/testsuite/g++.dg/cpp2a/concepts-lambda7.C | 14 ++++++++++++++ 2 files changed, 15 insertions(+), 3 deletions(-) create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-lambda7.C