On 1/16/25 10:25 AM, Simon Martin wrote:
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?
Yes.
Jason
