[libcxx-commits] [PATCH] D56913: decoupling Freestanding atomic<T> from libatomic.a

[Louis Dionne via Phabricator via libcxx-commits]

ldionne added a comment.

In D56913#1417198 <https://reviews.llvm.org/D56913#1417198>, @davide wrote:

> In D56913#1417184 <https://reviews.llvm.org/D56913#1417184>, @__simt__ wrote:
>
> > In D56913#1417172 <https://reviews.llvm.org/D56913#1417172>, @davide wrote:
> >
> > > This commit broke the atomic lldb data formatter.
> > >
> > > http://lab.llvm.org:8080/green/view/LLDB/job/lldb-cmake/21037/
> > >
> > > @shafik @jingham
> >
> >
> > It looks like LLDB is naming internal names here. Unfortunately, it now needs to learn the new names. On the plus side, there is now a single stable internal name for all compilation paths for this header, whereas it used to differ. So the final result ought to be seen as an improvement.
>
>
> Nobody disagrees but the formatter has to be fixed, hence the two people cc:ed.


Sorry -- I should have run the tests for the LLDB formatters. This is not the first time it happens.

However, seeing how error-prone it is to change something in libc++ without running the LLDB tests, would it make sense to test the data formatters as part of libc++ itself? One way to see the data formatters is as something that libc++ provides to work nicely with LLDB instead of as something LLDB tries to provide for libc++. When seen that way, it makes more sense that the data formatters use private parts of libc++, and the tests should be part of libc++'s test suite. I don't know how big of a change this is, but I'd encourage LLDB folks to consider this as this would reduce a cause of semi-frequent mistakes.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D56913