[Lldb-commits] [PATCH] D99484: Use `GNUInstallDirs` to support custom installation dirs.

Ericson2314 added a comment. This is now approved and passing CI, and I have also normalized the quoting @compnerd asked about (in I hope a satisfactory way). Should I be pinging someone to land it? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D99

[Lldb-commits] [PATCH] D99484: Use `GNUInstallDirs` to support custom installation dirs.

Ericson2314 added inline comments. Comment at: compiler-rt/cmake/base-config-ix.cmake:72 + set(COMPILER_RT_INSTALL_PATH "" CACHE PATH +"Prefix where built compiler-rt artifacts should be installed, comes before CMAKE_INSTALL_PREFIX.") option(COMPILER_RT_INCLUDE_TESTS "Ge

[Lldb-commits] [PATCH] D101462: Make it possible for targets to define their own MCObjectFileInfo

shafik added inline comments. Comment at: lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp:909 + new llvm::MCContext(llvm::Triple(triple), asm_info_up.get(), + reg_info_up.get(), nullptr, subtarget_info_up.get())); if (!context_up) --

[Lldb-commits] [PATCH] D101462: Make it possible for targets to define their own MCObjectFileInfo

shafik added inline comments. Comment at: clang/lib/Parse/ParseStmtAsm.cpp:589 + TheTarget->createMCObjectFileInfo( + /*PIC*/ false, Ctx)); + Ctx.setObjectFileInfo(MOFI.get()); `/*PIC=*/false` Comment at: llvm/tools/llvm-mca/llvm

[Lldb-commits] [PATCH] D101462: Make it possible for targets to define their own MCObjectFileInfo

flip1995 added a comment. > The refactoring adding `Triple` to `MCContext::MCContext` [...] should be > separate. In order to make the `MCContext` construction independent from the `MCObjectFileInfo`, passing the `Triple` to the `MCContext` is necessary anyway. Moving it completely to the `MCC

[Lldb-commits] [PATCH] D101237: [lldb] Fix [[no_unique_address]] and libstdc++ 11's std::unique_ptr

dblaikie added a comment. In D101237#2728726 , @teemperor wrote: >> (and it could tell clang exactly how large the structure is too - from the >> DWARF) > > We are actually doing that to my knowledge and return the `DW_AT_byte_size` > value for the reco

[Lldb-commits] [PATCH] D100965: [lldb] Refactor argument group by SourceLocationSpec (NFCI)

mib updated this revision to Diff 342079. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D100965/new/ https://reviews.llvm.org/D100965 Files: lldb/include/lldb/Breakpoint/BreakpointResolver.h lldb/include/lldb/Breakpoint/BreakpointResolverFileLine

[Lldb-commits] [PATCH] D100962: [lldb/Core] Add SourceLocationSpec class (NFC)

mib updated this revision to Diff 342073. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D100962/new/ https://reviews.llvm.org/D100962 Files: lldb/include/lldb/Core/SourceLocationSpec.h lldb/source/Core/CMakeLists.txt lldb/source/Core/SourceLoca

[Lldb-commits] [PATCH] D100962: [lldb/Core] Add SourceLocationSpec class (NFC)

mib updated this revision to Diff 342068. mib marked 2 inline comments as done. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D100962/new/ https://reviews.llvm.org/D100962 Files: lldb/include/lldb/Core/SourceLocationSpec.h lldb/source/Core/CMakeL

[Lldb-commits] [PATCH] D101221: [lldb/Symbol] Fix column breakpoint `move_to_nearest_code` match

JDevlieghere added inline comments. Comment at: lldb/include/lldb/Symbol/LineTable.h:344 + template + uint32_t FindLineEntryIndexByFileIndexImpl( + uint32_t start_idx, T file_idx, It seems like this method is only using `m_entries`, so I would turn this i

[Lldb-commits] [PATCH] D101655: [debugserver] Include LLDB_VERSION_SUFFIX in debugserver version

JDevlieghere added inline comments. Comment at: lldb/tools/debugserver/source/debugserver_vers.c.in:1 -const unsigned char debugserverVersionString[] __attribute__ ((used)) = "@(#)PROGRAM:LLDB PROJECT:lldb-@LLDB_VERSION_MAJOR@.@LLDB_VERSION_MINOR@.@LLDB_VERSION_PATCH@" "\n";

[Lldb-commits] [PATCH] D101655: [debugserver] Include LLDB_VERSION_SUFFIX in debugserver version

JDevlieghere created this revision. JDevlieghere added a reviewer: jasonmolenda. JDevlieghere requested review of this revision. https://reviews.llvm.org/D101655 Files: lldb/tools/debugserver/source/debugserver_vers.c.in Index: lldb/tools/debugserver/source/debugserver_vers.c.in =

[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 th

[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), b

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 times

[Lldb-commits] [lldb] 44d0ad5 - [lldb] Change DumpDataExtractorTest function names to lldb style (NFC)

Author: David Spickett Date: 2021-04-30T16:55:34+01:00 New Revision: 44d0ad53afbe06d1141b84321eaca234c60a1305 URL: https://github.com/llvm/llvm-project/commit/44d0ad53afbe06d1141b84321eaca234c60a1305 DIFF: https://github.com/llvm/llvm-project/commit/44d0ad53afbe06d1141b84321eaca234c60a1305.diff

[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] D101631: [lldb] DumpDataExtractor tests for item byte size errors

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG8da5d111a5d2: [lldb] DumpDataExtractor tests for item byte size errors (authored by DavidSpickett). Repository: rG LLVM Github Monorepo CHANGES S

[Lldb-commits] [lldb] 8da5d11 - [lldb] DumpDataExtractor tests for item byte size errors

Author: David Spickett Date: 2021-04-30T16:49:04+01:00 New Revision: 8da5d111a5d2f795d5c32ef194f2e93c8ac0ec3f URL: https://github.com/llvm/llvm-project/commit/8da5d111a5d2f795d5c32ef194f2e93c8ac0ec3f DIFF: https://github.com/llvm/llvm-project/commit/8da5d111a5d2f795d5c32ef194f2e93c8ac0ec3f.diff

[Lldb-commits] [PATCH] D101631: [lldb] DumpDataExtractor tests for item byte size errors

DavidSpickett updated this revision to Diff 341931. DavidSpickett added a comment. Update function names to lldb style Pascal case names. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D101631/new/ https://reviews.llvm.org/D101631 Files: lldb/unit

[Lldb-commits] [PATCH] D101585: [lldb] Ensure SBStructuredData::m_impl_up is always non-null

augusto2112 accepted this revision. augusto2112 added a comment. This revision is now accepted and ready to land. LGTM! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D101585/new/ https://reviews.llvm.org/D101585

[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 functio

[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 i

[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 Monore

[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 an

[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 need

[Lldb-commits] [lldb] a86cbd4 - [lldb] More tests for DumpDataExtractor

Author: David Spickett Date: 2021-04-30T16:16:38+01:00 New Revision: a86cbd475576b0c08537bea44c54cfc332f215f2 URL: https://github.com/llvm/llvm-project/commit/a86cbd475576b0c08537bea44c54cfc332f215f2 DIFF: https://github.com/llvm/llvm-project/commit/a86cbd475576b0c08537bea44c54cfc332f215f2.diff

[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/unittests/Core

[Lldb-commits] [PATCH] D101631: [lldb] DumpDataExtractor tests for item byte size errors

teemperor accepted this revision. teemperor added a comment. This revision is now accepted and ready to land. LGTM, thanks for writing these. Please check the code style remark in D101627 before merging this (you also need to change the functions here obviously

[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 LGTM

[Lldb-commits] [PATCH] D101631: [lldb] DumpDataExtractor tests for item byte size errors

DavidSpickett updated this revision to Diff 341896. DavidSpickett added a comment. Retrigger Buildkite after adding the parent revision. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D101631/new/ https://reviews.llvm.org/D101631 Files: lldb/unitt

[Lldb-commits] [PATCH] D101631: [lldb] DumpDataExtractor tests for item byte size errors

DavidSpickett created this revision. DavidSpickett requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D101631 Files: lldb/unittests/Core/DumpDataExtractorTest.cpp Index: lldb

[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 prov

[Lldb-commits] [PATCH] D101237: [lldb] Fix [[no_unique_address]] and libstdc++ 11's std::unique_ptr

teemperor added a comment. > (and it could tell clang exactly how large the structure is too - from the > DWARF) We are actually doing that to my knowledge and return the `DW_AT_byte_size` value for the record type. The relevant API that LLDB implements to get layout/size info back to Clang is

[Lldb-commits] [PATCH] D101153: [lldb][NFC] Specify guidelines for API tests

teemperor updated this revision to Diff 341832. teemperor added a comment. - Addressed Diana's comments (thanks!) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D101153/new/ https://reviews.llvm.org/D101153 Files: lldb/docs/resources/test.rst Index: lldb/docs/resources/test.rst ==

[Lldb-commits] [PATCH] D98653: [lldb] Refactor variable paths to support languages with non-pointer "this" (NFC)

teemperor requested changes to this revision. teemperor added a comment. This revision now requires changes to proceed. (Just pushing this back into Dave's review queue) Comment at: lldb/include/lldb/Symbol/CompilerDeclContext.h:77 /// in a struct, union or class. - boo

[Lldb-commits] [PATCH] D101250: Wrap edit line configuration calls into helper functions

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGfd89af6880f3: Wrap edit line configuration calls into helper functions (authored by nealsid, committed by teemperor). Repository: rG LLVM Github M

[Lldb-commits] [lldb] fd89af6 - Wrap edit line configuration calls into helper functions

Author: Neal (nealsid) Date: 2021-04-30T12:32:29+02:00 New Revision: fd89af6880f33ead708abe2f7d88ecb687d4e0d2 URL: https://github.com/llvm/llvm-project/commit/fd89af6880f33ead708abe2f7d88ecb687d4e0d2 DIFF: https://github.com/llvm/llvm-project/commit/fd89af6880f33ead708abe2f7d88ecb687d4e0d2.diff

[Lldb-commits] [PATCH] D101556: [lldb] Move and clean-up the Declaration class (NFC)

teemperor accepted this revision. teemperor added a comment. In D101556#2727603 , @mib wrote: > In D101556#2726434 , @teemperor > wrote: > >> IIRC they were #ifdef's out because of memory concerns. Did you have a

[Lldb-commits] [PATCH] D101585: [lldb] Ensure SBStructuredData::m_impl_up is always non-null

teemperor added a comment. LGTM. I'm going to be that guy and ask if you could test this (there is an API unittest directory with an example test, so copying that and just exercising the nullptr SBStructuredData is good enough). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[Lldb-commits] [PATCH] D101221: [lldb/Symbol] Fix column breakpoint `move_to_nearest_code` match

mib updated this revision to Diff 341813. mib retitled this revision from "[lldb/Symbol] Use column information for symbol resolution" to "[lldb/Symbol] Fix column breakpoint `move_to_nearest_code` match". mib edited the summary of this revision. mib added reviewers: aprantl, jingham. mib changed

[Lldb-commits] [PATCH] D101453: [lldb] Add tests for DumpDataExtractor formats

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

[Lldb-commits] [lldb] 8fdfc1d - [lldb] Add tests for DumpDataExtractor formats

Author: David Spickett Date: 2021-04-30T10:29:05+01:00 New Revision: 8fdfc1d64c51d949b84b2ee824f2ba9b9a3a6bd6 URL: https://github.com/llvm/llvm-project/commit/8fdfc1d64c51d949b84b2ee824f2ba9b9a3a6bd6 DIFF: https://github.com/llvm/llvm-project/commit/8fdfc1d64c51d949b84b2ee824f2ba9b9a3a6bd6.diff

[Lldb-commits] [PATCH] D101453: [lldb] Add tests for DumpDataExtractor formats

DavidSpickett updated this revision to Diff 341811. DavidSpickett added a comment. Address review comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D101453/new/ https://reviews.llvm.org/D101453 Files: lldb/unittests/Core/CMakeLists.txt ll

[Lldb-commits] [PATCH] D101539: Add null-pointer checks when accessing a TypeSystem's SymbolFile

teemperor added a comment. TypeSystemClang can be unit-tested just fine these days. It should also be possible to test Type (You can just model a random 'int' type from a TypeSystemClang instance). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D101

[Lldb-commits] [PATCH] D101539: Add null-pointer checks when accessing a TypeSystem's SymbolFile

DavidSpickett added a comment. General question, is it possible to write a test for this? (I've not looked into this area of code before) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D101539/new/ https://reviews.llvm.org/D101539 _