[cfe-dev] Fwd: [PATCH] Adding a specific_attribute_reverse_iterator
listmail at philipreames.com
Mon Nov 26 18:38:32 PST 2012
At the high level, I am opposed to this change. As Jordan said, trying
to mask over a problem rather than fixing it is a bad idea. This type
of "fix" could create a nightmare of legacy behavior when users
accidentally start relying on ordering.
Is there a bug somewhere for this issue? I've spent a fair amount of
time in the attribute handling code recently and was unaware of it.
Specifically to your point about RNull, the use of a distinguished value
doesn't seem too evil to me provided it provides all the standard
iterator functionality (!=).
Given that walking over the set of attributes without checking whether
its empty is an exceedingly common pattern in our code, I do not think
the created on demand empty attribute is a good idea.
An alternate design would be to have a single static empty attributes
array and return an reverse iterator to the beginning and end of that.
That has some nasty implications for walking an data structure while
mutating it. While that may be fine by the spec - note may, I haven't
checked - it would be a debugging disaster.
p.s. Thank you for looking into this and starting a conversation.
Please keep pushing, maybe we can get the underlying issue fixed.
On 11/14/2012 02:00 PM, Alexandros Tzannes wrote:
> Hi all,
> the current Clang implementation has a specific_attr_iterator<AttrType>
> that allows iterating over the attributes of a certain type (all
> attributes are kept in a single llvm::SmallVector), but it lacks a
> reverse iterator. This patch adds this functionality.
> Why is this useful/needed (in the short run).
> The C++11 standard allows attributes to appertain to types and portions
> of the declarator, e.g.:
> int[[attr1] *[[attr2] *[[attr3] var;
> However, Clang is still collapsing those attributes on the declaration
> int **var [[attr1,attr2,attr3]].
> According to C++11 when multiple attributes of the same kind appertain
> to the same "node", their order is irrelevant (note that these
> attributes may have different attribute parameters):
> int var [[attrA("x"), attrA("y")]]
> int var [[attrA("y"), attrA("x")]]
> must mean the same thing.
> However, until attributes are allowed to appertain to all the new
> locations described by C++11, it would be useful to use their order to
> "map" them onto the different nodes they should appertain. The
> specific_attr_reverse_iterator provides some functionality towards that
> The implementation is a slightly modified version of the code of the
> corresponding forward iterator. A small difference is that the forward
> iterator returns a pointer to the underlying type *T, whereas the
> reverse one returns an object of type typedef
> For that reason, Decl::attr_rbegin() and Decl::attr_rend() for
> attr_reverse_iterator check if the Decl has attributes, and if so,
> return the begin or the end; otherwise, they return a sentinel RNull,
> which is implemented as a static member of Decl.
> Using the RNull sentinel is a bit awkward. Two alternatives are the
> 1. Make the user responsible of checking if a Decl has an attributes
> vector before trying to get a specific_attr_reverse_iterator. Trying to
> get such an iterator on a Decl without any attrs will generate an
> assertion failure. This approach gets rid of RNull, but requires users
> to treat fwd and reverse iterators differently.
> 2. Each time the user tries to get a reverse iterator on a Decl without
> attributes, create an empty attribute vector and return the beginning
> (or the end) of it. This approach also removes the awkward RNull, but
> may create many unneeded empty attribute vectors.
> Please let me know if one of these alternatives is preferable.
> Alex Tzannes
> [[written during the Hacker Lab at last week's LLVM devellopers' meeting]]
> cfe-dev mailing list
> cfe-dev at cs.uiuc.edu
More information about the cfe-dev