[Lldb-commits] [PATCH] D54682: [Driver] Extract option parsing and option processing.

JDevlieghere added a comment. In https://reviews.llvm.org/D54682#1302448, @zturner wrote: > Lib option definitely does, as its entire purpose is to be powerful and > flexible enough to mimic arbitrary command line tools. I created https://reviews.llvm.org/D54692 to show what the driver would l

[Lldb-commits] [PATCH] D54692: [Driver] Use libOption with tablegen.

JDevlieghere added a comment. Current output: https://reviews.llvm.org/P8117 Tablegen output: https://reviews.llvm.org/P8118 Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54692/new/ https://reviews.llvm.org/D54692

[Lldb-commits] [PATCH] D54682: [Driver] Extract option parsing and option processing.

JDevlieghere abandoned this revision. JDevlieghere added a comment. Thanks everyone! Looks like the general consensus is in favor of using libOption, so I'm abandoning this in favor of D54692 . Please continue the discussion there. Repository: rLLDB LLDB CHA

[Lldb-commits] [PATCH] D54616: [Reproducers] Improve reproducer API and add unit tests.

JDevlieghere updated this revision to Diff 175293. JDevlieghere marked 6 inline comments as done. JDevlieghere added a comment. Thanks Pavel, really useful feedback, I learned a few new things :-) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54616/new/ https://reviews.llvm.org/D54616

[Lldb-commits] [PATCH] D54692: [Driver] Use libOption with tablegen.

JDevlieghere updated this revision to Diff 175300. JDevlieghere added a comment. Add old usage. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54692/new/ https://reviews.llvm.org/D54692 Files: tools/driver/CMakeLists.txt tools/driver/Driver.cpp tools/driver/Driver.h tools/driver

[Lldb-commits] [PATCH] D54692: [Driver] Use libOption with tablegen.

JDevlieghere updated this revision to Diff 175319. JDevlieghere added a comment. - Add grouping - Use basename for help CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54692/new/ https://reviews.llvm.org/D54692 Files: tools/driver/CMakeLists.txt tools/driver/Driver.cpp tools/driver

[Lldb-commits] [PATCH] D54692: [Driver] Use libOption with tablegen.

JDevlieghere added a comment. In D54692#1308604 , @clayborg wrote: > Can you attach new output with the grouping and extra usage? I updated the original snippet, see https://reviews.llvm.org/P8118 CHANGES SINCE LAST ACTION https://reviews.llvm.org/D5

[Lldb-commits] [PATCH] D54692: [Driver] Use libOption with tablegen.

JDevlieghere added a comment. In D54692#1308640 , @clayborg wrote: > I would wait to get another OK from anyone else just to be sure. Sounds good. Thanks Greg! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54692/new/ https://reviews.llvm.org/D

[Lldb-commits] [PATCH] D54914: Add a generic build script for building test inferiors

JDevlieghere added a comment. I personally prefer this approach, if only for the fact that the substitutions are more familiar and that this change would be local to lldb. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54914/new/ https://reviews.llvm.org/D54914 __

[Lldb-commits] [PATCH] D54692: [Driver] Use libOption with tablegen.

JDevlieghere updated this revision to Diff 175377. JDevlieghere added a comment. Will land this tomorrow once we've figured out how to integrate tablegen with the Xcode project. In the meantime I found some issues due to ordering of command options. Their relative order matters and we had tests

[Lldb-commits] [PATCH] D54692: [Driver] Use libOption with tablegen.

JDevlieghere updated this revision to Diff 175505. JDevlieghere edited the summary of this revision. JDevlieghere added a comment. - Add lit test for command options. - Change usage with prose. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54692/new/ https://reviews.llvm.org/D54692 Fil

[Lldb-commits] [PATCH] D54692: [Driver] Use libOption with tablegen.

JDevlieghere updated this revision to Diff 175526. JDevlieghere added a comment. - Add EXAMPLES section CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54692/new/ https://reviews.llvm.org/D54692 Files: lit/Driver/Inputs/Print0.in lit/Driver/Inputs/Print2.in lit/Driver/Inputs/Print4

[Lldb-commits] [PATCH] D54616: [Reproducers] Improve reproducer API and add unit tests.

JDevlieghere added a comment. Does anybody want to have another look or can I land this? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54616/new/ https://reviews.llvm.org/D54616 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http

[Lldb-commits] [PATCH] D54692: [Driver] Use libOption with tablegen.

This revision was automatically updated to reflect the committed changes. Closed by commit rL347709: [Driver] Use libOption with tablegen. (authored by JDevlieghere, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D54692?vs=175526&id=17

[Lldb-commits] [PATCH] D54616: [Reproducers] Improve reproducer API and add unit tests.

This revision was automatically updated to reflect the committed changes. Closed by commit rL347716: [Reproducers] Improve reproducer API and add unit tests. (authored by JDevlieghere, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D54

[Lldb-commits] [PATCH] D55013: [CMake] Streamline code signing for debugserver #2

JDevlieghere added inline comments. Comment at: tools/debugserver/CMakeLists.txt:18 + + option(LLDB_USE_ENTITLEMENTS "When code signing, use entitlements if available" ON) + set(LLDB_CODESIGN_IDENTITY lldb_codesign CACHE STRING Why do we need to define this op

[Lldb-commits] [PATCH] D55013: [CMake] Streamline code signing for debugserver #2

JDevlieghere added inline comments. Comment at: tools/debugserver/CMakeLists.txt:18 + + option(LLDB_USE_ENTITLEMENTS "When code signing, use entitlements if available" ON) + set(LLDB_CODESIGN_IDENTITY lldb_codesign CACHE STRING sgraenitz wrote: > JDevlieghere

[Lldb-commits] [PATCH] D55038: [Reproducers] Change how reproducers are initialized.

JDevlieghere created this revision. JDevlieghere added reviewers: jingham, aprantl, sgraenitz, labath. JDevlieghere added a project: LLDB. Herald added subscribers: teemperor, abidh, mgorny. This patch changes the way the reproducer is initialized. Rather than making changes at run time we now do

[Lldb-commits] [PATCH] D55038: [Reproducers] Change how reproducers are initialized.

JDevlieghere updated this revision to Diff 175993. JDevlieghere marked 12 inline comments as done. JDevlieghere added a comment. Address feedback from Pavel. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55038/new/ https://reviews.llvm.org/D55038 Files: include/lldb/API/SBDebugger.h

[Lldb-commits] [PATCH] D55038: [Reproducers] Change how reproducers are initialized.

JDevlieghere added inline comments. Comment at: source/Utility/Reproducer.cpp:41-42 +Reproducer::Reproducer(ReproducerMode mode, llvm::Optional root) { + // It's unfortunate that we have to do so much I/O here that can fail. The + // best we can do is not initialize the reprodu

[Lldb-commits] [PATCH] D55038: [Reproducers] Change how reproducers are initialized.

JDevlieghere updated this revision to Diff 176034. JDevlieghere added a comment. Test didn't run. Is there a way to REQUIRE either darwin or linux? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55038/new/ https://reviews.llvm.org/D55038 Files: include/lldb/API/SBDebugger.h include/

[Lldb-commits] [PATCH] D54617: [wip][Reproducers] Add file provider

JDevlieghere updated this revision to Diff 176035. JDevlieghere added a comment. - Get the external path with modifying the VFS in LLVM. - Integrate with the new integration logic. - Add a test. (that doesn't work yet, still WIP) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54617/new/

[Lldb-commits] [PATCH] D55128: [CMake] Store path to vendor-specific headers in clang-headers target property

JDevlieghere accepted this revision. JDevlieghere added a comment. This revision is now accepted and ready to land. This makes sense to me. I'm don't know if there's a better property but I think this matches the intended use, so I think it is fine. Repository: rC Clang CHANGES SINCE LAST AC

[Lldb-commits] [PATCH] D55038: [Reproducers] Change how reproducers are initialized.

JDevlieghere updated this revision to Diff 176161. JDevlieghere marked an inline comment as done. JDevlieghere added a comment. Make initialize return an error. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55038/new/ https://reviews.llvm.org/D55038 Files: include/lldb/API/SBDebugger

[Lldb-commits] [PATCH] D55038: [Reproducers] Change how reproducers are initialized.

JDevlieghere added a comment. In D55038#1314336 , @labath wrote: > In D55038#1314026 , @JDevlieghere > wrote: > > > Test didn't run. Is there a way to REQUIRE either darwin or linux? > > > I think the canonical way

[Lldb-commits] [PATCH] D55038: [Reproducers] Change how reproducers are initialized.

This revision was automatically updated to reflect the committed changes. Closed by commit rL348152: [Reproducers] Change how reproducers are initialized. (authored by JDevlieghere, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D55038

[Lldb-commits] [PATCH] D55038: [Reproducers] Change how reproducers are initialized.

JDevlieghere added a comment. In D55038#1317099 , @stella.stamenova wrote: > One of these tests fail on Windows (TestDriverOptions): > > http://lab.llvm.org:8014/builders/lldb-x64-windows-ninja/builds/1848/steps/test/logs/stdio Thanks, I've disabled the

[Lldb-commits] [PATCH] D55240: [FileSystem] Migrate CommandCompletions

JDevlieghere created this revision. JDevlieghere added reviewers: davide, labath. JDevlieghere added a project: LLDB. Make use of the convenience helpers from FileSystem. Repository: rLLDB LLDB https://reviews.llvm.org/D55240 Files: include/lldb/Host/FileSystem.h source/Commands/CommandC

[Lldb-commits] [PATCH] D55240: [FileSystem] Migrate CommandCompletions

JDevlieghere updated this revision to Diff 176525. JDevlieghere added a comment. Initialize the FS in the unit test. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55240/new/ https://reviews.llvm.org/D55240 Files: include/lldb/Host/FileSystem.h source/Commands/CommandCompletions.cpp

[Lldb-commits] [PATCH] D55240: [FileSystem] Migrate CommandCompletions

JDevlieghere marked an inline comment as done. JDevlieghere added inline comments. Comment at: source/Commands/CommandCompletions.cpp:180 + FileSystem fs = FileSystem::Instance(); std::error_code EC; labath wrote: > This should surely be a reference, no? Ma

[Lldb-commits] [PATCH] D55240: [FileSystem] Migrate CommandCompletions

This revision was automatically updated to reflect the committed changes. Closed by commit rL348287: [FileSystem] Migrate CommandCompletions (authored by JDevlieghere, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D55240?vs=176525&id=

[Lldb-commits] [PATCH] D55013: [CMake] Streamline code signing for debugserver #2

JDevlieghere accepted this revision. JDevlieghere added a comment. This revision is now accepted and ready to land. LGTM. Thanks! Comment at: tools/debugserver/CMakeLists.txt:18 + + option(LLDB_USE_ENTITLEMENTS "When code signing, use entitlements if available" ON) + set(LLD

[Lldb-commits] [PATCH] D55376: Generate LLDB website/documentation from rst with Sphinx

JDevlieghere created this revision. JDevlieghere added a reviewer: LLDB. Herald added subscribers: teemperor, arphaman, mgorny. The current LLDB website is written in HTML which is hard to maintain. We have quite a bit of HTML code checked in which can make it hard to differentiate between docum

[Lldb-commits] [PATCH] D55332: [CMake] Python bindings generation polishing

JDevlieghere added a comment. This LGTM if it works on Windows. Comment at: CMakeLists.txt:134 --srcRoot=${LLDB_SOURCE_DIR} - --targetDir=${LLDB_PYTHON_TARGET_DIR} - --cfgBldDir=${LLDB_PYTHON_TARGET_DIR} + --targetDir=$

[Lldb-commits] [PATCH] D55330: [CMake] Revised RPATH handling

JDevlieghere accepted this revision. JDevlieghere added a comment. This revision is now accepted and ready to land. LGTM with the question answered/addressed. Thanks Stefan, these patches are really great work! Comment at: tools/driver/CMakeLists.txt:27 +if(LLDB_BUILD_FRAMEWOR

[Lldb-commits] [PATCH] D54617: [wip][Reproducers] Add file provider

JDevlieghere updated this revision to Diff 177598. JDevlieghere added a comment. - Rebase - Copy over permissions - Fix path when launching. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54617/new/ https://reviews.llvm.org/D54617 Files: include/lldb/Host/FileSystem.h include/lldb/U

[Lldb-commits] [PATCH] D55574: Remove else statements after returns

JDevlieghere created this revision. JDevlieghere added a reviewer: LLDB. Herald added subscribers: jsji, teemperor, abidh, arphaman, atanasyan, kbarton, arichardson, javed.absar, ki.stfu, nemanjai, kubamracek, sdardis, emaste, srhines. Herald added a reviewer: espindola. Herald added a reviewer:

[Lldb-commits] [PATCH] D55582: [Reproducers] Add command reproducer

JDevlieghere created this revision. JDevlieghere added reviewers: labath, jingham, sgraenitz. JDevlieghere added a project: LLDB. Herald added a subscriber: teemperor. Add support for reproducing to the command interpreter. In capture mode we capture all the command to file and in reproducer mode

[Lldb-commits] [PATCH] D55574: Remove else statements after returns

JDevlieghere abandoned this revision. JDevlieghere added a comment. I'm going to abandon this for now until we have better tooling to address Jim's concerns here. I'm also not super happy with the formatting, which seems to be off in quite a few locations. The problem is that clang-tidy knows to

[Lldb-commits] [PATCH] D55584: [LLDB] Simplify Boolean expressions

JDevlieghere added a comment. In D55584#1328087 , @labath wrote: > I also find the `static_cast` thingy weird. The rest of the changes > seem to be towards the better (based on a pseudo-random sample), but the > change is a quite big. Me and Adrian wen

[Lldb-commits] [PATCH] D55582: [Reproducers] Add command reproducer

JDevlieghere updated this revision to Diff 177878. JDevlieghere added a comment. Address Pavel's comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55582/new/ https://reviews.llvm.org/D55582 Files: include/lldb/Core/Debugger.h include/lldb/Interpreter/CommandInterpreter.h li

[Lldb-commits] [PATCH] D55582: [Reproducers] Add command reproducer

JDevlieghere updated this revision to Diff 177906. JDevlieghere added a comment. Skip driver logic when replaying, otherwise commands are executed or sourced twice, once in the driver and once by replaying the commands. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55582/new/ https://r

[Lldb-commits] [PATCH] D55582: [Reproducers] Add command reproducer

JDevlieghere updated this revision to Diff 177950. JDevlieghere added a comment. When sourcing a file we should also ignore the individual commands otherwise they end up getting executed twice, once as part of the `command source` and once for every individual command in the file. CHANGES SINC

[Lldb-commits] [PATCH] D55626: [Reproducers] Add tests for functionality

JDevlieghere created this revision. JDevlieghere added reviewers: labath, davide, aprantl, zturner. JDevlieghere added a project: LLDB. Herald added a subscriber: teemperor. This patch adds test that check that functionality in lldb continues to work when replaying a reproducer. Currently the pa

[Lldb-commits] [PATCH] D55626: [Reproducers] Add tests for functionality

JDevlieghere updated this revision to Diff 178075. JDevlieghere marked an inline comment as done. JDevlieghere added a comment. Address Adrian's feedback CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55626/new/ https://reviews.llvm.org/D55626 Files: lit/Reproducer/Functionalities/Inp

[Lldb-commits] [PATCH] D55626: [Reproducers] Add tests for functionality

JDevlieghere marked 4 inline comments as done. JDevlieghere added inline comments. Comment at: lit/Reproducer/Functionalities/Inputs/stepping.c:8 +// +//===--===// +#include aprantl wrote: > Why

[Lldb-commits] [PATCH] D55582: [Reproducers] Add command reproducer

JDevlieghere added a comment. In D55582#1329479 , @labath wrote: > I am very sorry about this, but I am having some second thoughts about the > `RunReplay` thingy. I don't think I've correctly expressed what is bothering > me about it (and that's probabl

[Lldb-commits] [PATCH] D55328: [CMake] Revised LLDB.framework builds

JDevlieghere accepted this revision. JDevlieghere added a comment. Sorry for the late review, I had a look originally but didn't have any comments. LGTM! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55328/new/ https://reviews.llvm.org/D55328

[Lldb-commits] [PATCH] D55582: [Reproducers] Add command reproducer

JDevlieghere added a comment. I played around with this today: - I'm totally convinced that we should replay by swapping out stdin with a file from the reproducer. This makes this better overall. - I'm not convinced that capturing just stdin is an improvement. What I like about the current appr

[Lldb-commits] [PATCH] D55582: [Reproducers] Add command reproducer

JDevlieghere added a comment. In D55582#1331145 , @labath wrote: > Hm.. so the way, I'd imagine this working is that `-o` and friends (as well > as pretty much every other command line argument) would be captured by the SB > recorder, once they cross the

[Lldb-commits] [PATCH] D55776: Fix lldb's macosx/heap.py cstr command

JDevlieghere accepted this revision. JDevlieghere added a comment. This revision is now accepted and ready to land. LGTM. Thanks Davide! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55776/new/ https://reviews.llvm.org/D55776 ___ lldb-commi

[Lldb-commits] [PATCH] D55835: [dotest] Consider unexpected passes as failures.

JDevlieghere created this revision. JDevlieghere added reviewers: labath, davide, jingham, clayborg. JDevlieghere added a project: LLDB. Unexpected successes should be considered failures because they can hide regressions when not addressed. When a test is fixed and not re-enabled, it can easily

[Lldb-commits] [PATCH] D55837: [cmake] Make libcxx(abi) a dependency when building in-tree clang

JDevlieghere created this revision. JDevlieghere added reviewers: clayborg, sgraenitz, aprantl. JDevlieghere added a project: LLDB. Herald added a subscriber: mgorny. Herald added a reviewer: EricWF. As discussed on IRC this morning, when building an in-tree clang for testing we have to have libc

[Lldb-commits] [PATCH] D55837: [cmake] Make libcxx(abi) a dependency when building in-tree clang

JDevlieghere added a comment. In D55837#1334872 , @mgorny wrote: > How does this affect platforms that use other ABI library than libcxxabi? How > does this affect platforms that use libstdc++ as default C++ backend? I'm not familiar with that use case

[Lldb-commits] [PATCH] D55837: [cmake] Make libcxx(abi) a dependency when building in-tree clang for macOS

JDevlieghere added a comment. In D55837#1334893 , @labath wrote: > On linux libc++ is not a hard dependency for running tests (in fact, clang > will not use it even if it is checked out). I guess that has something to do > with the fact that libc++ is n

[Lldb-commits] [PATCH] D55837: [cmake] Make libcxx(abi) a dependency when building in-tree clang for macOS

JDevlieghere updated this revision to Diff 178727. JDevlieghere retitled this revision from "[cmake] Make libcxx(abi) a dependency when building in-tree clang" to "[cmake] Make libcxx(abi) a dependency when building in-tree clang for macOS". JDevlieghere added a comment. Only do this for macOS

[Lldb-commits] [PATCH] D55837: [cmake] Make libcxx(abi) a dependency when building in-tree clang for macOS

This revision was automatically updated to reflect the committed changes. Closed by commit rLLDB349539: [cmake] Make libcxx(abi) a dependency when building in-tree clang for macOS (authored by JDevlieghere, committed by ). Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm

[Lldb-commits] [PATCH] D55837: [cmake] Make libcxx(abi) a dependency when building in-tree clang for macOS

JDevlieghere marked 2 inline comments as done. JDevlieghere added inline comments. Comment at: CMakeLists.txt:160 list(APPEND LLDB_TEST_DEPS clang) +if(APPLE) + list(APPEND LLDB_TEST_DEPS cxx) clayborg wrote: > What if the user wants to use a differ

[Lldb-commits] [PATCH] D55827: Update current working directory to avoid test crashes

JDevlieghere added a comment. In D55827#1334671 , @labath wrote: > Hm... this seems like an important issue in the `RealFileSystem` (or our > usage of it), and I'm not sure it should be papered over like that. I mean, > lldb is a library, and requiring e

[Lldb-commits] [PATCH] D55835: [dotest] Consider unexpected passes as failures.

This revision was automatically updated to reflect the committed changes. Closed by commit rL349818: [dotest] Consider unexpected passes as failures. (authored by JDevlieghere, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D55835?vs=1

[Lldb-commits] [PATCH] D56129: Simplify ObjectFile::GetArchitecture

JDevlieghere accepted this revision. JDevlieghere added a comment. This revision is now accepted and ready to land. Maybe add a unit test for the operator bool? Otherwise this LGTM. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56129/new/ https://reviews.llvm.org/D56129 _

[Lldb-commits] [PATCH] D56208: Add file-based synchronization to flaky test

JDevlieghere accepted this revision. JDevlieghere added a comment. This revision is now accepted and ready to land. Thanks Adrian, looks good. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56208/new/ https://reviews.llvm.org/D56208 ___ lldb

[Lldb-commits] [PATCH] D56400: [CMake] Some cleanup around test preparations

JDevlieghere added inline comments. Comment at: CMakeLists.txt:43 if(LLDB_INCLUDE_TESTS) + # FIXME: LLDB_TEST_CXX_COMPILER is unused. + # FIXME: In standalone builds LLVM_BINARY_DIR points to LLDB's build directory and not to LLVM's! Why is this unused? Are w

[Lldb-commits] [PATCH] D56425: [BreakpointList] Simplify/modernize BreakpointList (NFC)

JDevlieghere created this revision. JDevlieghere added a reviewer: davide. I was looking at the code in BreakpointList.cpp and found it deserved a quick cleanup. - Extract duplicate code - Use range-based for loop - Use early return in loops Repository: rLLDB LLDB https://reviews.llvm.org/D

[Lldb-commits] [PATCH] D56425: [BreakpointList] Simplify/modernize BreakpointList (NFC)

JDevlieghere added a comment. I wonder if we should use a vector instead of a list here too. For small data sets a std::vector usually outperforms a std::list. I *think* the data-locality would outweigh the cost of copying the shared pointers. Repository: rLLDB LLDB CHANGES SINCE LAST ACTIO

[Lldb-commits] [PATCH] D56440: [CMake] Phase out LLDB_TEST_C/CXX_COMPILER in favor of single LLDB_TEST_COMPILER

JDevlieghere added a comment. It looks like `LLDB_TEST_COMPILER_IS_DEFAULT` is set but never read. Why do we need it exactly? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56440/new/ https://reviews.llvm.org/D56440 ___ lldb-commits mailing

[Lldb-commits] [PATCH] D56425: [BreakpointList] Simplify/modernize BreakpointList (NFC)

JDevlieghere updated this revision to Diff 180728. JDevlieghere added a comment. - Use std::vector - Code review feedback - Remove code duplication when returning a const by value. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56425/new/ https://reviews.llvm.org/D56425 Files: include

[Lldb-commits] [PATCH] D56425: [BreakpointList] Simplify/modernize BreakpointList (NFC)

JDevlieghere marked 10 inline comments as done. JDevlieghere added inline comments. Comment at: source/Breakpoint/BreakpointList.cpp:200 size_t curr_i = 0; - for (pos = m_breakpoints.begin(), curr_i = 0; pos != end; ++pos, ++curr_i) { + for (const auto &bp_sp : m_breakpoints

[Lldb-commits] [PATCH] D56425: [BreakpointList] Simplify/modernize BreakpointList (NFC)

JDevlieghere marked an inline comment as done. JDevlieghere added a comment. In D56425#1350234 , @clayborg wrote: > If we keep the list sorted we might be able to improve finding breakpoints by > ID, but that can be done if we need to. BreakpointList::Add

[Lldb-commits] [PATCH] D56425: [BreakpointList] Simplify/modernize BreakpointList (NFC)

This revision was automatically updated to reflect the committed changes. Closed by commit rLLDB350659: [BreakpointList] Simplify/modernize BreakpointList (NFC) (authored by JDevlieghere, committed by ). Changed prior to commit: https://reviews.llvm.org/D56425?vs=180728&id=180735#toc Repositor

[Lldb-commits] [PATCH] D56425: [BreakpointList] Simplify/modernize BreakpointList (NFC)

JDevlieghere added a comment. In D56425#1350236 , @JDevlieghere wrote: > In D56425#1350234 , @clayborg wrote: > > > If we keep the list sorted we might be able to improve finding breakpoints > > by ID, but that ca

[Lldb-commits] [PATCH] D46888: [DWARF] Have HashedNameToDIE store a DataExtractor by value

JDevlieghere accepted this revision. JDevlieghere added a comment. This revision is now accepted and ready to land. LGTM https://reviews.llvm.org/D46888 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/

[Lldb-commits] [PATCH] D46889: [DWARF] Extract indexing code into a separate class hierarchy

JDevlieghere added a comment. Looks good to me, but I'll defer to Greg as I've never actually touched this code myself. https://reviews.llvm.org/D46889 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/

[Lldb-commits] [PATCH] D47014: Fix _NSCFBoolean data formatter.

JDevlieghere created this revision. JDevlieghere added reviewers: jingham, clayborg, davide. In r265181 the test for the NSCFBoolean data formatter was removed. Later, in r279353 and r279446 a new implementation was provided for the formatter, which I believe never worked (and this wasn't caught b

[Lldb-commits] [PATCH] D47014: Fix _NSCFBoolean data formatter.

This revision was automatically updated to reflect the committed changes. Closed by commit rL332700: Fix _NSCFBoolean data formatter. (authored by JDevlieghere, committed by ). Changed prior to commit: https://reviews.llvm.org/D47014?vs=147312&id=147449#toc Repository: rL LLVM https://revie

[Lldb-commits] [PATCH] D47062: Suggest lldb-dotest to reproduce a failure.

JDevlieghere created this revision. JDevlieghere added reviewers: labath, davide, aprantl. Rather than trying to figure out what arguments to pass to `dotest.py`, suggest using `lldb-dotest`. Athough this obviously won't work for all invocations (i.e. when you're passing custom arguments to `lld

[Lldb-commits] [PATCH] D47064: Add some apple-tables lookup tests

JDevlieghere accepted this revision. JDevlieghere added a comment. This revision is now accepted and ready to land. LGTM! Comment at: lit/SymbolFile/DWARF/find-basic-function.cpp:17 // RUN: FileCheck --check-prefix=EMPTY %s +// +// RUN: clang %s -g -c -o %t --target=x86_64-a

[Lldb-commits] [PATCH] D47062: Suggest lldb-dotest to reproduce a failure.

JDevlieghere marked an inline comment as done. JDevlieghere added a comment. In https://reviews.llvm.org/D47062#1104558, @davide wrote: > Can you commit the whitespace fixes separately? Sure. Comment at: packages/Python/lldbsuite/test/dosep.py:122 print("[%s FAI

[Lldb-commits] [PATCH] D47062: Suggest lldb-dotest to reproduce a failure.

JDevlieghere updated this revision to Diff 147711. JDevlieghere marked an inline comment as done. JDevlieghere added a comment. - Use absolute path to `lldb-dotest` when `--executable` is passed. https://reviews.llvm.org/D47062 Files: packages/Python/lldbsuite/test/dosep.py packages/Python/

[Lldb-commits] [PATCH] D47133: Enable ProcessMachCore plugin on non-apple platforms

JDevlieghere accepted this revision. JDevlieghere added a comment. This revision is now accepted and ready to land. LGTM https://reviews.llvm.org/D47133 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/

[Lldb-commits] [PATCH] D47062: Suggest lldb-dotest to reproduce a failure.

JDevlieghere updated this revision to Diff 147787. JDevlieghere added a comment. Herald added a subscriber: mgorny. Make this work for Windows https://reviews.llvm.org/D47062 Files: packages/Python/lldbsuite/test/dosep.py packages/Python/lldbsuite/test/dotest.py packages/Python/lldbsuite/

[Lldb-commits] [PATCH] D46005: [test] Add a utility for dumping all tests in the dotest suite

JDevlieghere added a comment. I've given this some more though as I see valid points and concerns on both sides of the argument. I'm leaning towards running test cases in parallel because it offloads more work to lit at negligible cost (running make and launching the inferior are the biggest ti

[Lldb-commits] [PATCH] D47062: Suggest lldb-dotest to reproduce a failure.

JDevlieghere added a comment. I'm going to hold off on this until we have decided what to do for https://reviews.llvm.org/D46005. If we run the test cases as separate lit invocations, then the test format is in a better position to report this information. https://reviews.llvm.org/D47062 _

[Lldb-commits] [PATCH] D46005: [test] Add a utility for dumping all tests in the dotest suite

JDevlieghere added a comment. In https://reviews.llvm.org/D46005#1109693, @zturner wrote: > FWIW, I think the single biggest improvement one could make to the LLDB test > suite runtime is to compile all inferiors up front as part of the CMake step. > If you run the test suite twice every infer

[Lldb-commits] [PATCH] D47470: AppleDWARFIndex: Get function method-ness directly from debug info

JDevlieghere accepted this revision. JDevlieghere added a comment. This revision is now accepted and ready to land. Thanks Pavel, LGTM. https://reviews.llvm.org/D47470 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/c

[Lldb-commits] [PATCH] D47495: Support relative paths with less than two components in DBGSourcePathRemapping

JDevlieghere created this revision. JDevlieghere added reviewers: jasonmolenda, clayborg, labath. When reading DBGSourcePathRemapping from a dSYM, we remove the last two path components to make the source lookup more general. However, when dealing with a relative path that has less than 2 compon

[Lldb-commits] [PATCH] D47495: Support relative paths with less than two components in DBGSourcePathRemapping

JDevlieghere added inline comments. Comment at: include/lldb/Utility/FileSpec.h:535 - void RemoveLastPathComponent(); + void RemoveLastPathComponent(bool keep_dot = false); clayborg wrote: > Why add this? If the path is just "." to begin with, there is noth

[Lldb-commits] [PATCH] D47495: Support relative paths with less than two components in DBGSourcePathRemapping

JDevlieghere updated this revision to Diff 148986. JDevlieghere edited the summary of this revision. JDevlieghere added a comment. - Address Greg's feedback https://reviews.llvm.org/D47495 Files: source/Utility/FileSpec.cpp Index: source/Utility/FileSpec.cpp

[Lldb-commits] [PATCH] D47495: Support relative paths with less than two components in DBGSourcePathRemapping

JDevlieghere updated this revision to Diff 149083. JDevlieghere marked an inline comment as done. JDevlieghere added a comment. - Replace custom logic with LLVM's path logic. - Add tests. https://reviews.llvm.org/D47495 Files: include/lldb/Utility/FileSpec.h source/Utility/FileSpec.cpp un

[Lldb-commits] [PATCH] D47495: [FileSpec] Re-implmenet removeLastPathComponent

JDevlieghere added inline comments. Comment at: unittests/Utility/FileSpecTest.cpp:342 + EXPECT_FALSE(fs_posix_relative.RemoveLastPathComponent()); + EXPECT_STREQ("foo", fs_posix_relative.GetCString()); + labath wrote: > Is this the behavior you want here? I wa

[Lldb-commits] [PATCH] D47495: [FileSpec] Re-implmenet removeLastPathComponent

This revision was automatically updated to reflect the committed changes. Closed by commit rL333540: [FileSpec] Re-implmenet removeLastPathComponent (authored by JDevlieghere, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D47495?vs=14

[Lldb-commits] [PATCH] D47535: [FileSpec] Add support for lambdas to EnumerateDirectory. NFC

JDevlieghere created this revision. JDevlieghere added reviewers: clayborg, labath. Herald added a subscriber: llvm-commits. Support both lambda's and function pointers as arguments to EnumerateDirectory. Repository: rL LLVM https://reviews.llvm.org/D47535 Files: include/lldb/Utility/FileS

[Lldb-commits] [PATCH] D47535: [FileSpec] Add support for lambdas to EnumerateDirectory. NFC

JDevlieghere added a comment. In https://reviews.llvm.org/D47535#1116274, @labath wrote: > Could we just get rid of the baton version? It's the only way the function is used currently. How about just phasing it out when we touch the relevant code? Repository: rL LLVM https://reviews.llvm.

[Lldb-commits] [PATCH] D47539: [Platform] Accept arbitrary kext variants

JDevlieghere created this revision. JDevlieghere added reviewers: jasonmolenda, labath. When loading kexts in `PlatformDarwinKernel`, we use the BundleID as the filename to to create shared modules. In `GetSharedModule` we call `ExamineKextForMatchingUUID` for any BundleID it finds that is a mat

[Lldb-commits] [PATCH] D47535: [FileSpec] Add support for lambdas to EnumerateDirectory. NFC

JDevlieghere added a comment. In https://reviews.llvm.org/D47535#1116364, @labath wrote: > Actually, I wonder if we shouldn't just deprecate this function altogether. > What was your motivation for this patch? It seems we already have > `llvm::fs::(recursive_)directory_iterator` for this purpos

[Lldb-commits] [PATCH] D47535: [FileSpec] Add support for lambdas to EnumerateDirectory. NFC

JDevlieghere added a comment. In https://reviews.llvm.org/D47535#1116430, @labath wrote: > In https://reviews.llvm.org/D47535#1116392, @JDevlieghere wrote: > > > In https://reviews.llvm.org/D47535#1116364, @labath wrote: > > > > > Actually, I wonder if we shouldn't just deprecate this function >

[Lldb-commits] [PATCH] D47539: [Platform] Accept arbitrary kext variants

JDevlieghere updated this revision to Diff 149143. JDevlieghere added a comment. Don't use `EnumerateDirectory` https://reviews.llvm.org/D47539 Files: source/Plugins/Platform/MacOSX/PlatformDarwinKernel.cpp source/Plugins/Platform/MacOSX/PlatformDarwinKernel.h Index: source/Plugins/Platfo

[Lldb-commits] [PATCH] D47535: [FileSpec] Add support for lambdas to EnumerateDirectory. NFC

JDevlieghere abandoned this revision. JDevlieghere added a comment. Alright, let's abandon this and replace the existing uses with llvm iterators. Repository: rL LLVM https://reviews.llvm.org/D47535 ___ lldb-commits mailing list lldb-commits@list

[Lldb-commits] [PATCH] D47539: [Platform] Accept arbitrary kext variants

JDevlieghere added a comment. In https://reviews.llvm.org/D47539#1117114, @jasonmolenda wrote: > LGTM. If we added more knowledge specifically about kext bundle layouts, we > could restrict which files we test to see if they are valid binaries - but > we'd need to parse the Info.plist at the t

[Lldb-commits] [PATCH] D47470: AppleDWARFIndex: Get function method-ness directly from debug info

JDevlieghere added inline comments. Comment at: source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp:222 +return true; + return GetReferencedDIE(DW_AT_specification) + .GetParent() labath wrote: > clayborg wrote: > > labath wrote: > > > clayborg wrote: > > > >

<    1   2   3   4   5   6   7   8   9   10   >