On 3/19/19 12:30 PM, Jeff Law wrote:
On 2/22/19 9:42 AM, Marek Polacek wrote:
On Thu, Feb 21, 2019 at 03:39:21PM -0700, Martin Sebor wrote:
On 2/21/19 1:27 PM, Jeff Law wrote:
On 2/21/19 1:12 PM, Martin Sebor wrote:
On 2/21/19 12:08 PM, Jeff Law wrote:
On 2/18/19 7:53 PM, Martin Sebor wrote:
Please let me know what it will take to get the fix for these two
issues approved. I've answered the questions so I don't know what
else I'm expected to do here.
https://gcc.gnu.org/ml/gcc-patches/2019-02/msg00793.html
I think there is still a fundamental disagreement about whether or not
this should be handling expressions. Without an agreement on that it's
hard to see how this could go forward.
I think it's wrong to hold up a fix for an ICE because you don't
like the current design. The built-in successfully handles common
expressions (see c-c++-common/builtin-has-attribute-5.c). It just
fails for some of the less common ones. Not fixing those will only
penalize users who run into the ICE, and cast a bad light on
the quality of the release. Any design questions should be
settled separately of these ICEs (should have been when
the feature was being reviewed).
That said, I have explained the rationale for the current design.
Neither you nor Jakub has explained what you find wrong with it or
why either of your alternatives is preferable. You said it should
be an error to apply the built-in to expressions (why?). Jakub
thought there perhaps should be two built-ins, one for expressions
and another for decls. His rationale? The current design is not
good. Clearly, the two of you don't agree on what you'd like to
see; the only thing you agree on is that you don't like what's
there now. What do you expect me to do with that, now at the end
of stage 4?
Fix the ice in another way. Handling expressions here seems
fundamentally wrong. ISTM that making this query on an expression
should result in an immediate error.
Sorry but "it seems wrong" is not a compelling argument. You need
to explain what you think is wrong with it.
"Attributes apply to decls and types, not expressions" seems like an
argument strong enough. (There are also special attributes for enums
and ';' and labels.)
So I think Martin's argument is that for an expression we can look at
the type of the expression and see if the attribute is on that type.
That leads to some inconsistencies (that Jakub has pointed out) where an
unfolded expression could produce a different result than it's folded
result if the result was a constant.
The built-in is evaluated during parsing (same as __alignof__)
so I'm not sure I understand what inconsistencies it could be
subject to that __alignof__ isn't. I'm open to examples, I just
can't think of any.
AFAICT the goal (outside handling just types) here is to be able to ask
things like is a particular field within a structure packed, aliasing
properties of objects, etc.
Yes. The goal is to query functions, variables (including members),
and types for most attributes known to GCC that may be attached to
them.
My next thought was to use typeof to extract the type of the expression
and pass that through (independent of Marek suggesting the same).
Martin points out in a subsequent email that won't work because p->a in
the supplied example returns an int rather than a packed int. One
could argue that's the wrong return value for typeof, but I don't think
it's that clear cut. In this specific instance I'd claim it's wrong,
but I'd hazard a guess we don't have clear semantics in this space.
__alignof__ is documented to work this way:
If the operand of __alignof__ is an lvalue rather than a type,
its value is the required alignment for its type, taking into
account any minimum alignment specified with GCC's __attribute__
extension (see Variable Attributes). For example, after this
declaration:
struct foo { int x; char y; } foo1;
the value of __alignof__ (foo1.y) is 1, even though its actual
alignment is probably 2 or 4, the same as __alignof__ (int).
What isn't documented is the semantics for expressions that aren't
lvalues. They are nevertheless accepted and, AFAICT, __alignof__
returns the alignment of their type.
I'll note that our documentation clearly states that attributes can be
applied to functions, variables, labels, enums, statements and types.
No provision AFAICT is made for expressions. Users have never had the
ability or assumption that they can ask for attributes on an expression.
Not quite. As I said above, __alignof__ accepts expressions
because there is no way to apply the operator to a member without
using an expression of some sort. But it also accepts any other
expressions.
Martin's patch essentially allows for asking indirectly -- but I suspect
it's going to be inconsistent in more ways than Jakub pointed out as the
type system in gimple is "loose" in certain ways and the result could
well change as we go through the optimizer pipeline (ie, this is bigger
than just constant folding causing inconsistencies).
As I explained above, the built-in is fully evaluated and folded
by the front-ends during parsing. It never makes it into GIMPLE.
Given that we're talking about a new feature with no existing end user
expectations and the fact that attributes are documented as applying
only to things which are decls or types, I'm more inclined to stay
conservative at this point and reject expressions as syntax errors.
We can revisit in gcc-10 and see if there's a reasonable way to tighten
semantics around typeof and/or loosen the requirement that we can only
ask about attributes of DECL/TYPE nodes and define good semantics around
that if we try. I'm also willing to revisit if other reviewers chime in
with other opinions -- this kind of stuff isn't my strongest area.
There is no way to do what you suggest without preventing
the built-in from being used with struct members, and I don't
see what good crippling it like that would do, or what could
then be done differently to fix it.
By the way of an example, both the __alignof__ and the built-in
arguments are accepted today but the latter would not work if
references to members were considered an error:
struct S { __attribute__ ((packed, aligned (2))) int i; };
_Static_assert (__alignof__ (((struct S*)0)->i) == 2);
_Static_assert (__alignof__ (__typeof__ (((struct S*)0)->i)) == 4);
_Static_assert (__builtin_has_attribute (((struct S*)0)->i, packed));
_Static_assert (__builtin_has_attribute (((struct S*)0)->i, aligned));
_Static_assert (!__builtin_has_attribute (__typeof__ (((struct
S*)0)->i), packed));
_Static_assert (!__builtin_has_attribute (__typeof__ (((struct
S*)0)->i), aligned));
because the attribute is attached to i's DECL and not to its type,
and there is no other way to refer to the member i of struct S.
Martin
