[cfe-dev] how should AnnotateAttr be inherited?

[Richard Smith via cfe-dev]

+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?


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/20180505/640c1de5/attachment.html>