[libcxx-commits] [PATCH] D115100: [libc++] Enable the optimized _IsSame on GCC as well as Clang.

[Arthur O'Dwyer via Phabricator via libcxx-commits]

Quuxplusone marked 5 inline comments as done.
Quuxplusone added inline comments.


================
Comment at: libcxx/include/type_traits:564-565
+// _IsSame<A,B> and _IsSame<C,D> are the same type (namely, false_type).
+// If we ever need to support a compiler without the __is_same builtin,
+// we can make this an alias template for `is_same<T,U>::type`.
 
----------------
ldionne wrote:
> I would remove that part of the comment -- I don't see what it adds.
> 
> Also, I would document that this `_IsSame` can't be used anywhere that affects mangling in order to save some cycles if someone encounters the same issue in the future.
I added the latter part of the comment because it wasn't obvious to me at first glance. The alternative (and the first thing I happened to think of) is that on those platforms we define `_IsSame<T,U>` as an alias for `is_same<T,U>` and just have to beware of using `_IsSame` anywhere that the difference would be observable.
However, I'll move that into the commit message; and move the "affects mangling" bit (partly) out of the commit message into the comment.


================
Comment at: libcxx/include/type_traits:582-584
+// helper class
 
+struct __two {char __lx[2];};
----------------
ldionne wrote:
> I'm going to be really nitpicky and ask that you make this change in a separate NFC commit, no review needed. Just to keep reviews clean.
Done!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115100