[libcxx-commits] [PATCH] D131732: [libcxx][NFC] utilises compiler builtins for unary transform type-traits

[Christopher Di Bella via Phabricator via libcxx-commits]

cjdb added a comment.

In D131732#3750935 <https://reviews.llvm.org/D131732#3750935>, @rupprecht wrote:

> The commit says NFC, but we found this to be an observable behavior change with `__restrict` now being dropped as part of `std::decay`: https://godbolt.org/z/zqvW478jq. It impacted us via use of `std::make_pair` which decays the types, and we had a `static_assert` checking that the type passed in had `__restrict` on it.
>
> We have a workaround (just don't use `std::make_pair`), but I figured I should mention this since the commit is labelled NFC, so any observable change is a surprise.
>
> IIUC, because `__restrict` is non-standard C++, it's implementation defined whether `std::decay` wants to modify it or not. Therefore, both before and after are "correct", and code should not rely on which option we choose.

Thanks for flagging this.

It was a deliberate choice to have `__decay` drop all CVR qualifiers in D116203 <https://reviews.llvm.org/D116203>, with the rationale being that if C++ supported `restrict`, it would probably be affected by `std::decay`. The libc++ change was supposed to be NFC, but I'd forgotten about the strictness of `__decay` by the time I'd gotten around to changing libc++ code.

I see two paths forward:

1. Wait to see if any libc++ maintainers have opinions regarding `__restrict`. I don't think that they have up until now, since their CI would've caught it otherwise; though they might want to retroactively have opinions now that we know people have been negatively impacted by this.
2. Immediately patch Clang so that `__decay` doesn't impact `__restrict`, since people have been negatively impacted by this change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131732