[libc++][format] Improve ABI stability.

[Mordante]

Summary

During the review of D115991 @vitaut pointed out the enum shouldn't
depend on whether or not _LIBCPP_HAS_NO_INT128 is defined. The current
implementation lets the enum's ABI depend on this configuration option
without a good cause.

[Mordante]

Fix the CI. I had validated that -Wswitch wasn't used on Linux, but it seems
other CI jobs do, leading to an expected error.

[ldionne]

Still LGTM pending CI.

[• Quuxplusone]

libcxx/include/__format/format_arg.h

42–43

Consider replacing this comment with a test in libcxx/test/libcxx/ that does assert(__format::__arg_t::__ptr == 14); and is not skipped when _LIBCPP_HAS_NO_INT128.
Mantra: "If you liked it then you should have put a test on it!" If we're trying to ensure the ABI doesn't break here, let's put a test on that ABI.

108–117

Murphy's Law says that somebody will ignore this comment and add a case somethingelse: on line 106, at which point the comment will be wrong because these cases will fallthrough to that.
I guess they'll fall through only when int128 is unsupported and so we shouldn't be hitting these cases anyway... but still, can you just make both cases explicitly do #else _LIBCPP_UNREACHABLE() #endif instead?

[Mordante]

Addresses review comments and fixes CI.

[Mordante]

libcxx/include/__format/format_arg.h

42–43

I don't want to remove the comment, but I like your idea of adding a test.

108–117

I was doubting between this solution and the one you suggested. This convinces me the other solution is better.

libcxx/include/__format/format_arg.h

 // -*- C++ -*-
 //===----------------------------------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 

 
 // TODO FMT Remove this once we require compilers with proper C++20 support.
 // If the compiler has no concepts support, the format header will be disabled.
 // Without concepts support enable_if needs to be used and that too much effort
 // to support compilers with partial C++20 support.
 #if !defined(_LIBCPP_HAS_NO_CONCEPTS)
 
 namespace __format {
-/** The type stored in @ref basic_format_arg. */
+/// The type stored in @ref basic_format_arg.
+///
+/// @note The 128-bit types are unconditionally in the list to avoid the values
+/// of the enums to depend on the availability of 128-bit integers.
 enum class _LIBCPP_ENUM_VIS __arg_t : uint8_t {
   __none,
   __boolean,
   __char_type,
   __int,
   __long_long,
-#ifndef _LIBCPP_HAS_NO_INT128
   __i128,
-#endif
   __unsigned,
   __unsigned_long_long,
-#ifndef _LIBCPP_HAS_NO_INT128
   __u128,
-#endif
   __float,
   __double,
   __long_double,
   __const_char_type_ptr,
   __string_view,
   __ptr
 };
 } // namespace __format
 
 template <class _Visitor, class _Context>
 _LIBCPP_HIDE_FROM_ABI _LIBCPP_AVAILABILITY_FORMAT decltype(auto)
 visit_format_arg(_Visitor&& __vis, basic_format_arg<_Context> __arg) {
   switch (__arg.__type_) {
   case __format::__arg_t::__none:
     return _VSTD::invoke(_VSTD::forward<_Visitor>(__vis), monostate{});
   case __format::__arg_t::__boolean:
     return _VSTD::invoke(_VSTD::forward<_Visitor>(__vis), __arg.__boolean);
   case __format::__arg_t::__char_type:
     return _VSTD::invoke(_VSTD::forward<_Visitor>(__vis), __arg.__char_type);
   case __format::__arg_t::__int:
     return _VSTD::invoke(_VSTD::forward<_Visitor>(__vis), __arg.__int);
   case __format::__arg_t::__long_long:
     return _VSTD::invoke(_VSTD::forward<_Visitor>(__vis), __arg.__long_long);
-#ifndef _LIBCPP_HAS_NO_INT128
   case __format::__arg_t::__i128:
+#ifndef _LIBCPP_HAS_NO_INT128
     return _VSTD::invoke(_VSTD::forward<_Visitor>(__vis), __arg.__i128);
+#else
+    _LIBCPP_UNREACHABLE();
 #endif
   case __format::__arg_t::__unsigned:
     return _VSTD::invoke(_VSTD::forward<_Visitor>(__vis), __arg.__unsigned);
   case __format::__arg_t::__unsigned_long_long:
     return _VSTD::invoke(_VSTD::forward<_Visitor>(__vis),
                          __arg.__unsigned_long_long);
-#ifndef _LIBCPP_HAS_NO_INT128
   case __format::__arg_t::__u128:
+#ifndef _LIBCPP_HAS_NO_INT128
     return _VSTD::invoke(_VSTD::forward<_Visitor>(__vis), __arg.__u128);
+#else
+   _LIBCPP_UNREACHABLE();
 #endif
   case __format::__arg_t::__float:
     return _VSTD::invoke(_VSTD::forward<_Visitor>(__vis), __arg.__float);
   case __format::__arg_t::__double:
     return _VSTD::invoke(_VSTD::forward<_Visitor>(__vis), __arg.__double);
   case __format::__arg_t::__long_double:
     return _VSTD::invoke(_VSTD::forward<_Visitor>(__vis), __arg.__long_double);
   case __format::__arg_t::__const_char_type_ptr:
     return _VSTD::invoke(_VSTD::forward<_Visitor>(__vis),
                          __arg.__const_char_type_ptr);
   case __format::__arg_t::__string_view:
     return _VSTD::invoke(_VSTD::forward<_Visitor>(__vis), __arg.__string_view);
   case __format::__arg_t::__ptr:
     return _VSTD::invoke(_VSTD::forward<_Visitor>(__vis), __arg.__ptr);
   }
   _LIBCPP_UNREACHABLE();
 }
 
 template <class _Context>
 class _LIBCPP_TEMPLATE_VIS _LIBCPP_AVAILABILITY_FORMAT basic_format_arg {
 public:
   // TODO FMT Define the handle class.
   class handle;
 
   _LIBCPP_HIDE_FROM_ABI basic_format_arg() noexcept
       : __type_{__format::__arg_t::__none} {}
 
   _LIBCPP_HIDE_FROM_ABI explicit operator bool() const noexcept {
     return __type_ != __format::__arg_t::__none;
   }
 
 private:
   using char_type = typename _Context::char_type;

libcxx/test/libcxx/utilities/format/format.arguments/format.arg/arg_t.compile.pass.cpp

+//===----------------------------------------------------------------------===//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+// UNSUPPORTED: c++03, c++11, c++14, c++17
+// UNSUPPORTED: libcpp-no-concepts
+// UNSUPPORTED: libcpp-has-no-incomplete-format
+
+// <format>
+
+// namespace __format { enum class __arg_t : uint8_t{...}; }
+
+#include <format>
+
+#include <type_traits>
+
+#include "test_macros.h"
+
+static_assert(std::is_same_v<std::underlying_type_t<std::__format::__arg_t>, uint8_t>);
+
+static_assert(uint8_t(std::__format::__arg_t::__none) == 0);
+static_assert(uint8_t(std::__format::__arg_t::__boolean) == 1);
+static_assert(uint8_t(std::__format::__arg_t::__char_type) == 2);
+static_assert(uint8_t(std::__format::__arg_t::__int) == 3);
+static_assert(uint8_t(std::__format::__arg_t::__long_long) == 4);
+static_assert(uint8_t(std::__format::__arg_t::__i128) == 5);
+static_assert(uint8_t(std::__format::__arg_t::__unsigned) == 6);
+static_assert(uint8_t(std::__format::__arg_t::__unsigned_long_long) == 7);
+static_assert(uint8_t(std::__format::__arg_t::__u128) == 8);
+static_assert(uint8_t(std::__format::__arg_t::__float) == 9);
+static_assert(uint8_t(std::__format::__arg_t::__double) == 10);
+static_assert(uint8_t(std::__format::__arg_t::__long_double) == 11);
+static_assert(uint8_t(std::__format::__arg_t::__const_char_type_ptr) == 12);
+static_assert(uint8_t(std::__format::__arg_t::__string_view) == 13);
+static_assert(uint8_t(std::__format::__arg_t::__ptr) == 14);