Hi Jason, The revised patch is part of the version 4 series, cross-referencing this.
> On 23 Jan 2026, at 14:06, Jason Merrill <[email protected]> wrote: > > On 1/22/26 11:22 PM, Nina Dinka Ranns wrote: >> Hello, >> Most of the comments were addressed as suggested. Please see below for the >> comments that may still need your attention. >> Best, >> Nina >> On Sun, 18 Jan 2026 at 10:01, Jason Merrill <[email protected] >> <mailto:[email protected]>> wrote: >> On 12/1/25 10:18 PM, Iain Sandoe wrote: >> > From: Nina Ranns <[email protected] >> <mailto:[email protected]>> >> > >> > Changes since v1 >> > - fixed a merge error in the removal of C++2a code. >> > - rebased onto r16-5785-g3b30d09ac7bbf8 (includes change to >> default to >> > C++20). >> > >> > --- 8< --- >> > >> > (build_call_a_1): Split out the functionality needed >> > for thunk-like calls. >> You can't add a _1 function to cp-tree.h, it needs a real name. >> But it seems like we should be able to avoid splitting build_call_a, >> see >> comments on build_thunk_like_call. >> This is the one issue in this patch that needs to be resolved before >> merge. >> Done. This will require an adjustment in patch 9 to account for new argument >> to `build_thunk_like_call` > > Hmm, do we actually need the new argument? Isn't the caller always > current_function_decl? > > Also the function comment looks incomplete: > >> +/* Build and return a thunk like call to FUNC from CALLER using the supplied >> + arguments. The call is like a thunk call in the fact that we do not >> + want to create additional copies of the arguments >> + */ > > Various other function comments have formatting issues as well. amended for missing spaces and content. >> > (build_over_call): Ensure that we pass the original >> > -void >> > -maybe_update_postconditions (tree fndecl) >> > +/* Map from FUNCTION_DECL to a FUNCTION_DECL for either the >> PRE_FN or POST_FN. >> > + These are used to parse contract conditions and are called >> inside the body >> > + of the guarded function. */ >> > +static GTY(()) hash_map<tree, tree> *decl_pre_fn; >> > +static GTY(()) hash_map<tree, tree> *decl_post_fn; >> So many hash_maps! >> We discussed adding pre and post fn to the contract_decl_map, but that >> would mean we would store >> a pre and post fn information for every function with contracts. Having it >> in separate maps means we >> only have a pre entry for a function with preconditions and a post entry for >> a function with post conditions. > > Fair enough. > > But it looks even though you removed the uses of orig_from_outlined, the > variable is still there. This change was reverted - it causes fails. As noted we have tried to keep the individual memory footprints reasonably small, it might be possible to improve and we can tackle that in slower time. >> > + /* The contract check functions are never a cdtor */ >> > + DECL_CXX_DESTRUCTOR_P (fn) = DECL_CXX_CONSTRUCTOR_P (fn) = 0; >> > + >> > + DECL_NAME (fn) = contracts_fixup_name (DECL_NAME(fndecl), >> > + pre, >> > + DECL_CXX_CONSTRUCTOR_P >> (fndecl) >> > + || DECL_CXX_DESTRUCTOR_P >> (fndecl)); >> > + >> > + DECL_INITIAL (fn) = NULL_TREE; >> > + CONTRACT_HELPER (fn) = pre ? ldf_contract_pre : ldf_contract_post; >> > + >> > + DECL_VIRTUAL_P (fn) = false; >> > + >> > + /* These functions should be internal. */ >> ...but for callee checks for a vague linkage fndecl they should be in a >> comdat group with it. >> Reinstating the old code that did this. It will require a change in the >> caller side wrappers >> (patch9 ) to make sure the wrapper is added to a new comdat group with its >> respective pre and post function > > Sounds good. > >> > + >> > + return NULL_TREE; >> > +} >> > + >> > +/* Build and add a call to the post-condition checking function, >> when that >> > + is in use. */ >> > + >> > +static void >> > +add_post_condition_fn_call (tree fndecl) >> > +{ >> > + gcc_checking_assert (DECL_POST_FN (fndecl) >> > + && DECL_POST_FN (fndecl) != error_mark_node); >> > + >> > + releasing_vec args = build_arg_list (fndecl); >> > + if (get_postcondition_result_parameter (fndecl)) >> > + vec_safe_push (args, DECL_RESULT (fndecl)); >> This still won't preserve modifications of scalar return values, right? >> It looks like there are no tests of const_cast modifications of >> constified things. >> Some tests added. I don't fully understand why we think this will not >> preserve modifications of >> scalar return values. I could not trigger a problem where a modification of >> a scalar return value was not preserved. Can you elaborate, please ? > > But the new test doesn't select outlining; if I add > -fcontract-checks-outlined it fails because the above passes the scalar > return value by value to the .post function and there's no way for the > modifications to make it back to the caller. > > Please add an additional copy of the test for -outlined. done, we have a solution for the issue - but not ready in time for this pass - and we do not want to delay the commit. >> @@ -11534,7 +11534,9 @@ build_cxx_call (tree fn, int nargs, tree *argarray, >> SET_EXPR_LOCATION (fn, loc); >> fndecl = get_callee_fndecl (fn); >> - if (!orig_fndecl) >> + if (!fndecl && orig_fndecl) >> + fndecl = orig_fndecl; >> + else if (!orig_fndecl) >> orig_fndecl = fndecl; > > orig_fndecl is supposed to be only for diagnostics, it looks wrong to use it > to set fndecl. Is this actually needed? the call.cc changes are no longer present. > > Jason >
