ahatanak added inline comments.
================ Comment at: clang/include/clang/Basic/AttrDocs.td:4109 - [[clang::not_tail_called]] int foo2() override; - }; }]; ---------------- Quuxplusone wrote: > (Moving into a thread) > > > This patch doesn't prevent the call to method in the code below from being > > tail called, > > but I suppose users would expect the attribute to prevent the tail call? > ``` > struct B { > virtual void method(); > }; > struct D : B { > [[clang::not_tail_called]] void method() override; > }; > ``` > > The way virtual calls are handled in C++ is, all attributes and properties of > the call are determined based on the //static// type at the call site; and > then the //runtime destination// of the call is determined from the pointer > in the vtable. Attributes and properties have no runtime existence, and so > they physically cannot affect anything at runtime. Consider > https://godbolt.org/z/P3799e : > > ``` > struct Ba { > virtual Ba *method(int x = 1); > }; > struct Da : Ba { > [[clang::not_tail_called]] [[nodiscard]] Da *method(int x = 2) noexcept > override; > }; > auto callera(Da& da) { > Ba& ba = da; > ba.method(); > } > ``` > Here the call that is made is a //virtual// call (because `Ba::method` is > virtual); with a default argument value of `1` (because `Ba::method`'s `x` > parameter has a default value of `1`); and it returns something of type `Ba*` > (because that's what `Ba::method` returns); and it is not considered to be > noexcept (because `Ba::method` isn't marked noexcept); and it's okay to > discard the result (because `Ba::method` is not nodiscard) and it is > tail-called (because `Ba::method` doesn't disallow tail calls). All of these > attributes and properties are based on the //static// type of variable `ba`, > despite the fact that //at runtime// we'll end up jumping to the code for > `Da::method`. According to the source code, statically, `Da::method` has a > default argument of `2`, returns `Da*`, is noexcept, and is nodiscard, and > disallows tail-calls. But we're not calling `da.method()`, we're calling > `ba.method()`; so none of that matters to our call site at `callera`. > > I think this patch is a good thing. OK, I see. I think this patch is fine then. Should we add an explanation of how virtual functions are handled? The doc currently just says the attribute prevents tail-call optimization on statically bound calls. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D96832/new/ https://reviews.llvm.org/D96832 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits