[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:4109
-      [[clang::not_tail_called]] int foo2() override;
-    };
   }];
----------------
(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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96832