diff mbox series

[v2] c++, contracts: No implicit capture in lambda contract assertions [PR124648].

Message ID 20260408093243.49042-1-iain@sandoe.co.uk
State New
Headers
Series [v2] c++, contracts: No implicit capture in lambda contract assertions [PR124648]. |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gcc_build--master-aarch64 success Build passed

Commit Message

Iain Sandoe April 8, 2026, 9:32 a.m. UTC 
  Hi Marek,

>>+  if (current_class_ref && !CP_TYPE_CONST_P (TREE_TYPE (current_class_ref)))
>>+    current_class_ref = view_as_const (current_class_ref);

>This change makes current_class_ref_copy unused.  Looks like this function
>is missing the
>
> /* Revert (any) constification of the current class object.  */
> current_class_ref = current_class_ref_copy;
>
>but there's
>
> current_class_ref = saved_ccr;
>
>below so I guess current_class_ref_copy can be removed.

Yes, done.

> Can we move the !CP_TYPE_CONST_P check into view_as_const instead?

Yeah that's a much better place for it, done as attached,

Retested on x86_64-darwin and powerpc64le-linux,
OK for trunk, when?
thanks
Iain

--- 8< --- 

We were currently accepting invalid code by allowing contract assertions
to trigger implicit lambda captures contrary to:
[expr.prim.lambda.closure] / p10.

The solution here is to mark captures that occur within contract
assertion scopes and then clear that mark if we then see a normal
capture for the same entity - we must defer the error handling since
the following is valid:

  auto f5 = [=] {
    contract_assert (i > 0); // OK, i is referenced elsewhere.
    return i;
  };

	PR c++/124648

gcc/cp/ChangeLog:

	* cp-tree.h (DECL_CONTRACT_CAPTURE_P): New.
	* parser.cc (cp_parser_lambda_body): Scan the captures for
	ones were only added in contract assertion scopes.  Issue
	errors for those found.
	* semantics.cc (process_outer_var_ref): Mark implicit
	captures that occur in contract assertion scopes.  Clear
	the mark if the entity is subsequently captured 'normally'.

gcc/testsuite/ChangeLog:

	* g++.dg/contracts/cpp26/expr.prim.lambda.closure.p10.C: New test.

Signed-off-by: Iain Sandoe <iain@sandoe.co.uk>
---
 gcc/cp/cp-tree.h                              |  8 +++
 gcc/cp/parser.cc                              | 21 ++++++++
 gcc/cp/semantics.cc                           | 27 ++++++++--
 .../cpp26/expr.prim.lambda.closure.p10.C      | 53 +++++++++++++++++++
 4 files changed, 106 insertions(+), 3 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/contracts/cpp26/expr.prim.lambda.closure.p10.C

Comments

Iain Sandoe April 8, 2026, 9:37 a.m. UTC | #1 
Apologies all, I attached the wrong patch to this response

> On 8 Apr 2026, at 15:02, Iain Sandoe <iains.gcc@gmail.com> wrote:
> 
> Hi Marek,
> 
>>> +  if (current_class_ref && !CP_TYPE_CONST_P (TREE_TYPE (current_class_ref)))
>>> +    current_class_ref = view_as_const (current_class_ref);
> 
>> This change makes current_class_ref_copy unused.  Looks like this function
>> is missing the
>> 
>> /* Revert (any) constification of the current class object.  */
>> current_class_ref = current_class_ref_copy;
>> 
>> but there's
>> 
>> current_class_ref = saved_ccr;
>> 
>> below so I guess current_class_ref_copy can be removed.
> 
> Yes, done.
> 
>> Can we move the !CP_TYPE_CONST_P check into view_as_const instead?
> 
> Yeah that's a much better place for it, done as attached,
> 
> Retested on x86_64-darwin and powerpc64le-linux,
> OK for trunk, when?
> thanks
> Iain

here is the correct one.
Jason Merrill April 8, 2026, 2:22 p.m. UTC | #2 
On 4/8/26 5:37 AM, Iain Sandoe wrote:
> Apologies all, I attached the wrong patch to this response
> 
>> On 8 Apr 2026, at 15:02, Iain Sandoe <iains.gcc@gmail.com> wrote:
>>
>> Hi Marek,
>>
>>>> +  if (current_class_ref && !CP_TYPE_CONST_P (TREE_TYPE (current_class_ref)))
>>>> +    current_class_ref = view_as_const (current_class_ref);
>>
>>> This change makes current_class_ref_copy unused.  Looks like this function
>>> is missing the
>>>
>>> /* Revert (any) constification of the current class object.  */
>>> current_class_ref = current_class_ref_copy;
>>>
>>> but there's
>>>
>>> current_class_ref = saved_ccr;
>>>
>>> below so I guess current_class_ref_copy can be removed.
>>
>> Yes, done.
>>
>>> Can we move the !CP_TYPE_CONST_P check into view_as_const instead?
>>
>> Yeah that's a much better place for it, done as attached,
>>
>> Retested on x86_64-darwin and powerpc64le-linux,
>> OK for trunk, when?
>> thanks
>> Iain
> 
> here is the correct one.

>  view_as_const (tree decl)
>  {
> -  if (!contract_const_wrapper_p (decl))
> +  if (decl
> +      && !contract_const_wrapper_p (decl)
> +      && !CP_TYPE_CONST_P (TREE_TYPE (decl)))

Do we still need to check contract_const_wrapper_p?  It seems redundant.

Jason
Iain Sandoe April 8, 2026, 4:12 p.m. UTC | #3 
> On 8 Apr 2026, at 19:52, Jason Merrill <jason@redhat.com> wrote:
> 
> On 4/8/26 5:37 AM, Iain Sandoe wrote:
>> Apologies all, I attached the wrong patch to this response
>>> On 8 Apr 2026, at 15:02, Iain Sandoe <iains.gcc@gmail.com> wrote:
>>> 
>>> Hi Marek,
>>> 
>>>>> +  if (current_class_ref && !CP_TYPE_CONST_P (TREE_TYPE (current_class_ref)))
>>>>> +    current_class_ref = view_as_const (current_class_ref);
>>> 
>>>> This change makes current_class_ref_copy unused.  Looks like this function
>>>> is missing the
>>>> 
>>>> /* Revert (any) constification of the current class object.  */
>>>> current_class_ref = current_class_ref_copy;
>>>> 
>>>> but there's
>>>> 
>>>> current_class_ref = saved_ccr;
>>>> 
>>>> below so I guess current_class_ref_copy can be removed.
>>> 
>>> Yes, done.
>>> 
>>>> Can we move the !CP_TYPE_CONST_P check into view_as_const instead?
>>> 
>>> Yeah that's a much better place for it, done as attached,
>>> 
>>> Retested on x86_64-darwin and powerpc64le-linux,
>>> OK for trunk, when?
>>> thanks
>>> Iain
>> here is the correct one.
> 
>> view_as_const (tree decl)
>> {
>> -  if (!contract_const_wrapper_p (decl))
>> +  if (decl
>> +      && !contract_const_wrapper_p (decl)
>> +      && !CP_TYPE_CONST_P (TREE_TYPE (decl)))
> 
> Do we still need to check contract_const_wrapper_p?  It seems redundant.

I think we still need it 
  contract_const_wrapper_p checks to see if we already applied a const-ification wrapper.
  CP_TYPE_CONST_P checks if we need a const-ification wrapper.

I do not think that the first subsumes the second.

Iain



> Jason
>
Jason Merrill April 9, 2026, 7:10 a.m. UTC | #4 
On 4/8/26 12:12 PM, Iain Sandoe wrote:
> 
>> On 8 Apr 2026, at 19:52, Jason Merrill <jason@redhat.com> wrote:
>>
>> On 4/8/26 5:37 AM, Iain Sandoe wrote:
>>> Apologies all, I attached the wrong patch to this response
>>>> On 8 Apr 2026, at 15:02, Iain Sandoe <iains.gcc@gmail.com> wrote:
>>>>
>>>> Hi Marek,
>>>>
>>>>>> +  if (current_class_ref && !CP_TYPE_CONST_P (TREE_TYPE (current_class_ref)))
>>>>>> +    current_class_ref = view_as_const (current_class_ref);
>>>>
>>>>> This change makes current_class_ref_copy unused.  Looks like this function
>>>>> is missing the
>>>>>
>>>>> /* Revert (any) constification of the current class object.  */
>>>>> current_class_ref = current_class_ref_copy;
>>>>>
>>>>> but there's
>>>>>
>>>>> current_class_ref = saved_ccr;
>>>>>
>>>>> below so I guess current_class_ref_copy can be removed.
>>>>
>>>> Yes, done.
>>>>
>>>>> Can we move the !CP_TYPE_CONST_P check into view_as_const instead?
>>>>
>>>> Yeah that's a much better place for it, done as attached,
>>>>
>>>> Retested on x86_64-darwin and powerpc64le-linux,
>>>> OK for trunk, when?
>>>> thanks
>>>> Iain
>>> here is the correct one.
>>
>>> view_as_const (tree decl)
>>> {
>>> -  if (!contract_const_wrapper_p (decl))
>>> +  if (decl
>>> +      && !contract_const_wrapper_p (decl)
>>> +      && !CP_TYPE_CONST_P (TREE_TYPE (decl)))
>>
>> Do we still need to check contract_const_wrapper_p?  It seems redundant.
> 
> I think we still need it
>    contract_const_wrapper_p checks to see if we already applied a const-ification wrapper.
>    CP_TYPE_CONST_P checks if we need a const-ification wrapper.
> 
> I do not think that the first subsumes the second.

But won't CP_TYPE_CONST_P be true for all const wrappers?

Jason
diff mbox series

Patch

diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index c0822da8283..352988294c3 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -593,6 +593,7 @@  extern GTY(()) tree cp_global_trees[CPTI_MAX];
       DECL_DECLARED_CONSTINIT_P (in VAR_DECL)
       TYPE_DECL_FOR_LINKAGE_PURPOSES_P (in TYPE_DECL)
    8: DECL_DECLARED_CONSTEXPR_P (in VAR_DECL, FUNCTION_DECL)
+      DECL_CONTRACT_CAPTURE_P (in FIELD_DECL)
 
    Usage of language-independent fields in a language-dependent manner:
 
@@ -5367,6 +5368,13 @@  get_vec_init_expr (tree t)
 #define DECL_NORMAL_CAPTURE_P(NODE) \
   DECL_LANG_FLAG_7 (FIELD_DECL_CHECK (NODE))
 
+/* True when a field decl relates to a lambda capture that has currently been
+   made to satisfy a use within a contract check.  Reset to false when the
+   capture is required outside a contract check.  Used to diagnose cases where
+   a capture is only made within contract checks.  */
+#define DECL_CONTRACT_CAPTURE_P(NODE) \
+  DECL_LANG_FLAG_8 (FIELD_DECL_CHECK (NODE))
+
 /* Nonzero if TYPE is an anonymous union or struct type.  We have to use a
    flag for this because "A union for which objects or pointers are
    declared is not an anonymous union" [class.union].  */
diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc
index 518858fc776..8b26a1de72d 100644
--- a/gcc/cp/parser.cc
+++ b/gcc/cp/parser.cc
@@ -13745,6 +13745,27 @@  cp_parser_lambda_body (cp_parser* parser, tree lambda_expr)
        finish_function (which will try to emit the contracts).  */
     cp_parser_late_contracts (parser, fco);
 
+    /* Check that we have not caused captures that only relate to contracts.
+       [expr.prim.lambda.closure] / P10.  */
+    if (flag_contracts)
+      {
+	gcc_checking_assert (current_lambda_expr () == lambda_expr);
+	for (tree le = LAMBDA_EXPR_CAPTURE_LIST (lambda_expr); le;
+	     le = TREE_CHAIN (le))
+	  {
+	    tree cap_fld = TREE_PURPOSE (le);
+	    if (TREE_CODE (cap_fld) == FIELD_DECL
+		&& DECL_CONTRACT_CAPTURE_P (cap_fld))
+	      {
+		auto_diagnostic_group d;
+		tree expr = TREE_VALUE (le);
+		location_t loc = DECL_SOURCE_LOCATION (cap_fld);
+		error_at (loc, "%qE is not implicitly captured by a contract"
+			  " assertion", expr);
+		inform (DECL_SOURCE_LOCATION (expr), "%q#E declared here", expr);
+	      }
+	  }
+      }
     finish_lambda_function (body);
   }
 
diff --git a/gcc/cp/semantics.cc b/gcc/cp/semantics.cc
index abd1156a2dc..f836ab47ab5 100644
--- a/gcc/cp/semantics.cc
+++ b/gcc/cp/semantics.cc
@@ -4619,6 +4619,17 @@  process_outer_var_ref (tree decl, tsubst_flags_t complain, bool odr_use)
 
       if (d && d != decl && is_capture_proxy (d))
 	{
+	  if (flag_contracts && !processing_contract_condition)
+	    {
+	      /* We might have created a capture for a contract_assert ref. to
+		 some var, if that is now captured 'normally' then this is OK.
+		 Otherwise we leave the capture marked as incorrect.  */
+	      gcc_checking_assert (DECL_HAS_VALUE_EXPR_P (d));
+	      tree proxy = DECL_VALUE_EXPR (d);
+	      gcc_checking_assert (TREE_CODE (proxy) == COMPONENT_REF
+				&& TREE_CODE (TREE_OPERAND (proxy, 1)) == FIELD_DECL);
+              DECL_CONTRACT_CAPTURE_P (TREE_OPERAND (proxy, 1)) = false;
+	    }
 	  if (DECL_CONTEXT (d) == containing_function)
 	    /* We already have an inner proxy.  */
 	    return d;
@@ -4667,10 +4678,20 @@  process_outer_var_ref (tree decl, tsubst_flags_t complain, bool odr_use)
       return error_mark_node;
     }
   /* Do lambda capture when processing the id-expression, not when
-     odr-using a variable.  */
+     odr-using a variable, but uses in a contract must not cause a capture.  */
   if (!odr_use && context == containing_function)
-    decl = add_default_capture (lambda_stack,
-				/*id=*/DECL_NAME (decl), initializer);
+    {
+      decl = add_default_capture (lambda_stack,
+				  /*id=*/DECL_NAME (decl), initializer);
+      if (flag_contracts && processing_contract_condition)
+	{
+	  gcc_checking_assert (DECL_HAS_VALUE_EXPR_P (decl));
+	  tree proxy = DECL_VALUE_EXPR (decl);
+	  gcc_checking_assert (TREE_CODE (proxy) == COMPONENT_REF
+				&& TREE_CODE (TREE_OPERAND (proxy, 1)) == FIELD_DECL);
+          DECL_CONTRACT_CAPTURE_P (TREE_OPERAND (proxy, 1)) = true;
+        }
+    }
   /* Only an odr-use of an outer automatic variable causes an
      error, and a constant variable can decay to a prvalue
      constant without odr-use.  So don't complain yet.  */
diff --git a/gcc/testsuite/g++.dg/contracts/cpp26/expr.prim.lambda.closure.p10.C b/gcc/testsuite/g++.dg/contracts/cpp26/expr.prim.lambda.closure.p10.C
new file mode 100644
index 00000000000..5a165c48166
--- /dev/null
+++ b/gcc/testsuite/g++.dg/contracts/cpp26/expr.prim.lambda.closure.p10.C
@@ -0,0 +1,53 @@ 
+// N5008 :
+// [expr.prim.lambda.closure]/p10
+// If all potential references to a local entity implicitly captured by a
+// lambda-expression L occur within the function contract assertions of the
+// call operator or operator template of L or within assertion-statements
+// within the body of L, the program is ill-formed.
+// [Note 4: Adding a contract assertion to an existing C++ program cannot
+//  cause additional captures. — end note]
+// { dg-do compile { target c++26 } }
+// { dg-additional-options "-fcontracts -fsyntax-only" }
+
+auto gl0 = [] (int x) 
+  pre (x > 10) { return x; }; // OK
+
+static int i = 0;
+
+void
+foo ()
+{
+  auto f1 = [=]
+    pre (i > 0) {};  // OK, no local entities are captured.
+
+  int i = 1;
+
+  auto f2 = [=]
+    pre (i > 0) {};  // { dg-error {'i' is not implicitly captured by a contract assertion} }
+
+  auto f3 = [i]
+    pre (i > 0) {};  // OK, i is captured explicitly.
+
+  auto f4 = [=] {
+    contract_assert (i > 0); // { dg-error {'i' is not implicitly captured by a contract assertion} }
+  };
+
+  auto f5 = [=] {
+    contract_assert (i > 0); // OK, i is referenced elsewhere.
+    return i;
+  };
+
+  auto f6 = [=] pre (                // #1
+    []{
+      bool x = true;
+      return [=]{ return x; }();    // OK, #1 captures nothing.
+    }()) {};
+
+// TODO: lambda captures in function contract specifiers are not yet
+// fully functional.
+#if 0
+  bool y = true;
+  auto f7 = [=]
+    pre([=]{ return y; }()); // error: outer capture of y is invalid.
+#endif
+}