[cfe-dev] how should AnnotateAttr be inherited?

[Jacob Bandes-Storch via cfe-dev]

Thank you, isInherited() is what I was looking for :)

On the topic of attributes in general, I found it curious that clang
completely drops unrecognized attributes and they aren't made available in
any Attr (as far as I could tell), and thus can't be checked by custom
plugins, etc. Do you know if there's a specific reason for this? Why not
keep the original spelling around in something like "UnrecognizedAttr"?

Jacob

On Sun, May 6, 2018 at 7:08 AM, Aaron Ballman <aaron.ballman at gmail.com>
wrote:

> On Sat, May 5, 2018 at 11:26 PM, Richard Smith <richard at metafoo.co.uk>
> wrote:
> > +Aaron
> >
> > Tangentially related:
> >
> > It strikes me our current inheritance behavior for AnnotateAttr (ie,
> inherit
> > unless already present) makes no sense, as we have no idea what the
> > attribute means or how it should be inherited from one declaration to
> > another. If one declaration has the attribute and a redeclaration has the
> > attribute with a different string, under what circumstances should we
> drop
> > the old attribute? It seems to me that the only sensible options are
> > "inherit all attributes even if one is already present on the new
> > declaration" (eg, set InheritEvenIfAlreadyPresent) and "never inherit an
> > AnnotateAttr" (eg, make it not be an InheritableAttr).
> >
> > I have no particular preference between those two; either option seems
> like
> > it creates some extra hassle for some users of the attribute, and less
> > hassle for others, but the status quo seems wrong. Aaron, what do you
> think?
>
> I agree that the status quo is wrong, but I don't have a strong sense
> for whether we should always inherit or never inherit. I kind of feel
> that always inheriting but only if there's a difference in attributes
> would be in the spirit of the annotate attribute's "this is totally
> generic and can mean anything" design. However, I don't think there's
> a "clearly correct" behavior here, which also suggests that perhaps it
> should be an error (or at least a warning) to try to inherit annotate
> attributes with different arguments.
>
> ~Aaron
>
> >
> >
> > On 5 May 2018 at 20:18, Richard Smith <richard at metafoo.co.uk> wrote:
> >>
> >> On 5 May 2018 at 13:33, Jacob Bandes-Storch via cfe-dev
> >> <cfe-dev at lists.llvm.org> wrote:
> >>>
> >>> I am using an AST matcher and would like to prevent an annotation
> >>> attribute from being placed on an out-of-line constructor definition.
> >>>
> >>>     class Foo {
> >>>         explicit Foo(int);  // ok
> >>>         [[clang::annotate("implicit")]] Foo(int);  // ok
> >>>     };
> >>>
> >>>     explicit Foo::Foo() {}  // disallowed
> >>>     [[clang::annotate("implicit")]] Foo::Foo(int) {}  // should be
> >>> disallowed
> >>>
> >>>
> >>> However it seems that when I match the out-of-line decl,
> >>> `decl->specific_attrs<AnnotateAttr>()` contains the attribute even if
> it was
> >>> placed on the original in-class declaration and not on the out-of-line
> >>> definition.
> >>>
> >>> How can I determine whether an Attr* was written on the out-of-line
> decl?
> >>
> >>
> >> You can look over the AnnotateAttr(s) on the out-of-line declaration and
> >> check !Attr::isInherited() to see whether they were written on that
> >> declaration.
> >>
> >>>
> >>> I tried checking (!(attr->getLocation() < decl->getLocStart()) &&
> >>> !(decl->getLocEnd() < attr->getLocation())), however it seems the two
> >>> locations are not comparable. When I look at their getRawEncoding()s,
> the
> >>> attribute has values in ranges like 2147587494-2147587527 while the
> decl's
> >>> values are more reasonable, such as 3049-3079.
> >>
> >>
> >> The numerical values of source locations (and their comparison order
> under
> >> <) have no strong relation to the semantics of the program and should be
> >> thought of as arbitrary; we give a total order under operator< to
> facilitate
> >> use of SourceLocations in std::map and the like. (You can use
> >> SourceManager::isBeforeInTranslationUnit if you want to compare source
> >> locations in order of appearance, in cases where that makes sense.)
> >> Currently, the high bit of source locations indicates that they
> represent a
> >> location within a macro expansion (presumably in your actual testcase
> the
> >> attribute was coming from a macro expansion), but it would be unwise to
> rely
> >> on that and you should use the high-level semantically-aware
> functionality
> >> in SourceManager instead.
> >>
> >>>
> >>> I couldn't find any documentation about attribute source locations
> being
> >>> negative.
> >>
> >>
> >> The documentation comment for the SourceLocation class describes the
> >> encoding we currently use:
> >>
> >> /// Technically, a source location is simply an offset into the
> manager's
> >> view
> >> /// of the input source, which is all input buffers (including macro
> >> /// expansions) concatenated in an effectively arbitrary order. The
> >> manager
> >> /// actually maintains two blocks of input buffers. One, starting at
> >> offset
> >> /// 0 and growing upwards, contains all buffers from this module. The
> >> other,
> >> /// starting at the highest possible offset and growing downwards,
> >> contains
> >> /// buffers of loaded modules.
> >> ///
> >> /// In addition, one bit of SourceLocation is used for quick access to
> the
> >> /// information whether the location is in a file or a macro expansion.
> >
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20180506/5bcf5c2e/attachment.html>