On Wed, Nov 25, 2020 at 12:26:17PM -0500, Jason Merrill wrote:
> > + if (DECL_BIT_FIELD (fld)
> > + && DECL_NAME (fld) == NULL_TREE)
> > + continue;
>
> I think you want to check DECL_PADDING_P here; the C and C++ front ends set
> it on unnamed bit-fields, and that's what is_empty_type looks at.
Ok, changed in my copy. I'll also post a patch for
__builtin_clear_padding to use DECL_PADDING_P in there instead of
DECL_BIT_FIELD/DECL_NAME==NULL.
> > + if (TREE_CODE (TREE_TYPE (arg)) == ARRAY_TYPE)
> > + {
> > + /* Don't perform array-to-pointer conversion. */
> > + arg = mark_rvalue_use (arg, loc, true);
> > + if (!complete_type_or_maybe_complain (TREE_TYPE (arg), arg, complain))
> > + return error_mark_node;
> > + }
> > + else
> > + arg = decay_conversion (arg, complain);
>
> bit_cast operates on an lvalue argument, so I don't think we want
> decay_conversion at all here.
>
> > + if (error_operand_p (arg))
> > + return error_mark_node;
> > +
> > + arg = convert_from_reference (arg);
>
> This shouldn't be necessary; the argument should already be converted from
> reference. Generally we call convert_from_reference on the result of some
> processing, not on an incoming argument.
Removing these two regresses some tests in the testsuite.
It is true that std::bit_cast's argument must be a reference, and so when
one uses std::bit_cast one won't run into these problems, but the builtin's
argument itself is an rvalue and so we need to deal with people calling it
directly.
So, commenting out the decay_conversion and convert_from_reference results
in:
extern V v;
...
__builtin_bit_cast (int, v);
no longer being reported as invalid use of incomplete type, but
error: '__builtin_bit_cast' source size '' not equal to destination type size
'4'
(note nothing in between '' for the size because the size is NULL).
Ditto for:
extern V *p;
...
__builtin_bit_cast (int, *p);
I guess I could add some hand written code to deal with incomplete types
to cure these. But e.g. decay_conversion also calls mark_rvalue_use which
we also need e.g. for -Wunused-but-set*, but don't we also need it e.g. for
lambdas? The builtin is after all using the argument as an rvalue
(reads it).
Another change that commenting out those two parts causes is different
diagnostics on bit-cast4.C,
bit-cast4.C:7:30: error: '__builtin_bit_cast' is not a constant expression
because 'const int* const' is a pointer type
bit-cast4.C:7:30: error: '__builtin_bit_cast' is not a constant expression
because 'int D::* const' is a pointer to member type
bit-cast4.C:7:30: error: '__builtin_bit_cast' is not a constant expression
because 'int (D::* const)() const' is a pointer to member type
The tests expect 'const int*', 'int D::*' and 'int (D::*)() const', i.e. the
toplevel qualifiers stripped from those.
Commenting out just the arg = convert_from_reference (arg); doesn't regress
anything though, it is the decay_conversion.
> > +/* Attempt to interpret aggregate of TYPE from bytes encoded in target
> > + byte order at PTR + OFF with LEN bytes. MASK contains bits set if the
> > value
> > + is indeterminate. */
> > +
> > +static tree
> > +cxx_native_interpret_aggregate (tree type, const unsigned char *ptr, int
> > off,
> > + int len, unsigned char *mask,
> > + const constexpr_ctx *ctx, bool *non_constant_p,
> > + location_t loc)
>
> Can this be, say, native_interpret_initializer in fold-const? It doesn't
> seem closely tied to the front end other than diagnostics that could move to
> the caller, like you've already done for the non-aggregate case.
The middle-end doesn't need it ATM for anything, plus I think the
ctx/non_constant_p/loc and diagnostics is really C++ FE specific.
If you really want it in fold-const.c, the only way I can imagine it is
that it would be
tree
native_interpret_aggregate (tree type, const unsigned char *ptr, int off,
int len, unsigned char *mask = NULL,
tree (*mask_callback) (void *, int) = NULL,
void *mask_data = NULL)
where C++ would call it with the mask argument, as mask_callback a FE function
that would emit the diagnostics and decide what to return when mask is set
on something, and mask_data would be a pointer to struct containing
const constexpr_ctx *ctx; bool *non_constant_p; location_t loc;
for it.
Jakub
