On Tue, May 8, 2018 at 4:04 PM, Jakub Jelinek <ja...@redhat.com> wrote:
> On Tue, May 08, 2018 at 01:03:00PM -0400, Jason Merrill wrote:
>> On Sun, May 6, 2018 at 1:56 PM, Jakub Jelinek <ja...@redhat.com> wrote:
>> > --- gcc/c-family/c-common.c.jj 2018-03-27 21:58:55.598502113 +0200
>> > +++ gcc/c-family/c-common.c 2018-05-05 10:55:47.951600802 +0200
>> > @@ -6171,7 +6171,7 @@ c_common_to_target_charset (HOST_WIDE_IN
>> > traditional rendering of offsetof as a macro. Return the folded
>> > result. */
>> >
>> > tree
>> > -fold_offsetof_1 (tree expr, enum tree_code ctx)
>> > +fold_offsetof_1 (tree expr, bool nonptr, enum tree_code ctx)
>>
>> The comment needs to document the NONPTR parameter.
>
> Ok.
>
>> > @@ -6287,7 +6291,7 @@ fold_offsetof_1 (tree expr, enum tree_co
>> > tree
>> > fold_offsetof (tree expr)
>> > {
>> > - return convert (size_type_node, fold_offsetof_1 (expr));
>> > + return convert (size_type_node, fold_offsetof_1 (expr, true));
>> > }
>>
>> Since all the uses of fold_offset_1 involve converting to a particular
>> type, I wonder about wrapping it so that the argument for nonptr is
>> determined from that type.
>
> So like this?
>
> 2018-05-08 Jakub Jelinek <ja...@redhat.com>
>
> PR c++/85662
> * c-common.h (fold_offsetof_1): Add TYPE argument.
> * c-common.c (fold_offsetof_1): Add TYPE argument, if it is not a
> pointer type, convert the pointer constant to TYPE and use size_binop
> with PLUS_EXPR instead of fold_build_pointer_plus. Adjust recursive
> calls.
> (fold_offsetof): Pass size_type_node as TYPE to fold_offsetof_1.
>
> * c-fold.c (c_fully_fold_internal): Pass TREE_TYPE (expr) as TYPE
> to fold_offsetof_1.
> * c-typeck.c (build_unary_op): Pass argtype as TYPE to
> fold_offsetof_1.
>
> * cp-gimplify.c (cp_fold): Pass TREE_TYPE (x) as TYPE to
> fold_offsetof_1.
>
> * g++.dg/ext/offsetof2.C: New test.
>
> --- gcc/c-family/c-common.h.jj 2018-05-06 23:12:49.185619717 +0200
> +++ gcc/c-family/c-common.h 2018-05-08 21:47:40.976737821 +0200
> @@ -1033,7 +1033,7 @@ extern bool c_dump_tree (void *, tree);
>
> extern void verify_sequence_points (tree);
>
> -extern tree fold_offsetof_1 (tree, tree_code ctx = ERROR_MARK);
> +extern tree fold_offsetof_1 (tree, tree, tree_code ctx = ERROR_MARK);
> extern tree fold_offsetof (tree);
>
> extern int complete_array_type (tree *, tree, bool);
> --- gcc/c-family/c-common.c.jj 2018-05-06 23:12:49.135619681 +0200
> +++ gcc/c-family/c-common.c 2018-05-08 21:56:24.635088315 +0200
> @@ -6168,10 +6168,12 @@ c_common_to_target_charset (HOST_WIDE_IN
>
> /* Fold an offsetof-like expression. EXPR is a nested sequence of component
> references with an INDIRECT_REF of a constant at the bottom; much like the
> - traditional rendering of offsetof as a macro. Return the folded result.
> */
> + traditional rendering of offsetof as a macro. TYPE is the desired type of
> + the whole expression to which it will be converted afterwards.
> + Return the folded result. */
>
> tree
> -fold_offsetof_1 (tree expr, enum tree_code ctx)
> +fold_offsetof_1 (tree type, tree expr, enum tree_code ctx)
> {
> tree base, off, t;
> tree_code code = TREE_CODE (expr);
> @@ -6196,10 +6198,12 @@ fold_offsetof_1 (tree expr, enum tree_co
> error ("cannot apply %<offsetof%> to a non constant address");
> return error_mark_node;
> }
> + if (!POINTER_TYPE_P (type))
> + return convert (type, TREE_OPERAND (expr, 0));
> return TREE_OPERAND (expr, 0);
>
> case COMPONENT_REF:
> - base = fold_offsetof_1 (TREE_OPERAND (expr, 0), code);
> + base = fold_offsetof_1 (type, TREE_OPERAND (expr, 0), code);
> if (base == error_mark_node)
> return base;
>
> @@ -6216,7 +6220,7 @@ fold_offsetof_1 (tree expr, enum tree_co
> break;
>
> case ARRAY_REF:
> - base = fold_offsetof_1 (TREE_OPERAND (expr, 0), code);
> + base = fold_offsetof_1 (type, TREE_OPERAND (expr, 0), code);
> if (base == error_mark_node)
> return base;
>
> @@ -6273,12 +6277,14 @@ fold_offsetof_1 (tree expr, enum tree_co
> /* Handle static members of volatile structs. */
> t = TREE_OPERAND (expr, 1);
> gcc_checking_assert (VAR_P (get_base_address (t)));
> - return fold_offsetof_1 (t);
> + return fold_offsetof_1 (type, t);
>
> default:
> gcc_unreachable ();
> }
>
> + if (!POINTER_TYPE_P (type))
> + return size_binop (PLUS_EXPR, base, convert (type, off));
> return fold_build_pointer_plus (base, off);
> }
>
> @@ -6287,7 +6293,7 @@ fold_offsetof_1 (tree expr, enum tree_co
> tree
> fold_offsetof (tree expr)
> {
> - return convert (size_type_node, fold_offsetof_1 (expr));
> + return convert (size_type_node, fold_offsetof_1 (size_type_node, expr));
> }
Maybe add a type parameter that defaults to size_type_node...
>
> --- gcc/c/c-fold.c.jj 2018-01-17 22:00:12.310228253 +0100
> +++ gcc/c/c-fold.c 2018-05-08 21:52:43.303940175 +0200
> @@ -473,7 +473,8 @@ c_fully_fold_internal (tree expr, bool i
> && (op1 = get_base_address (op0)) != NULL_TREE
> && INDIRECT_REF_P (op1)
> && TREE_CONSTANT (TREE_OPERAND (op1, 0)))
> - ret = fold_convert_loc (loc, TREE_TYPE (expr), fold_offsetof_1 (op0));
> + ret = fold_convert_loc (loc, TREE_TYPE (expr),
> + fold_offsetof_1 (TREE_TYPE (expr), op0));
...and then this can be
fold_offsetof (op0, TREE_TYPE (exp0))
? Or a separate function called something like
fold_offsetof_with_type, if you'd prefer.
Jason
