[libcxx-commits] [PATCH] D118926: [libc++] Adds TEST_IF_INT128.

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

Quuxplusone added a comment.

In D118926#3305279 <https://reviews.llvm.org/D118926#3305279>, @Mordante wrote:

> For format I use it in a few places. The difference there is it's at the end of the list which makes it more awkward to use `#ifdef` due to the "hanging" `>`. [...] The reason to put them at the end is that the test does something like `auto foo = std::get<1>(types)`.

Sounds like `std::get<1>(types)` is the root of the problem. Since you know `get<1>(types)` is always `long long`, why not replace the magic number `1` with the "magic number" `long long`? Then the test would be more robust not only against people adding `uint128_t` at the beginning of the list, but //also// against people sorting the list, or adding `char16_t`, or whatever other permutations they might do under maintenance. (Btw, I can imagine a reason it wouldn't be quite that simple: maybe the type list contains both `long long` and `int64_t`, so `long long` would be ambiguous while `1` is unambiguous. But still.)

Also, how come these format-uses aren't part of this PR? Are they not landed yet? Could we defer this PR until after the format-uses are landed, and/or take this (currently too-abstract) discussion over to that PR?

> 1. `_LIBCPP_IF_WIDE_CHARACTERS` and `TEST_IF_INT128`
> 2. `_LIBCPP_IF_HAS_WIDE_CHARACTERS` and `TEST_IF_HAS_INT128`
>
> Based on Arthur's suggestion I'm now more tending to option 2.

Weakly option 2. However, I remain unconvinced that the macro is needed at all.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118926