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? Thanks, Simon
