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.
> (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.
> + /* 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.
@@ -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?
Jason
