[PATCH] Fix PR c++/70121 (premature folding of const var that was implicitly captured)

Patrick Palka patrick@parcs.ath.cx
Thu Mar 10 22:58:00 GMT 2016


Within a lambda we should implicitly capture an outer const variable
only if it's odr-used in the body of the lambda.  But we are currently
making the decision of whether to capture such a variable, or else to
fold it to a constant, too early -- before we can know whether it's
being odr-used or not.  So we currently always fold a const variable to
a constant if possible instead of otherwise capturing it, but of course
doing this is wrong if e.g. the address of this variable is taken inside
the lambda's body.

This patch reverses the behavior of process_outer_var_ref, so that we
always implicitly capture a const variable if it's capturable, instead
of always trying to first fold it to a constant.  This behavior however
is wrong too, and introduces a different but perhaps less important
regression: if we implicitly capture by value a const object that is not
actually odr-used within the body of the lambda, we may introduce a
redundant call to its copy/move constructor, see pr70121-2.C.

Ideally we should be capturing a variable only if it's not odr-used
within the entire body of the lambda, but doing that would be a
significantly less trivial patch I think.  So I wonder if this patch is
a good tradeoff for GCC 6.  But I mostly wonder if my analysis and
proposed ideal solution is correct :)  Either way I'm planning to
bootstrap + regtest this patch and also test it against Boost.

gcc/cp/ChangeLog:

	PR c++/70121
	* semantics.c (process_outer_var_ref): Don't fold DECL to a
	constant if it's otherwise going to get implicitly captured.

gcc/testsuite/ChangeLog:

	PR c++/70121
	* g++.dg/cpp0x/lambda/pr70121.C: New test.
	* g++.dg/cpp0x/lambda/pr70121-2.C: New test.
---
 gcc/cp/semantics.c                            |  6 +++++
 gcc/testsuite/g++.dg/cpp0x/lambda/pr70121-2.C | 22 ++++++++++++++++
 gcc/testsuite/g++.dg/cpp0x/lambda/pr70121.C   | 37 +++++++++++++++++++++++++++
 3 files changed, 65 insertions(+)
 create mode 100644 gcc/testsuite/g++.dg/cpp0x/lambda/pr70121-2.C
 create mode 100644 gcc/testsuite/g++.dg/cpp0x/lambda/pr70121.C

diff --git a/gcc/cp/semantics.c b/gcc/cp/semantics.c
index ea72e0e..a9dbf16 100644
--- a/gcc/cp/semantics.c
+++ b/gcc/cp/semantics.c
@@ -3332,6 +3332,12 @@ process_outer_var_ref (tree decl, tsubst_flags_t complain)
 	   the closure type when instantiating the lambda context.  That is
 	   probably also the way to handle lambdas within pack expansions.  */
 	return decl;
+      /* Don't try to fold DECL to a constant if we are otherwise going to
+	 implicitly capture it.  FIXME we should avoid folding DECL to a
+	 constant only if it's odr-used within the lambda body, but we can't
+	 determine that at this point.  */
+      else if (lambda_expr && context == containing_function)
+	;
       else if (decl_constant_var_p (decl))
 	{
 	  tree t = maybe_constant_value (convert_from_reference (decl));
diff --git a/gcc/testsuite/g++.dg/cpp0x/lambda/pr70121-2.C b/gcc/testsuite/g++.dg/cpp0x/lambda/pr70121-2.C
new file mode 100644
index 0000000..4676068
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/lambda/pr70121-2.C
@@ -0,0 +1,22 @@
+// PR c++/70121
+// { dg-do compile { target c++11 } }
+
+struct X
+{
+  int a;
+
+  constexpr X () : a (28) { };
+  X (const X&) = delete;
+  X (const X&&) = delete;
+};
+
+void
+baz (void)
+{
+  constexpr X x;
+  // These are non-odr usages of x.a so they should each be folded to a
+  // constant expression without having to capture and copy x.
+  auto ff1 = [=] { return x.a; }; // { dg-bogus "deleted" "" { xfail *-*-* } }
+  const auto &ff2 = [=] { return x.a; }; // { dg-bogus "deleted" "" { xfail *-*-* } }
+  const auto &&ff3 = [=] { return x.a; }; // { dg-bogus "deleted" "" { xfail *-*-* } }
+}
diff --git a/gcc/testsuite/g++.dg/cpp0x/lambda/pr70121.C b/gcc/testsuite/g++.dg/cpp0x/lambda/pr70121.C
new file mode 100644
index 0000000..db8a4ca
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/lambda/pr70121.C
@@ -0,0 +1,37 @@
+// PR c++/70121
+// { dg-do compile { target c++11 } }
+
+static void
+foo (void)
+{
+  const int val = 28;
+  auto ff_v = [=]() -> const int& { return val; }; // { dg-bogus "temporary" }
+  auto ff_r = [&]() -> const int& { return val; }; // { dg-bogus "temporary" }
+
+  if (&ff_r () != &val)
+    __builtin_abort ();
+
+  if (ff_v () + ff_r () != 56)
+    __builtin_abort ();
+}
+
+static void
+bar (void)
+{
+  const int val = 28;
+  static const int *p;
+  auto ff_v = [=] { p = &val; }; // { dg-bogus "lvalue required" }
+  auto ff_r = [&] { p = &val; }; // { dg-bogus "lvalue required" }
+
+  ff_v ();
+  ff_r ();
+  if (p != &val)
+    __builtin_abort ();
+}
+
+int
+main ()
+{
+  foo ();
+  bar ();
+}
-- 
2.8.0.rc1.12.gfce6d53



More information about the Gcc-patches mailing list