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. 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. 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. 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. 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). 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. Jeff
