[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.

104–113

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.

104–113

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

 
 // 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
#endif
   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;