[PATCH] D96832: [Clang][Attributes] Allow not_tail_called attribute to be applied to virtual function.

[Arthur O'Dwyer via Phabricator via cfe-commits]

Quuxplusone added inline comments.


================
Comment at: clang/include/clang/Basic/AttrDocs.td:4087-4088
 
-Marking virtual functions as ``not_tail_called`` is an error:
+Marking virtual functions as ``not_tail_called`` will not have effect on the
+overriding functions of derived classes:
 
----------------
ahatanak wrote:
> Quuxplusone wrote:
> > aaron.ballman wrote:
> > > 
> > /will not have effect/will have no effect/
> > However, this phrasing is easy to interpret the wrong way around: actually marking a (base-class) virtual function //will// affect overriding functions in derived classes! You meant that marking the //overrider// wouldn't retroactively affect the //overridden// function from the base class.
> > I think the correct explanation would be more like this:
> > 
> >     Generally, marking an overriding virtual function as ``not_tail_called`` is
> >     not useful, because this attribute is a property of the static type. Calls
> >     made through a pointer or reference to the base class type will respect
> >     the ``no_tail_called`` attribute of the base class's member function,
> >     regardless of the runtime destination of the call.
> > 
> > I think it'd also be correct and helpful to add:
> > 
> >     Similarly, calls made through a function pointer will respect the
> >     ``no_tail_called`` attribute of the function pointer, not of its
> >     runtime destination.
> > 
> > (I admit this is mildly redundant with the `foo2` example above.)
> > calls made through a function pointer will respect the ``no_tail_called`` attribute of the function pointer
> 
> But function pointers currently can't be annotated with `no_tail_called`, right?
Ah, maybe. By "will respect the not-tail-called attribute of the function pointer," I meant both "will not-tail-call it if it has the attribute" and also "may tail-call it if it lacks the attribute." Sounds like only the latter is hittable today (as shown in the `foo2` example).

Also, I notice that everyone (including me) keeps misspelling the name of this attribute. I wish it were spelled as a command, like "nodiscard" — e.g. "notailcall" or "do_not_tail_call" — but I assume that ship has sailed?  Anyway, apparently `not_tail_called` is the current spelling, and we should make sure the docs use it consistently :)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D96832/new/

https://reviews.llvm.org/D96832