On 16 Jan 2025, at 16:05, Jason Merrill wrote: > On 1/16/25 7:19 AM, Simon Martin wrote: >> Hi Jakub, Jason, >> >> On 15 Jan 2025, at 22:55, Jakub Jelinek wrote: >> >>> On Wed, Jan 15, 2025 at 04:48:59PM -0500, Jason Merrill wrote: >>>>> --- a/gcc/cp/decl.cc >>>>> +++ b/gcc/cp/decl.cc >>>>> @@ -11686,6 +11686,7 @@ fold_sizeof_expr (tree t) >>>>> false, false); >>>>> if (r == error_mark_node) >>>>> r = size_one_node; >>>>> + r = cp_fold_convert (TREE_TYPE (t), r); >>>> >>>> Instead of adding this conversion in all cases, let's change >>>> size_one_node >>>> to >>>> >>>> build_int_cst (size_type_node, 1) >>> That would need to be r = build_int_cst (TREE_TYPE (t), 1); >> Jason is right: my patch does not need to do TREE_TYPE (t), and can >> simply use size_type_node - oversight on my part. >> >>> I guess, while that is maybe fine, I don't see how it could avoid >>> the cp_fold_convert call, because size_one_node can be returned >>> also from e.g. c-family c_sizeof_or_alignof_type or its typeck.cc >>> callers, >>> or it can be size_int (something) etc. >> I took another look at the code paths in fold_sizeof_expr and I >> believe >> that we’re “good” in all of them except if we hit >> typeck.cc:2077 >> (I have not been able so far to craft a test that does), because the >> code either builds a node with size_type_node as type, or goes >> through >> c_sizeof_or_alignof_type that fold_convert’s everything (except >> error_mark_node) to size_type_node (in c-common.cc:4028). >> >> I can run a regression test round with a patch that uses >> “build_int_cst (size_type_node, 1)” in decl.cc:11930 and >> typeck.cc:2077, which should cover all the *current* cases for >> fold_sizeof_expr. However, the initial patch has the advantage that: >> - It’s consistent with what c_sizeof_or_alignof_type does >> - It will cover the (unlikely?) possibility that some new code >> path is >> added some day that that does not use the right type >> - It adds no cost to nominal cases, since cp_fold_convert does >> nothing >> if we already have the right type >> >> Jason, would you still like me to test and submit a patch that uses >> build_int_cst in the two places identified above instead of doing >> cp_fold_convert (size_type_node, r) in decl.cc:11930? > > No need; your original patch is OK, thanks. Thanks Jason, I will merge it momentarily. Since it’s a regression from GCC 12, is it OK for branches as well?
Simon
