<div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Tue, 12 Oct 2021 at 22:16, Aaron Ballman <<a href="mailto:aaron@aaronballman.com">aaron@aaronballman.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On Tue, Oct 12, 2021 at 7:49 AM Martin Brænne <<a href="mailto:mboehme@google.com" target="_blank">mboehme@google.com</a>> wrote:<br>
><br>
><br>
><br>
> On Mon, 11 Oct 2021 at 18:54, Aaron Ballman <<a href="mailto:aaron@aaronballman.com" target="_blank">aaron@aaronballman.com</a>> wrote:<br>
>><br>
>> On Mon, Oct 11, 2021 at 10:28 AM Martin Brænne via cfe-dev<br>
>> <<a href="mailto:cfe-dev@lists.llvm.org" target="_blank">cfe-dev@lists.llvm.org</a>> wrote:<br>
>> ><br>
>> > I would like to propose a new attribute `annotate_type` that would be analogous to the existing `annotate` attribute but for use on types. As for `annotate`, the typical use case would be to add annotations to be used by static analysis tools.<br>
>> ><br>
>> > I have a draft patch implementing this attribute (<a href="https://reviews.llvm.org/D111548" rel="noreferrer" target="_blank">https://reviews.llvm.org/D111548</a>), but before it's reviewed I wanted to solicit some feedback more broadly on whether people feel this makes sense.<br>
>><br>
>> Thanks for the proposal! One question I have is whether we need an<br>
>> additional attribute for this instead of reworking [[clang::annotate]]<br>
>> so that it applies to either a type or a declaration. Is a separate<br>
>> attribute necessary because there may be unfortunate problems with<br>
>> using __attribute__((annotate)) as the spelling?<br>
><br>
><br>
> Yes, that's exactly the problem. Today, the GNU spelling can be applied both before and after the type name:<br>
><br>
>   __attribute__((annotate("foo"))) int i1;<br>
>   int __attribute__((annotate("foo"))) i2;<br>
><br>
> We would need to reinterpret the second variant to refer (or "appertain") to the decl-specifier-seq, not to the declaration (which is, IIUC, what the C++ standard would prescribe if this was a C++ attribute). But as it's possible that this variant is already being used "in the wild" with the intent that it should refer to the declaration, I think this isn't something we can change.<br>
<br>
Agreed, thanks for the confirmation that this was the reason why we<br>
need a second attribute.<br>
<br>
> Hence, the practical solution seems to be to introduce a separate attribute for types, for which it is then unambiguous what it should apply to.<br>
<br>
Another possible solution would be to not support<br>
__attribute__((annotate)) as a type attribute (e.g., you can use<br>
[[clang::annotate]] as a declaration or a type attribute, but we would<br>
restrict __attribute__((annotate)) to only ever be a declaration<br>
attribute.).<br></blockquote><div><br></div><div>I guess that would be an option too, though I think this would require some changes to Clang to allow C++ attributes to be used in this position:</div><div><br></div><div>int [[clang::annotate]] foo;</div><div><br></div><div>See also discussion below.</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
> As a side note, IIUC, Clang currently more generally lumps GNU attributes that occur in these two positions together, right? I'm looking at Parser::ParseSimpleDeclaration(), specifically this line:<br>
><br>
> DS.takeAttributesFrom(Attrs);<br>
><br>
> For C++ attributes, I believe this would not be correct, but that's not (currently) a concern because AFAICT Clang currently doesn't allow C++ attributes to occur after a decl-specifier-seq at all (because, I presume, Clang doesn't yet support any C++ attributes that can occur in this position).<br>
<br>
We currently don't support any attributes that appertain to the<br>
declaration specifiers (in any spelling mode).</blockquote><div><br></div><div>But one of the enumerators in TypeAttrLocation is this:</div><div><br></div><div>  /// The attribute is in the decl-specifier-seq.<br>  TAL_DeclSpec,<br></div><div><br></div><div>And it looks as if it is used?</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">GNU attributes will<br>
"slide" to the type or the declaration based on the name of the<br>
attribute when doing semantic processing for them (see the<br>
moveAttrFromListToList() calls in SemaType.cpp for some examples of<br>
this).<br></blockquote><div><br></div><div>Thanks, that's an interesting pointer.</div><div><br></div><div>I wondering now though: What do attributes attach to when the "sliding" logic doesn't apply to them (because it doesn't know them)?</div><div><br></div><div>Maybe I should clarify my use case (which will probably benefit the whole discusssion). What I'm looking to be able to do is to attach attributes to different levels of a potentially multi-level pointer. For example:</div><div><br></div><div>int __attribute__((annotate_type("A"))) * __attribute__((annotate_type("B"))) * __attribute__((annotate_type("C"))) pp;</div><div><br></div><div>The "A" annotation is what I mean by being able to attach an attribute to the decl specifiers (maybe my terminology isn't accurate here?).</div><div><br></div><div>One use case for this is being able to specify which of the levels a nullability annotation refers to and not just have it always implicitly refer to the outermost pointer, as I believe the existing nullability annotations do. (See the Clang fork here for an example that does something very similar: <a href="https://github.com/sampsyo/quala">https://github.com/sampsyo/quala</a>)</div><div><br></div><div>Staying with the nullability example, it's important to be able to attach an annotation to the base type itself -- imagine the `int` in the example above was instead a `unique_ptr`.</div><div><br></div><div>This does seem to be possible to some extent with Clang today because if I feed the example above to the code in my draft change, I get `AttributedTypeLoc`s for the types `int`, `int *`, and `int **`, with annotations "A", "B", and "C" respectively.<br></div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
> This also means that if we want to use the proposed `annotate_type` attribute in this position, we have to use the GNU spelling.<br>
<br>
I don't think that's correct. We certainly support type attributes<br>
using a C++ spelling already.<br></blockquote><div><br></div><div>I think this only works though if the attribute is applied to a declarator, but not to the decl specifiers? Here's an excerpt from the test in my draft change (<a href="https://reviews.llvm.org/D111548">https://reviews.llvm.org/D111548</a>):</div><div><br></div><div>  int __attribute__((annotate_type("bar"))) w;<br>  int [[clang::annotate_type("bar")]] w2; // expected-error {{'annotate_type' attribute cannot be applied to types}}<br><br>  int *__attribute__((annotate_type("bar"))) x;<br>  int *[[clang::annotate_type("bar")]] x2;<br></div><div><br></div><div>The error seems to be coming from the following chunk of code in Parser::ParseDeclarationSpecifiers(), which is specific to C++ attributes:</div><div><br></div><div>        // Reject C++11 attributes that appertain to decl specifiers as<br>        // we don't support any C++11 attributes that appertain to decl<br>        // specifiers. This also conforms to what g++ 4.8 is doing.<br>        ProhibitCXX11Attributes(attrs, diag::err_attribute_not_type_attr);</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
> This is an unfortunate niggle, but teaching Clang to correctly process C++ attributes in this position seems like a non-trivial undertaking because it would require us to treat GNU spellings and C++ spellings differently in Parser::ParseSimpleDeclaration(), which seems like it would be a bit of a headache.<br>
<br>
I don't think we'll have to go down that route, hopefully.<br>
<br>
>> What kind of type semantic impacts does this attribute have on the<br>
>> type identity for things like overloading, name mangling, type<br>
>> conversion, etc?<br>
><br>
> Good point. The intent is that the attribute should have no impact. Anything else would be pretty problematic from a portability point of view because other compilers will simply ignore the attribute -- so if it had an effect on the type system in Clang, that could cause other compilers to generate different code.<br>
<br>
Vendor-specific attributes are inherently not portable or safe to<br>
ignore.</blockquote><div><br>But for C++ attributes at least, the standard says that compilers should do exactly this (i.e. ignore unknown attributes)?</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Generally speaking, if a type attribute has no type semantics,<br>
it probably shouldn't be a type attribute. However, because this<br>
attribute doesn't really have any semantics beyond "keep this<br>
information associated with the type", it's a bit more complex.<br>
Consider:<br>
<br>
void func(int i);<br>
void func(int [[clang::annotate("hoo boy")]] i);<br>
<br>
I can see an argument that this is not a valid overload set because<br>
both functions accept an integer.</blockquote><div><br></div><div>I think nullability attributes are a good analogy here (and essentially "prior art"). Here's an example:</div><div><br></div><div>void func(int* i) {}<br>void func(int* _Nonnull i) {}<br></div><div><br></div><div>(<a href="https://godbolt.org/z/sP6za4WEE">https://godbolt.org/z/sP6za4WEE</a>)<br></div><div><br></div><div>Clang doesn't consider these to be overloads and instead complains about a redefinition.</div><div><br></div><div>I can certainly see how for different use cases you _would_ want an attribute to create a different type -- but I would say those would then require a second, separate attribute.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">However:<br>
<br>
void func(int i) [[clang::annotate("ABI=v1")]];<br>
void func(int i) [[clang::annotate("ABI=v2")]];<br>
<br>
I can see an argument that this is a valid overload set because<br>
something else (the linker, whatever) is using that annotated<br>
information to provide additional semantics beyond what the compiler<br>
cares about.<br></blockquote><div><br></div><div>As a data point that my be useful, I looked at what Clang does when I move those attributes to a position where they are declaration attributes (so that I can run the current Clang on them), i.e.:</div><div><br></div><div> [[clang::annotate("ABI=v1")]] void func(int i) {}<br> [[clang::annotate("ABI=v2")]] void func(int i) {}</div><div><br></div><div>(<a href="https://godbolt.org/z/fo5vrso6d">https://godbolt.org/z/fo5vrso6d</a>)<br></div><div><br></div><div>Again, Clang doesn't consider these to be overloads and instead complains about a redefinition.<br></div><div> </div><div>So it seems that the existing semantics of the `annotate` attribute are similar to what I'm proposing for the `annotate_type` attribute.</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
>> Also, one problem here is -- [[clang::annotate]] isn't just used for<br>
>> the static analyzer, it's also something that can be used by backend<br>
>> tools (for example, when using attribute plugins).<br>
><br>
><br>
> It's good that you bring this up because I'm actually also thinking about extending ParsedAttrInfo to handle type attributes (with a new handleTypeAttribute() function as an analog to handleDeclAttribute()). The proposed annotate_type attribute would then also be a natural candidate for an attribute that the new handleTypeAttribute() could add to types.<br>
<br>
Oh, neat! Unifying type/stmt/decl attributes more is definitely a great goal.<br>
<br>
> Is this the concern that you had here or were you thinking about something different?<br>
<br>
Not really, it was more along the lines of lowering to LLVM IR (below)<br>
so that the backend can also take advantage of this.<br>
<br>
>> Do we need to<br>
>> consider what information should be lowered to LLVM IR when this new<br>
>> attribute appears on a type?<br>
>><br>
>> Thanks!<br>
><br>
> My intent was that this should have no effect on code generation.<br>
><br>
> Lowering annotations on declarations to LLVM IR has some obvious use cases, but for annotations on types it's less obvious how it would be useful to lower these to IR (and also less obvious to me how one would represent these). (Did you have anything specific in mind here?)<br>
<br>
LLVM currently has the ability to describe some types (e.g.,<br>
`%struct.X = type { i32 }`), so I was envisioning attaching this<br>
attribute to the type (either as an LLVM attribute or metadata, I have<br>
no idea what the distinction really is between them). LLVM currently<br>
supports plugins as does Clang, and Clang's support for attributes in<br>
plugins still strongly encourages users to use existing attributes to<br>
surface functionality, so I can imagine users wanting to use the<br>
annotate type attribute in the same way they already use the annotate<br>
declaration attribute for this sort of collusion. However, I don't<br>
think the codegen needs to happen up front -- mostly trying to<br>
understand the shape of the design still.<br></blockquote><div><br></div><div>Makes sense.</div><div><br></div><div>Sorry that this has become such a long email, but I hope I've been able to give you a clearer picture of what I'm trying to achieve.</div><div><br></div><div>Cheers,</div><div><br></div><div>Martin</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
~Aaron<br>
<br>
> This could actually make another case for why we have two different attributes, because that would make the distinction clearer that we're doing code generation for one but not the other.<br>
><br>
> Thanks,<br>
><br>
> Martin<br>
><br>
>><br>
>> ~Aaron<br>
>><br>
>> ><br>
>> > Thanks,<br>
>> ><br>
>> > Martin<br>
>> ><br>
>> > --<br>
>> ><br>
>> > Martin Brænne<br>
>> ><br>
>> > Software Engineer<br>
>> ><br>
>> > <a href="mailto:mboehme@google.com" target="_blank">mboehme@google.com</a><br>
>> ><br>
>> ><br>
>> > Google Germany GmbH<br>
>> > Erika-Mann-Straße 33<br>
>> > 80363 München<br>
>> ><br>
>> > Geschäftsführer: Paul Manicle, Halimah DeLaine Prado<br>
>> ><br>
>> > Registergericht und -nummer: Hamburg, HRB 86891<br>
>> ><br>
>> > Sitz der Gesellschaft: Hamburg<br>
>> ><br>
>> ><br>
>> > Diese E-Mail ist vertraulich. Wenn Sie nicht der richtige Adressat sind, leiten Sie diese bitte nicht weiter, informieren Sie den Absender und löschen Sie die E-Mail und alle Anhänge. Vielen Dank.<br>
>> ><br>
>> ><br>
>> ><br>
>> > This e-mail is confidential. If you are not the right addressee please do not forward it, please inform the sender, and please erase this e-mail including any attachments. Thanks.<br>
>> ><br>
>> > _______________________________________________<br>
>> > cfe-dev mailing list<br>
>> > <a href="mailto:cfe-dev@lists.llvm.org" target="_blank">cfe-dev@lists.llvm.org</a><br>
>> > <a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev" rel="noreferrer" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev</a><br>
><br>
><br>
><br>
> --<br>
><br>
> Martin Brænne<br>
><br>
> Software Engineer<br>
><br>
> <a href="mailto:mboehme@google.com" target="_blank">mboehme@google.com</a><br>
><br>
><br>
> Google Germany GmbH<br>
> Erika-Mann-Straße 33<br>
> 80363 München<br>
><br>
> Geschäftsführer: Paul Manicle, Halimah DeLaine Prado<br>
><br>
> Registergericht und -nummer: Hamburg, HRB 86891<br>
><br>
> Sitz der Gesellschaft: Hamburg<br>
><br>
><br>
> Diese E-Mail ist vertraulich. Wenn Sie nicht der richtige Adressat sind, leiten Sie diese bitte nicht weiter, informieren Sie den Absender und löschen Sie die E-Mail und alle Anhänge. Vielen Dank.<br>
><br>
><br>
><br>
> This e-mail is confidential. If you are not the right addressee please do not forward it, please inform the sender, and please erase this e-mail including any attachments. Thanks.<br>
</blockquote></div><br clear="all"><div><br></div>-- <br><div dir="ltr" class="gmail_signature"><div dir="ltr"><div><div dir="ltr"><div><div dir="ltr"><div><div dir="ltr"><div dir="ltr"><div dir="ltr"><span><p dir="ltr" style="line-height:1.38;margin-top:0pt;margin-bottom:0pt"><span style="font-size:16px;font-family:Arial;color:rgb(0,0,0);font-weight:700;vertical-align:baseline;white-space:pre-wrap;background-color:transparent">Martin </span><font color="#000000" face="Arial"><span style="font-size:16px;white-space:pre-wrap"><b>Brænne</b></span></font></p><p dir="ltr" style="line-height:1.38;margin-top:0pt;margin-bottom:0pt"><span style="font-size:13.3333px;font-family:Arial;color:rgb(102,102,102);vertical-align:baseline;white-space:pre-wrap;background-color:transparent">Software Engineer</span></p><p dir="ltr" style="line-height:1.38;margin-top:0pt;margin-bottom:0pt"><span style="font-size:13.3333px;font-family:Arial;color:rgb(102,102,102);vertical-align:baseline;white-space:pre-wrap;background-color:transparent"><a href="mailto:mboehme@google.com" target="_blank">mboehme@google.com</a></span><br></p><p dir="ltr" style="line-height:1.38;margin-top:0pt;margin-bottom:0pt"><br></p><p dir="ltr" style="line-height:1.38;margin-top:0pt;margin-bottom:0pt"><span><span style="font-size:10pt;font-family:Arial;color:rgb(102,102,102);background-color:transparent;vertical-align:baseline;white-space:pre-wrap"><span style="border:none;display:inline-block;overflow:hidden;width:130px;height:48px"><img src="https://lh3.googleusercontent.com/0h4r5vSiCkVQWUJnZee4YR8tvgAGLZWeq2v1wdj1Ngn4gmVy0zIUYtLLBoLm6409EeeGWlrfqPU7dkpbkN5Dx4RBO3VRiI9WAPZY3WYjtvPTcmUU4vn6nL6cNMvjFX4_equklVWj" width="130" height="48" style="margin-left: 0px; margin-top: 0px;"></span></span></span></p></span><div><div style="font-size:13px"><span style="font-family:Arial,Verdana,sans-serif"><font color="#666666">Google Germany GmbH</font></span></div><div><div style="font-size:13px;font-family:Arial,Verdana,sans-serif"><font color="#666666">Erika-Mann-Straße 33</font></div><div style="font-size:13px;font-family:Arial,Verdana,sans-serif"><font color="#666666">80363 München<br></font></div><div style="font-size:13px;font-family:Arial,Verdana,sans-serif"><font color="#666666"><br></font></div><div style="font-family:Arial,Verdana,sans-serif"><span><p dir="ltr" style="line-height:1.38;margin-top:0pt;margin-bottom:0pt"><span style="font-size:10.6667px;font-family:Arial;color:rgb(183,183,183);vertical-align:baseline;white-space:pre-wrap;background-color:transparent">Geschäftsführer: Paul Manicle, Halimah DeLaine Prado</span></p><p dir="ltr" style="line-height:1.38;margin-top:0pt;margin-bottom:0pt"><span style="font-size:10.6667px;font-family:Arial;color:rgb(183,183,183);vertical-align:baseline;white-space:pre-wrap;background-color:transparent">Registergericht und -nummer: Hamburg, HRB 86891</span></p><p dir="ltr" style="line-height:1.38;margin-top:0pt;margin-bottom:0pt"><span style="font-size:10.6667px;font-family:Arial;color:rgb(183,183,183);vertical-align:baseline;white-space:pre-wrap;background-color:transparent">Sitz der Gesellschaft: Hamburg</span></p><br><p dir="ltr" style="line-height:1.38;margin-top:0pt;margin-bottom:0pt"><span style="font-size:10.6667px;font-family:Arial;color:rgb(183,183,183);vertical-align:baseline;white-space:pre-wrap;background-color:transparent">Diese E-Mail ist vertraulich. Wenn Sie nicht der richtige Adressat sind, leiten Sie diese bitte nicht weiter, informieren Sie den Absender und löschen Sie die E-Mail und alle Anhänge. Vielen Dank.</span></p><p dir="ltr" style="line-height:1.38;margin-top:0pt;margin-bottom:0pt"><span style="font-size:10.6667px;font-family:Arial;color:rgb(183,183,183);vertical-align:baseline;white-space:pre-wrap;background-color:transparent">       </span></p><p dir="ltr" style="line-height:1.38;margin-top:0pt;margin-bottom:0pt"><span style="font-size:10.6667px;font-family:Arial;color:rgb(183,183,183);vertical-align:baseline;white-space:pre-wrap;background-color:transparent">This e-mail is confidential. If you are not the right addressee please do not forward it, please inform the sender, and please erase this e-mail including any attachments. Thanks.</span></p></span></div></div></div></div></div></div></div></div></div></div></div></div></div></div>