Hi Jason, I am posting the version 4 patch as part of the complete series - which will cross- reference this mail.
> On 20 Jan 2026, at 03:15, Jason Merrill <[email protected]> wrote: > > On 1/19/26 10:57 PM, Iain Sandoe wrote: >>> This must be fixed before merge. >> Done, also added logic for the Wc++26-compat and fixed up with >> warn_keyword_macro. >> Perhaps (at some point) we should add a ‘__contract_assert’ that is >> generally available? >> I would see no reason for contracts not to work back to at least C++17 (we >> might >> want to consider if there are any implications to postconditions in the >> absence of >> guaranteed copy elision if we wanted to go earlier). > > Makes sense. Added __contract_assert >>>> +/* Returns an invented parameter declaration of the form 'TYPE ID' for the >>>> + purpose of parsing the postcondition. >>>> + >>>> + We use a PARM_DECL instead of a VAR_DECL so that tsubst forces a lookup >>>> + in local specializations when we instantiate these things later. */ It turns out that this comment is misleading, the use of a PARM_DECL instead of a VAR_DECL causes various constraints to be lifted in the check_outer_var and other code - (but in the process causes us to accept invalid, at least in the case you mentioned below). This turned out to be a bit of a rabbit-hole, I changed the PARM_DECL to a VAR_DECL and then worked through the fallout that caused : see notes later. >>> ??? tsubst_expr does retrieve_local_specialization for VAR_DECL, too. I admit that this has always been one of the more puzzling bits of the old code. >>> The difference is that if that returns null, (and it’s a PARM_DECL) >>> we then build a new PARM_DECL, but only in unevaluated context, which a >>> contract specifier shouldn't be... >>> except that you also add processing_postcondition, which seems like a big >>> kludge. And seems like it shouldn't be necessary since you >>> register_local_specialization in rebuild_postconditions. Indeed, it is not needed (removed). >>> If we need to register_local_specialization elsewhere (say, >>> tsubst_contract), let's do that and lose processing_postcondition. We already do register the local specialisation in tsubst_contract, but we are not calling that here (only a subset of the functionality) >> Umm .. inherited code ... > This comment is inherited, but processing_postcondition is not. as noted above, removed. >>>> + /* Always update the context of the result variable so that it can >>>> + be remapped by remap_contracts. */ >>>> + DECL_CONTEXT (oldvar) = fndecl; >>> >>> When would it not have the right context? >> Much of the time it will be empty, since we do not have a function decl >> when we are parsing the contract specifiers. We set the context here in >> ‘maybe_rebuild_postconditions` which is called after deferred parsing >> or when we come to use the contracts (for free functions). It is also set >> in tsubst for instantiations. >> I added: >> gcc_checking_assert (!DECL_CONTEXT (oldvar) >> || DECL_CONTEXT (oldvar) == fndecl); > > But by the time we get to rebuild_postconditions in grokfndecl we've already > set DECL_CONTEXT. Deferred parsing is even later. And tsubst_function_decl > sets DECL_CONTEXT immediately after tsubsting the parms. This is another reason that it’s confusing making this a PARM_DECL. We are actually dealing with the postcondition placeholder for the return value, which is independent of the function parms. We could create another function to update the context specifically, but it seems reasonable to do it here. Remainder of comments on this issue follow below. >>>> + DECL_CHAIN (no_excp_) = BIND_EXPR_VARS (cc_bind); >>>> + BIND_EXPR_VARS (cc_bind) = no_excp_; >>> >>> I don't see why you need two variables: >>> >>> >>> ...and then just set cond to check_failed. >> TODO: on hold until the library ABI is finalised. > > Dropping no_excp_ seems like a simple non-ABI change, but I guess it can wait. I did this now anyway - and removed the support for some of the GNU extensions, since they are not going to be present in GCC-16. >>>> + else if (fn && processing_contract_condition && DECL_DESTRUCTOR_P (fn)) >>>> + error ("invalid use of %<this%> after it is valid"); >>>> else if (fn) >>>> error ("invalid use of %<this%> in non-member function"); >>>> else >>>> @@ -4674,6 +4688,9 @@ process_outer_var_ref (tree decl, tsubst_flags_t >>>> complain, bool odr_use) >>>> } >>>> return error_mark_node; >>>> } >>>> + else if (processing_contract_condition && (TREE_CODE (decl) == >>>> PARM_DECL)) >>>> + /* Use of a parameter in a contract condition is fine. */ >>>> + return decl; >>> >>> Only if it's a parameter of the current function. >>> Why does outer_var_p return true in this case? >> At this point the PARM does not have DECL_CONTEXT since we don’t yet have >> the function decl. >> If we have a PARM_DECL and outer_var_p is true (and we are processing a >> contract condition) >> it’s not clear how the parm could belong to anything else, I guess I’m >> missing something? > void f (int i) { > struct A { > void g() pre(i); > }; > } > > I guess you want to add a check that DECL_CONTEXT is null. > > ...though maybe outer_var_p should return false for a PARM_DECL with null > DECL_CONTEXT. It seems that this needs more consideration - I think that some of the issues are to do with using the substitution machinery in an unusual way, and it could be better to avoid that. The patch to allow use of a VAR_DECL instead of relying on some of the special treatment of the PARM_DECL turned out to be as much of a series of work-arounds as the original - so I shelved that. >> +/* If we have a successful constant evaluation, now check whether there is >> + a failed or non-constant contract that would invalidate this. */ >> + >> +static bool >> +check_for_failed_contracts (constexpr_global_ctx *global_ctx) >> +{ >> + if (!flag_contracts || !global_ctx->contract_statement) >> + return false; >> + >> + /* [basic.contract.eval]/7.3 */ >> + location_t loc = EXPR_LOCATION (global_ctx->contract_statement); >> + if (global_ctx->contract_condition_non_const) >> + { >> + error_at (loc, "contract condition is not constant"); >> + return true; >> + } > > From your [basic.contract.eval] citation, this also counts as a contract > violation, so it also should be an error only for terminating semantic. Oops.. I misread the interaction between the sections, fixed. >> + /* [intro.compliance.general]/2.3.4. */ >> + warning_at (loc, 0, "contract predicate is false in constant expression"); > > Instead of separate error/warning calls you could call emit_diagnostic with > diagnostics::kind::error/warning. yes, nicer, Done. > >> @@ -3656,6 +3666,10 @@ finish_this_expr (void) >> } >> else if (fn && DECL_STATIC_FUNCTION_P (fn)) >> error ("%<this%> is unavailable for static member functions"); >> + else if (fn && processing_contract_condition && DECL_CONSTRUCTOR_P (fn)) >> + error ("invalid use of %<this%> in a constructor %<pre%> condition"); >> + else if (fn && processing_contract_condition && DECL_DESTRUCTOR_P (fn)) >> + error ("invalid use of %<this%> a destructor %<post%> condition"); > > Missing “in” thanks, fixed. ===== end set 1 ==== ==== second mail ==== Second email retractions included above ==== third mail ==== >> @@ -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. fixed. >> @@ -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. fixed > >> @@ -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. fixed ====== END =======
