teemperor requested changes to this revision.
teemperor added a comment.
This revision now requires changes to proceed.
Thanks for working on this!
Some notes/nits:
- nit: The patch seems to have a bunch of unrelated clang-format changes for
CPlusPlusLanguage.cpp in it. You can prevent that by using git clang-format on
your local git commit before putting the diff on Phabricator (git tells
clang-format which lines were modified by you and clang-format will only format
those).
- LibStdcppBitset.cpp seems to be a 99% identical copy of LibCxxBitset.cpp. I
think those two classes can just be one class with some variable for the
different field name in each implementation.
- Same for the test. I think we can merge that into something like
`.../data-formatter-stl/generic/bitset` and then just have two `test_...`
methods where one is decorated for libc++ and one for libstdc++. You can change
the Makefile values for each test with the `dictionary` argument of
`self.build` (something like `self.build(dictionary={"USE_LIBSTDCPP":"1"})`
should work I believe).
================
Comment at: lldb/source/Plugins/Language/CPlusPlus/CMakeLists.txt:9
LibCxxBitset.cpp
+ LibStdcppBitset.cpp
LibCxxInitializerList.cpp
----------------
I think the idea is to keep this sorted alphabetically (even though this
position probably make more sense, but oh well...)
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D112180/new/
https://reviews.llvm.org/D112180
_______________________________________________
lldb-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
