On 1/19/26 10:57 PM, Iain Sandoe wrote:
Hi Jason,

Thanks for the review.

The attached patch will be part of the V3 series (which needs work to
patch 8 to complete).

The current series (including all review changes, completed to date) is
here:
  
https://forge.sourceware.org/iains/gcc-TEST/commits/branch/C%2B%2B26-contracts-base

The individual commits for fixes to this specific review are here:

https://forge.sourceware.org/iains/gcc-TEST/commits/branch/C%2B%2B26-contracts-base

(the branches are also on GH if that is easier to access)
https://github.com/iains/gcc-git/tree/C%2B%2B26-contracts-base
https://github.com/iains/gcc-git/tree/contracts_v2_patch_3_review

I believe that I’ve addressed the merge blockers here and also many of the 
other points
including the typographical and comment ones.

This work has been rebased onto r16-6881-gddda478641ffcc, which includes the
reflection commits and the annual changes to (c).
how does it look now?

Ah, I didn't address this in my previous reply.

With the v3 fixes I'm satisfied that we can go ahead and merge patch 1-7 if a release manager agrees, and fix the remaining issues incrementally. I still need to review the updated patch 8.

A few more updates in addition to those in my last reply:

@@ -2089,6 +2096,8 @@ struct GTY(()) saved_scope {
   /* Nonzero if we are parsing the substatement of expansion-statement.  */
   BOOL_BITFIELD expansion_stmt : 1;
+ BOOL_BITFIELD x_comparing_contracts : 1;
+
   int unevaluated_operand;
   int inhibit_evaluation_warnings;
   int noexcept_operand;
@@ -2161,12 +2170,23 @@ extern GTY(()) struct saved_scope *scope_chain;
 #define processing_contract_condition \
   (scope_chain->bindings->kind == sk_contract)
+#define processing_postcondition scope_chain->x_processing_postcondition
+
+/* Nonzero if we are matching contracts of two functions.  Depending on
+   whether a decl has been genericized or not, PARM_DECL may be adjusted
+   to be an invisible reference.  */
+#define comparing_contracts scope_chain->x_comparing_contracts

It doesn't make sense for this sort of flag to be part of scope_chain, it shouldn't be set across template instantiation; see comparing_specializations which is just a file-scope variable.

@@ -2123,6 +2124,32 @@ cp_genericize_r (tree *stmt_p, int *walk_subtrees, void 
*data)
       wtd->bind_expr_stack.pop ();
       break;
+ case ASSERTION_STMT:
+    case PRECONDITION_STMT:
+    case POSTCONDITION_STMT:
+      {
+       tree check = build_contract_check (stmt);
+       if (check)
+         /* We need to genericize the contract independently of everything
+          else we genericized until now.  When recursively genericizing, we
+          keep a track of all the statements that have been seen.  Building a
+          contract check will create new statements, which may reuse an
+          already freed statement.  If such an already freed statement has
+          been cached in p_set, we will fail to correctly genericize newly
+          created contract tree.  */

Isn't this avoided by all the copying done elsewhere?

Also I have trouble imagining how a new contract check would involve already freed code.

+         cp_genericize_tree (&check, wtd->handle_invisiref_parm_p);
+       else
+         /* If we didn't build a check, replace it with void_node so we don't
+         leak contracts into GENERIC.  */
+         check = void_node;
+       *stmt_p = check;
+       *walk_subtrees = 0;
+       /* Return early and do not add the contract statement into the
+          cache.  */
Likewise, I'm not sure why this is needed.

@@ -14118,6 +14163,18 @@ cp_parser_statement (cp_parser* parser, tree 
in_statement_expr,
   /* Peek at the next token.  */
   token = cp_lexer_peek_token (parser->lexer);
+ /* FIXME: is this relevant any more? */
+  /* If we have contracts, check that they're valid in this context.  */
+  if (std_attrs != error_mark_node)
+    {
+      if (tree pre = lookup_attribute ("pre", std_attrs))
+       error_at (EXPR_LOCATION (TREE_VALUE (pre)),
+                 "preconditions cannot be statements");
+      else if (tree post = lookup_attribute ("post", std_attrs))
+       error_at (EXPR_LOCATION (TREE_VALUE (post)),
+                 "postconditions cannot be statements");
+    }
+

One more obsolete bit.

@@ -26045,6 +26054,13 @@ cp_parser_init_declarator (cp_parser* parser,
     {
       /* If the init-declarator isn't initialized and isn't followed by a
         `,' or `;', it's not a valid init-declarator.  */
+      tree contract_attr_name = NULL_TREE;
+      if (token->type == CPP_NAME)
+       {
+         contract_attr_name = token->u.value;
+         contract_attr_name = canonicalize_attr_name (contract_attr_name);
+       }
+
       if (token->type != CPP_COMMA
          && token->type != CPP_SEMICOLON)
        {

And this.

Reply via email to