[Lldb-commits] [PATCH] D101627: [lldb] More tests for DumpDataExtractor

shafik added a comment. +1 In D101627#2729594 , @jingham wrote: > I would resist this change. It's unnecessarily disruptive, would again break > git archeology, and really have no significant benefit. I also think the > lldb conventions for naming

[Lldb-commits] [PATCH] D101627: [lldb] More tests for DumpDataExtractor

teemperor added a comment. FWIW, I think the idea was for LLVM to actually move to LLDB's function naming, so our life will soon(TM) be easier when/if that happens. I also agree that the CapitalizedVariableNames in Clang have some difficulties (especially when they conflict with class names),

Re: [Lldb-commits] [PATCH] D101627: [lldb] More tests for DumpDataExtractor

I would resist this change. It's unnecessarily disruptive, would again break git archeology, and really have no significant benefit. I also think the lldb conventions for naming things are much clearer than the llvm ones. Knowing that something is a ivar by looking at the name is a real

[Lldb-commits] [PATCH] D101627: [lldb] More tests for DumpDataExtractor

teemperor added a comment. Jepp, they are capitalized :) FWIW, maybe we should see if we can fix this at the next US dev meeting (which hopefully happens). git has by now a filter for mass-refactors so the only problem is getting everyone to on board with breaking the whole code base and make

[Lldb-commits] [PATCH] D101627: [lldb] More tests for DumpDataExtractor

DavidSpickett added inline comments. Comment at: lldb/unittests/Core/DumpDataExtractorTest.cpp:40 +} + +static void testDumpWithOffset(offset_t start_offset, DavidSpickett wrote: > teemperor wrote: > > I think you missed the capitalisation here and in the

[Lldb-commits] [PATCH] D101627: [lldb] More tests for DumpDataExtractor

DavidSpickett added inline comments. Comment at: lldb/unittests/Core/DumpDataExtractorTest.cpp:40 +} + +static void testDumpWithOffset(offset_t start_offset, teemperor wrote: > I think you missed the capitalisation here and in the function above :) Doh. In fact

[Lldb-commits] [PATCH] D101627: [lldb] More tests for DumpDataExtractor

teemperor added inline comments. Comment at: lldb/unittests/Core/DumpDataExtractorTest.cpp:40 +} + +static void testDumpWithOffset(offset_t start_offset, I think you missed the capitalisation here and in the function above :) Repository: rG LLVM Github

[Lldb-commits] [PATCH] D101627: [lldb] More tests for DumpDataExtractor

teemperor added a comment. In D101627#2729303 , @DavidSpickett wrote: >> The_underscore_style_for_functions isn't LLDB (or LLVM) code style. I missed >> that in the first review. So just rename the utility functions to >> TestDumpMultiline and so on

[Lldb-commits] [PATCH] D101627: [lldb] More tests for DumpDataExtractor

DavidSpickett added a comment. > The_underscore_style_for_functions isn't LLDB (or LLVM) code style. I missed > that in the first review. So just rename the utility functions to > TestDumpMultiline and so on and this LGTM. I'm just going to accept this as > renaming the variables shouldn't

[Lldb-commits] [PATCH] D101627: [lldb] More tests for DumpDataExtractor

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGa86cbd475576: [lldb] More tests for DumpDataExtractor (authored by DavidSpickett). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[Lldb-commits] [PATCH] D101627: [lldb] More tests for DumpDataExtractor

DavidSpickett updated this revision to Diff 341916. DavidSpickett added a comment. Change function names to match coding style. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D101627/new/ https://reviews.llvm.org/D101627 Files:

[Lldb-commits] [PATCH] D101627: [lldb] More tests for DumpDataExtractor

teemperor accepted this revision. teemperor added a comment. This revision is now accepted and ready to land. The_underscore_style_for_functions isn't LLDB (or LLVM) code style. I missed that in the first review. So just rename the utility functions to `TestDumpMultiline` and so on and this

[Lldb-commits] [PATCH] D101627: [lldb] More tests for DumpDataExtractor

DavidSpickett created this revision. DavidSpickett requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. - Using a base address or skipping it with LLDB_INVALID_ADDRESS - Using a data offset, which does not effect the printed addresses - Not