[libcxx-commits] [PATCH] D117811: [libc++] Remove _VSTD

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

Quuxplusone requested changes to this revision.
Quuxplusone added inline comments.
This revision now requires changes to proceed.


================
Comment at: libcxx/include/__format/format_arg.h:141
       _LIBCPP_AVAILABILITY_FORMAT friend __format_arg_store<_Ctx, _Args...>
-      _VSTD::make_format_args(const _Args&...);
+      std::_LIBCPP_ABI_NAMESPACE::make_format_args(const _Args&...);
 
----------------
philnik wrote:
> Mordante wrote:
> > Why is this required? Is this the `Add gcc workarounds`? This deviates from the previous assertion that there was no difference between using `std::` and `std::__1::`. The requirement to sometimes use `std::_LIBCPP_ABI_NAMESPACE` instead of `std::` feels error prone to me.
> > 
> > 
> Yes, these are the GCC workarounds. I think this is a GCC bug, but nobody has confirmed it yet, so I didn't open an issue. It seems GCC doesn't handle friends properly with inline namespaces.
I think it should work to just remove `_VSTD::` from this line, because `make_format_args` is already in the same namespace as `basic_format_arg`, isn't it? (And ADL applies to //calls//, not //friend declarations//.) Thus:
```
  template <class _Ctx, class... _Args>
  _LIBCPP_HIDE_FROM_ABI _LIBCPP_AVAILABILITY_FORMAT
  friend __format_arg_store<_Ctx, _Args...> make_format_args(const _Args&...);
```
(Drive-by fixed indentation.) If this way works, which I believe it should, I'd very much prefer to do it (unless @Mordante is planning some future change that will explain why we //don't// want to do this even though it works).

Ditto below.

Also, if this works, please land this should-be-NFC change (and below) in their own separate commit. Let's have changing-the-`#define _VSTD` be a one-line commit, so it's really really trivial to revert if needed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117811