On 3/21/19 3:59 PM, Martin Sebor wrote: > On 3/19/19 9:33 PM, Jeff Law wrote: >> On 3/19/19 8:22 PM, Joseph Myers wrote: >>> On Tue, 19 Mar 2019, Jeff Law wrote: >>> >>>> I'll note that our documentation clearly states that attributes can be >>>> applied to functions, variables, labels, enums, statements and types. >>> >>> A key thing here is that they can be applied to fields - that is, >>> they can >>> be properties of a FIELD_DECL. Referring to a field either requires an >>> expression referring to that field, or some other custom syntax that >>> uses >>> both a type name and a field name (like in __builtin_offsetof). >>> >> Thanks for chiming in, your opinions on this kind of thing are greatly >> appreciated. >> >> Understood WRT applying to fields, and I think that's consistent with >> some of what Jakub has expressed elsewhere -- specifically that we >> should consider allowing COMPONENT_REFs as the exception, returning the >> attributes of the associated FIELD_DECL in that case. >> >> Is there a need to call out BIT_FIELD_REFs where we can't actually get >> to the underlying DECL? And if so, how do we do that in the end user >> documentation that is clear and consistent? > > Is it possible to see a BIT_FIELD_REF here? I couldn't find a way. Unsure. It was a generic concern -- I don't have a motivating example. From very light reading in the front-ends, you're more likely to get one from the C++ front-end rather the C front-end. It may also be able to get a BIT_FIELD_REF from some OMP constructs. In general it looks like the front-ends aren't generating them often, preferring to stick with COMPONENT_REF instead.
> >> >> One of the big worries I've got here is that documenting when an >> expression as an argument to __builtin_has_attribute (or any attribute >> query) can be expected to work. It's always easier on our end users to >> loosen semantics of extensions over time than it is to tighten them. > > I wonder if a part of the issue isn't due to a mismatch between > the C terminology and what GCC uses internally. Every argument > to the built-in that's not a type (and every argument to attribute > copy which doesn't accept types) must be a C expression: My hesitation has been based more on the core concept that we don't attach attributes to expressions, only types and decls. Thus asking if an expression has any given attribute doesn't make much sense at first glance. In the copy attribute case the docs were pretty clear when and how it applied to expressions -- specifically stating that we're going to look at the underlying type and pull the attributes from there. Now that in turn raises its own set of issues. If the front-ends were to change the type of an expression (and they have in the past, particularly for COMPONENT_REF/INDIRECT_REF to avoid subsequent nop-casts), then we have to worry about the inconsistencies in behavior that's going to expose to the user. Another case where that could potentially happen would be LTO. I believe LTO will canonicalize/merge types to some degree and I don't offhand know the rules there and inconsistencies could be exposed to the user. Given that I think the front-end code that changed types of COMPONENT_REF/INDIRECT_REF to avoid subsequent nop-conversions is no longer in the tree and that (in theory) type merging in LTO should be considering attributes I went ahead and pushed those concerns aside for the copy attribute patch. I'll be doing the same when re-re-re-reviewing the __builtin_has_attribute patch :-) jeff
