Hello Jason. Revised patch as part of version 4 series
> On 18 Dec 2025, at 08:56, Jason Merrill <[email protected]> wrote: > > On 12/1/25 10:18 PM, Iain Sandoe wrote: >> From: Nina Ranns <[email protected]> >> > > Shorter could be > > bool had = hash_map_safe_put<hm_ggc> (decl_wrapper_fn, fndecl, wrapper); > gcc_checking_assert (!had); It turns out not to be shorter when the bool has to be guarded on CHECKING_P >> +static tree >> +get_orig_func_for_wrapper (tree wrapper) >> +{ >> + gcc_checking_assert (wrapper); >> + tree *result = hash_map_safe_get (decl_for_wrapper, wrapper); >> + return result ? *result : NULL_TREE; >> +} > > Comment needed. done >> + size_t l = strlen (n); > > IDENTIFIER_LENGTH? fixed >> + fnname = copy_node (DECL_NAME (fndecl)); > > Copying an identifier doesn't make sense to me? >> + tree wrapper_return_type = copy_node (TREE_TYPE (TREE_TYPE (fndecl))); > > Nor copying a type. > >> + tree wrapper_type = build_function_type (wrapper_return_type, >> wrapper_args); > > How (/why) is this different from TREE_TYPE (fndecl)? > >> + /* Contract violation wrapper function is always noexcept. Otherwise, >> + the wrapper function is noexcept if the checked function is noexcept. >> */ > > Always...otherwise? > > We still seem to be rebuilding TREE_TYPE (fndecl). All removed and simplified. >> + //DECL_DECLARED_INLINE_P (wrapdecl) = true; >> + DECL_DISREGARD_INLINE_LIMITS (wrapdecl) = true; > > Why do we want to require inlining? we don’t ... > >> + suppress_warning (wrapdecl); > Which warnings? Not clear, part of the original code - we can remove at some stage. >> + && !DECL_CONTRACT_WRAPPER(fndecl)) > > Space before ( fixed >> + && (TREE_CODE (CONTRACT_STATEMENT (attr)) == PRECONDITION_STMT))) > > This seems like it could be shorter to write, perhaps by adding a function > taking attr and returning cmk_*, so > > if (remap_kind != contract_attr_kind (attr)) we want to remove the layer that relates to the wrapping the specifiers in an attribute-like tree list, but we have now revised the comments and function names to reflect that they are now function contract specifiers, rather than attributes. >> + return false; >> + >> + > > Extra blank line fixed >> >> + /* FIXME: Maybe we should check if fndecl is still dependent? */ > > No need to check here I'd think, but we might check in > build_contract_wrapper_function that we aren't trying to build a wrapper for > a templated function. done > >> + gcc_checking_assert(!DECL_HAS_CONTRACTS_P (wrapdecl)); > > Space before ( fixed >> + /* We do not support contracts on virtual functions yet. Client side >> wrapping is >> + not supported for cxx2a contracts. */ > > No need to mention cxx2a contracts. fixed -===== end ===
