[libcxx-commits] [PATCH] D143447: [libc++] Implement P1328R1 constexpr std::type_info::operator==

[Louis Dionne via Phabricator via libcxx-commits]

ldionne marked 6 inline comments as done.
ldionne added subscribers: goncharov, mstorsjo.
ldionne added a comment.

In D143447#4110194 <https://reviews.llvm.org/D143447#4110194>, @aaron.ballman wrote:

> Precommit CI looks like it perhaps has some relevant failures that need to be addressed.

It looks like this is the VCRuntime used by the bots being too old and not supporting `constexpr`. I think the right thing to do is to UNSUPPORT the test in that case.

@goncharov @mstorsjo  Would it be possible to update the VCRuntime on the Windows hosts?



================
Comment at: libcxx/include/typeinfo:112
+      // from different translation units, so it is sufficient to compare their addresses.
+      if (__libcpp_is_constant_evaluated()) {
+        return this == &__arg;
----------------
Mordante wrote:
> Just curious, but why `__libcpp_is_constant_evaluated` instead of `std::is_constant_evaluated`?
This has to work in pre-C++20 too.


================
Comment at: libcxx/include/typeinfo:113
+      if (__libcpp_is_constant_evaluated()) {
+        return this == &__arg;
+      }
----------------
Mordante wrote:
> 
Since we know what the type is and we control it, we know it doesn't overload `operator&` so I'd be happy to avoid a dependency on `std::addressof`.


================
Comment at: libcxx/test/std/language.support/support.rtti/type.info/type_info.equal.pass.cpp:11
+//
+//  bool operator==(const type_info& rhs) const noexcept; // constexpr since C++23
+
----------------
aaron.ballman wrote:
> Mordante wrote:
> > Can you add a test for `noexcept` too?
> I'd also like a test for typeid of a polymorphic type since that's special. e.g.,
> ```
> #include <typeinfo>
> 
> struct Base {
>   virtual void func() {}
> };
> struct Derived : Base {
>   virtual void func() {}
> };
> 
> static constexpr Derived D;
> constexpr const Base &get_it() {
>   return D;
> }
> 
> static_assert(typeid(get_it()) == typeid(Derived));
> ```
> (or something similar)
Addressed both. Aaron, it'd be great if you could have a look to make sure the test I added addresses what you wanted -- I think it does.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143447