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.