[PATCH] D55517: Remove `_VSTD`

JF Bastien via Phabricator reviews at reviews.llvm.org
Mon Dec 10 11:28:04 PST 2018


jfb added a comment.

In D55517#1325909 <https://reviews.llvm.org/D55517#1325909>, @EricWF wrote:

> In D55517#1325835 <https://reviews.llvm.org/D55517#1325835>, @jfb wrote:
>
> > In D55517#1325829 <https://reviews.llvm.org/D55517#1325829>, @ldionne wrote:
> >
> > > In D55517#1325586 <https://reviews.llvm.org/D55517#1325586>, @jfb wrote:
> > >
> > > > > Quick Question: What's the difference between writing `_VSTD::` and `std::`? Nothing.
> > > >
> > > > I thought it was `std:: _LIBCPP_ABI_NAMESPACE` ?
> > >
> > >
> > > That's correct, however I don't think it's a big deal. Indeed, discussed it and we couldn't find a reason why we would ever want to call a function that is not in the current ABI's namespace. And even if we did, we could always use `std::<ABI namespace>::function`.
> >
> >
> > Sure, my point is that the commit message isn't right, and should be updated to have this rationale.
>
>
> I think the commit message is correct. There is no functional difference, and the fact that `_VSTD` spells of `std::__1` is functionally a distinction without a difference.
>  I purposefully omitted any  mention of mixing ABI versions internally because the idea is non-functional. We only ever have one versioning namespace, what other version are we expecting to want to call?


I think your commit message is fun and terse, but it doesn't say why you're actually correct. You're explaining it here, Marshall has voiced concerns about downsides. I think your commit message should explain this and say why you think the downsides aren't relevant. That makes it easier to go back to your change in the future and understand why the change was OK without looking at this discussion.


Repository:
  rCXX libc++

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

https://reviews.llvm.org/D55517





More information about the libcxx-commits mailing list