[libcxx-commits] [PATCH] D141321: [libc++] Add [[clang::lifetimebound]] attribute to std::forward and friends

[Aaron Ballman via Phabricator via libcxx-commits]

aaron.ballman added a comment.

In D141321#4040624 <https://reviews.llvm.org/D141321#4040624>, @ldionne wrote:

> Thanks! This is a nice improvement IMO.
>
> However, the CI issue you're seeing is actually a very interesting and slightly deeper issue. Indeed, remember when Clang decided to define `forward` and `move` as builtins in D123345 <https://reviews.llvm.org/D123345>? One impact of that change is that we don't control our `std::forward` and `std::move` anymore, so we'd need to make the change in this patch inside Clang, not inside libc++. An obvious follow-up question is then do we make the change in Clang and strive to keep libc++ in synchronization with how Clang defines `std::forward`? Or do we not care about keeping them in sync? Should we have a way to ensure that both definitions stay in sync?
>
> CC Clang folks @rnk @rsmith @aaron.ballman

I think trying to stay in sync is probably the right approach, but I don't have good ideas about how to automatically catch differences. However, I don't expect to make a lot of changes to these signatures, so maybe we don't need something to catch differences. I suspect the correct way to do this in Clang is to modify `Builtins.def` to add a new encoding for lifetime bound to the list of attributes, and then update `ASTContext::GetBuiltinType()`/`Sema::ActOnFunctionDeclarator()` to properly add the attribute where needed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141321