[libcxx-commits] [PATCH] D144279: [libc++][format] Fixes formatting vector<bool>

[Mark de Wever via Phabricator via libcxx-commits]

Mordante marked 3 inline comments as done.
Mordante added a comment.

Thanks for the reviews!



================
Comment at: libcxx/include/__bit_reference:158
 public:
+    using __container = typename _Cp::__self;
+
----------------
ldionne wrote:
> philnik wrote:
> > Mordante wrote:
> > > philnik wrote:
> > > > Why not just `_Cp`?
> > > To make sure this is used with a `std::vector`, similar to the friend at line 155.
> > How would it not be used with a `std::vector` or `std::bitset`? If a user tries to instantiate it with another class it's not exactly hard to add the alias.
> FWIW I think the `__self` thing is really weird, but I would support changing that in its own patch if we want to do something about it.
> How would it not be used with a `std::vector` or `std::bitset`? If a user tries to instantiate it with another class it's not exactly hard to add the alias.

Of course you can, but you're not allowed to since it uses the ugly name.

> FWIW I think the `__self` thing is really weird, but I would support changing that in its own patch if we want to do something about it.

I don't feel to strongly about it, so I will not pursue it. (I've more things to do, which I deem important.)



================
Comment at: libcxx/include/__bit_reference:158
 public:
+    using __container = typename _Cp::__self;
+
----------------
Mordante wrote:
> ldionne wrote:
> > philnik wrote:
> > > Mordante wrote:
> > > > philnik wrote:
> > > > > Why not just `_Cp`?
> > > > To make sure this is used with a `std::vector`, similar to the friend at line 155.
> > > How would it not be used with a `std::vector` or `std::bitset`? If a user tries to instantiate it with another class it's not exactly hard to add the alias.
> > FWIW I think the `__self` thing is really weird, but I would support changing that in its own patch if we want to do something about it.
> > How would it not be used with a `std::vector` or `std::bitset`? If a user tries to instantiate it with another class it's not exactly hard to add the alias.
> 
> Of course you can, but you're not allowed to since it uses the ugly name.
> 
> > FWIW I think the `__self` thing is really weird, but I would support changing that in its own patch if we want to do something about it.
> 
> I don't feel to strongly about it, so I will not pursue it. (I've more things to do, which I deem important.)
> 
> FWIW I think the `__self` thing is really weird, but I would support changing that in its own patch if we want to do something about it.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144279