[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] 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] 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] [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] 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] D101556: [lldb] Move and clean-up the Declaration class (NFC)

teemperor added a comment. IIRC they were #ifdef's out because of memory concerns. Did you have a chance to benchmark this quickly (e.g., trying to attach to a debug Clang and see how much it increases memory) Otherwise this LGTM. FWIW, we also need this for the SourceLocation in Clang Decls f

[Lldb-commits] [PATCH] D101537: [lldb] Make the NSSet formatter faster and less prone to infinite recursion

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGa76df78470d7: [lldb] Make the NSSet formatter faster and less prone to infinite recursion (authored by teemperor). Herald added a project: LLDB. Hera

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

teemperor accepted this revision. teemperor added a comment. This revision is now accepted and ready to land. FWIW, we do have a test for this named TestBuiltinFormats.py. It's using the SB API so it's not as a direct test as this one, so I think having this too is good. I think this looks good

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

teemperor added a comment. In D100965#2720235 , @jingham wrote: > In D100965#2716239 , @teemperor > wrote: > >> I am wondering how SourceLocationSpec is related to lldb's Declaration class >> (which is FileSpec

[Lldb-commits] [PATCH] D101333: Also display the underlying error message when displaying a fixit

teemperor accepted this revision. teemperor added a comment. This revision is now accepted and ready to land. LGTM, thanks! Comment at: lldb/source/Expression/UserExpression.cpp:309 +os << "expression failed to parse:\n"; +if (diagnostic_manager.Diagnostics().si

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

teemperor updated this revision to Diff 340897. teemperor added a comment. - Americanized the commas (Her Majesty the Queen won't be pleased, won't she?). - Just removed some justs. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D101153/new/ https://reviews.llvm.org/D101153 Files: lldb

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

teemperor accepted this revision. teemperor added a comment. This revision is now accepted and ready to land. LGTM, thanks! I can to land this for you, but first let's wait a bit to see if anyone else has any comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://

[Lldb-commits] [PATCH] D100977: [lldb] Use forward type in pointer-to-member

teemperor added a comment. Actually it seems Clang also rejects it when running on Windows? I would have assumed that only the `-fms-compatibility-version=19.0` or something like `-fdelayed-template-parsing` matters here but I'm not sure why this is different on Windows. I'm skipping for GCC an

[Lldb-commits] [PATCH] D100977: [lldb] Use forward type in pointer-to-member

teemperor added a comment. Sorry and thanks for commenting Stella! So it seems that using an incomplete type in the test is rejected by GCC and MSVC but accepted by Clang and ICC. I believe that should be accepted and that's also what the internet(TM) and Shafik are saying, so I'm going to skip

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

teemperor updated this revision to Diff 340520. teemperor marked 5 inline comments as done. teemperor added a comment. - Update diff with feedback. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D101153/new/ https://reviews.llvm.org/D101153 Files: lldb/docs/resources/test.rst Index: l

[Lldb-commits] [PATCH] D100977: [lldb] Use forward type in pointer-to-member

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGe439a463a308: [lldb] Use forward type in pointer-to-member (authored by emrekultursay, committed by teemperor). Repository: rG LLVM Github Monorep

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

teemperor marked 7 inline comments as done. teemperor added a comment. Addressing feedback. Comment at: lldb/docs/resources/test.rst:206 + +**Don't unnecessarily launch the test executable.** +Launching a process and running to a breakpoint can often be the most ---

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

teemperor added a comment. I am wondering how SourceLocationSpec is related to lldb's Declaration class (which is FileSpec + line + column and describes a location in source code)? It seems like the current SourceLocationSpec is just a Declaration with the two additional search variables (and t

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

teemperor added a comment. Ignore my comments above. I assumed we didn't have a minimal crash reproducer from the comment in the dependent patch. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D101237/new/ https://reviews.llvm.org/D101237 _

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

teemperor added a comment. You're right, we can't fix up the types afterwards so I guess inferring the attribute isn't an option here. I'll try to find some time to properly look at this next week, but in the meantime I pushed your bug report into creduce over night and that's what it spit out

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

teemperor added a comment. grep tells me that attribute also influences a bunch of other things in subtle ways (It seems to potentially influence ABIs and Obj-C encodings?) and we don't really know what other things this might influence in the future. So I have the suspicion that putting that a

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

teemperor added a comment. If anyone feels like any of the guidelines is actually controversial then let me know and I'll remove it from this review and split it out into its own patch. (I am also aware that the text directly above has some small overlap with the guidelines as it for example al

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

teemperor updated this revision to Diff 339978. teemperor added a comment. - Improve some wording. 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] D101153: [lldb][NFC] Specify guidelines for API tests

teemperor created this revision. teemperor added a reviewer: LLDB. teemperor added a project: LLDB. Herald added a subscriber: JDevlieghere. teemperor requested review of this revision. This patch specifies a few guidelines that our API tests should follow. The motivations for this are twofold:

[Lldb-commits] [PATCH] D101094: lldb/Instrumentation: NFC-ish use GetFrameCodeAddressForSymbolication()

teemperor added inline comments. Comment at: lldb/source/Plugins/InstrumentationRuntime/MainThreadChecker/InstrumentationRuntimeMainThreadChecker.cpp:273 + ThreadSP new_thread_sp = std::make_shared( + *process_sp, tid, PCs, pcs_are_call_addresses); shafi

[Lldb-commits] [PATCH] D100977: [lldb] Use forward type in pointer-to-member

teemperor added a comment. I assume you don't have a commit access, so I'm going to land this for you tomorrow. Thanks again for the patch! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D100977/new/ https://reviews.llvm.org/D100977 ___

[Lldb-commits] [PATCH] D99989: [lldb-vscode] Distinguish shadowed variables in the scopes request

teemperor added inline comments. Comment at: lldb/tools/lldb-vscode/lldb-vscode.cpp:2934 +// We first find out which variable names are duplicated +std::unordered_map variable_name_counts; +for (auto i = start_idx; i < end_idx; ++i) { clayborg wrote:

[Lldb-commits] [PATCH] D91780: [lldb] Fix that the expression commands --top-level flag overwrites --allow-jit false

This revision was automatically updated to reflect the committed changes. Closed by commit rGd616a6bd107f: [lldb] Fix that the expression commands --top-level flag overwrites --allow-jit… (authored by teemperor). Herald added a project: LLDB. Herald added a subscriber: lldb-commits. Repository:

[Lldb-commits] [PATCH] D100846: [lldb] Don't leak LineSequence in PDB parsers

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGe3dd82ae3c4e: [lldb] Don't leak LineSequence in PDB parsers (authored by teemperor). Herald added a subscriber: lldb-commits. Repository: rG LLVM

[Lldb-commits] [PATCH] D99989: [lldb-vscode] Distinguish shadowed variables in the scopes request

teemperor reopened this revision. teemperor added inline comments. This revision is now accepted and ready to land. Herald added a subscriber: JDevlieghere. Comment at: lldb/tools/lldb-vscode/lldb-vscode.cpp:2924 +// We first find out which variable names are duplicated +

[Lldb-commits] [PATCH] D100977: [lldb] Use forward type in pointer-to-member

teemperor accepted this revision. teemperor added a comment. This revision is now accepted and ready to land. The test below reproduces the eager layout generation (and the resulting crashes) for me. Just apply it on top and then this is good to go. Thanks for tracking this down! diff --git

[Lldb-commits] [PATCH] D81550: [lldb] Add support for evaluating expressions in static member functions

This revision was automatically updated to reflect the committed changes. teemperor marked an inline comment as done. Closed by commit rG00764c36edf8: [lldb] Add support for evaluating expressions in static member functions (authored by teemperor). Herald added a project: LLDB. Herald added a subs

[Lldb-commits] [PATCH] D100977: [lldb] Use forward type in pointer-to-member

teemperor added a comment. Were you able to reduce the reproducer you got to a test? If it's really the same bug as in D100180 then I guess a similar test case should trigger the issue (e.g., a class with a pointer-to-member member that references some type th

[Lldb-commits] [PATCH] D98153: Some FormatEntity.cpp cleanup and unit testing

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGe2039142f6b1: Some FormatEntity.cpp cleanup and unit testing (authored by nealsid, committed by teemperor). Repository: rG LLVM Github Monorepo C

[Lldb-commits] [PATCH] D98153: Some FormatEntity.cpp cleanup and unit testing

teemperor accepted this revision. teemperor added a comment. Thanks for updating and thanks for the patch! I can land it right now. Comment at: lldb/unittests/Core/FormatEntityTest.cpp:154 +TEST(FormatEntity, LookupAllEntriesInTree) { + for (const auto &testString : lookupStri

[Lldb-commits] [PATCH] D76964: Fix an issue where the IgnoreName function was not allowing "Class" to be looked up inside a namespace or other class.

teemperor accepted this revision. teemperor added a comment. This revision is now accepted and ready to land. I think this review got kind of stuck. I think the only missing thing is simplifying the `self.dbg.CreateTarget` call in the test, but otherwise this looks good to me. I'm just going to

[Lldb-commits] [PATCH] D100795: [lldb] Fix RichManglingContext::FromCxxMethodName() leak

teemperor accepted this revision. teemperor added a comment. This revision is now accepted and ready to land. LGTM, thanks for fixing this! We don't really have a thread for keep track of what other leak sanitiser failures needs to be fixed, but I believe with this revision (+ the revisions tha

[Lldb-commits] [PATCH] D100800: [lldb] Fix demangler leaks in the DWARF AST parser

teemperor accepted this revision. teemperor added a comment. This revision is now accepted and ready to land. LGTM. I still kinda like the unique_ptr deleter but let's not block bot-fixes with refactoring requests. I'll open a review for the unique_ptr as a follow up. In D100800#2699984

[Lldb-commits] [PATCH] D100800: [lldb] Fix demangler leaks in the DWARF AST parser

teemperor added a comment. Thanks for fixing this, I guess we really need a leak sanitizer/valgrind bot for LLDB... I just have some minor comments but otherwise this LGTM. Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:1229 if (!function_decl

[Lldb-commits] [PATCH] D91508: [LLDB/Lua] add support for one-liner breakpoint callback

teemperor added a comment. I think this might have broken the Windows build as it seems the LLDBSwigLuaBreakpointCallbackFunction return type is an error for MSVC (and not a warning which we seem to ignore locally). To quote someone from the Discord server: I'm getting the following build er

[Lldb-commits] [PATCH] D100212: [lldb] Delete dead StackFrameList::Merge

This revision was automatically updated to reflect the committed changes. Closed by commit rG5a5a94ed34b0: [lldb] Delete dead StackFrameList::Merge (authored by teemperor). Herald added a project: LLDB. Herald added a subscriber: lldb-commits. Repository: rG LLVM Github Monorepo CHANGES SINCE

[Lldb-commits] [PATCH] D100180: [lldb] Don't recursively load types of static member variables in the DWARF AST parser

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG34c697c85e9d: [lldb] Don't recursively load types of static member variables in the DWARF AST… (authored by teemperor). Herald added a subscriber: ll

[Lldb-commits] [PATCH] D100212: [lldb] Delete dead StackFrameList::Merge

teemperor created this revision. teemperor added a reviewer: LLDB. teemperor requested review of this revision. That code is unused since it's check-in in 2010 (and I believe it would leak memory when called as it releases the passed unique_ptr), so let's delete it. https://reviews.llvm.org/D10

[Lldb-commits] [PATCH] D99890: [lldb] Fix bug where memory read --outfile is not truncating the file

teemperor accepted this revision. teemperor added a comment. This revision is now accepted and ready to land. LGTM now beside the now redundant import Comment at: lldb/test/API/functionalities/memory/read/TestMemoryRead.py:7 import lldbsuite.test.lldbutil as lldbutil +import t

[Lldb-commits] [PATCH] D99890: [lldb] Fix bug where memory read --outfile is not truncating the file

teemperor requested changes to this revision. teemperor added a comment. This revision now requires changes to proceed. Thanks for the patch! I'll be extra nit-picky about the test to make up for the great meme steal of 2021. Comment at: lldb/test/API/functionalities/memory/re

[Lldb-commits] [PATCH] D99827: Clarifying the documentation for variable formatting wrt to qualifiers and adding a test that demonstrates this

teemperor accepted this revision. teemperor added a comment. This revision is now accepted and ready to land. LGTM, thanks! FWIW, there are actually a lot more 'qualifiers' supported in Clang that are ignored by the formatters (`restrict`, Obj-C garbage collector descriptions, custom address spa

[Lldb-commits] [PATCH] D99867: [lldb] Replace unneeded use of Foundation with ObjectiveC in tests (NFC)

teemperor added inline comments. Comment at: lldb/test/API/commands/frame/recognizer/main.m:1 -#import +#import kastiglione wrote: > teemperor wrote: > > teemperor wrote: > > > I guess this could also be just `stdio.h` (FWIW, the printfs in tests can > > > a

[Lldb-commits] [PATCH] D99867: [lldb] Replace unneeded use of Foundation with ObjectiveC in tests (NFC)

teemperor added inline comments. Comment at: lldb/test/API/commands/frame/recognizer/main.m:1 -#import +#import teemperor wrote: > I guess this could also be just `stdio.h` (FWIW, the printfs in tests can > also be removed as they just require including syst

[Lldb-commits] [PATCH] D99867: [lldb] Replace unneeded use of Foundation with ObjectiveC in tests (NFC)

teemperor accepted this revision. teemperor added a comment. This revision is now accepted and ready to land. Herald added a subscriber: JDevlieghere. Thanks for cleaning this up! Comment at: lldb/test/API/commands/frame/recognizer/main.m:1 -#import +#import ---

[Lldb-commits] [PATCH] D99652: [lldb] Prevent that LLDB randomly crashes in CommandLineParser::addOption by initializing LLVM's command line parser

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG18dbe0f954a7: [lldb] Prevent that LLDB randomly crashes in CommandLineParser::addOption by… (authored by teemperor). Herald added a subscriber: lldb-

[Lldb-commits] [PATCH] D99513: [lldb] Add a test for Obj-C properties with conflicting names

This revision was automatically updated to reflect the committed changes. Closed by commit rG6919c58262b0: [lldb] Add a test for Obj-C properties with conflicting names (authored by teemperor). Herald added a project: LLDB. Herald added a subscriber: lldb-commits. Repository: rG LLVM Github Mon

[Lldb-commits] [PATCH] D99331: [TESTS] Fix TestInlineStepping with ccac compiler

teemperor added a comment. So from what I understand this is resolved? If yes, can you abandon/close this revision (You can do this by selecting the "Abandon revision" action above the input field where you can comment). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https:

[Lldb-commits] [PATCH] D98153: Some FormatEntity.cpp cleanup and unit testing

teemperor accepted this revision. teemperor added a comment. This revision is now accepted and ready to land. LGTM, thanks for the patch (and especially the unit test)! Some two nits left but feel free to fix those when merging. If you don't have commit access I can also do that, just let me kno

[Lldb-commits] [PATCH] D81810: LLDB step-instruction gets stuck on jump to self

teemperor requested changes to this revision. teemperor added a comment. This revision now requires changes to proceed. Herald added subscribers: JDevlieghere, pengfei. (Marking as requested-changes to get this out of my review queue) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACT

[Lldb-commits] [PATCH] D99331: [TESTS] Fix TestInlineStepping with ccac compiler

teemperor edited reviewers, added: jingham; removed: LLVM. teemperor added a comment. Thanks for the patch! A few general notes: - You probably want to upload the diffs with more context (`git diff -U` or something like that) to get rid of the "Context not available" in the source changes

[Lldb-commits] [PATCH] D95713: [lldb/Plugins] Add ScriptedProcess Process Plugin

teemperor added a comment. In D95713#2647722 , @davezarzycki wrote: > In D95713#2647548 , @teemperor wrote: > >> The tests are failing because Dave's bot is running without enabled Python. >> The same is true for

[Lldb-commits] [PATCH] D95713: [lldb/Plugins] Add ScriptedProcess Process Plugin

teemperor added a comment. The tests are failing because Dave's bot is running without enabled Python. The same is true for the Windows bot. Putting the plugin behind `#ifdef LLDB_ENABLE_PYTHON` should fix this. FWIW, even with enabled Python this seems to have some minor problems with command

[Lldb-commits] [PATCH] D98153: Some FormatEntity.cpp cleanup and unit testing

teemperor requested changes to this revision. teemperor added a comment. This revision now requires changes to proceed. Sorry for the delay! > It seems like the pre-merge check failures are because I incorrectly set the > project repo when I first created the revision. I noticed in the build >

[Lldb-commits] [PATCH] D98842: [lldb] Make the api, shell and unit tests independent lit test suites

teemperor accepted this revision. teemperor added a comment. This revision is now accepted and ready to land. Clean builds for all the test targets work now and this also looks good, so let's ship it. (also thanks for working on that, the unit test builds where a big annoyance when testing API

[Lldb-commits] [PATCH] D98880: [lldb] Move Apple simulators test targets under API

teemperor accepted this revision. teemperor added a comment. This revision is now accepted and ready to land. LGTM, thanks! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D98880/new/ https://reviews.llvm.org/D98880 ___ lldb-commits mailing list

[Lldb-commits] [PATCH] D98842: [lldb] Make the api, shell and unit tests independent lit test suites

teemperor requested changes to this revision. teemperor added a comment. This revision now requires changes to proceed. Just gonna post a comment to point out that this apparently adds all tests twice to `check-all` which makes the tests go kaputt (but I only messaged Jonas directly about that,

[Lldb-commits] [PATCH] D98770: [lldb] Correct typo in memory read error

teemperor added a comment. Thanks! (and thanks for the patch) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D98770/new/ https://reviews.llvm.org/D98770 ___ lldb-commits mailing list lldb-commits@lists.llv

[Lldb-commits] [PATCH] D98770: [lldb] Correct typo in memory read error

teemperor accepted this revision. teemperor added a comment. This revision is now accepted and ready to land. LGTM (You can just land these straightforward patches directly without waiting for review, the dev policy allows that) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

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

teemperor added inline comments. Comment at: lldb/include/lldb/Symbol/CompilerDeclContext.h:77 /// in a struct, union or class. - bool IsClassMethod(lldb::LanguageType *language_ptr, - bool *is_instance_method_ptr, - ConstString *la

[Lldb-commits] [PATCH] D98441: [lldb] Fix the man page build

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG75f97cdafe52: [lldb] Fix the man page build (authored by teemperor). Herald added a subscriber: lldb-commits. Repository: rG LLVM Github Monorepo

[Lldb-commits] [PATCH] D98196: [lldb] Add missing debugserver/lldb-server dependencies to check-lldb

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGaada8984e617: [lldb] Add missing debugserver dependency to check-lldb (authored by teemperor). Herald added a subscriber: lldb-commits. Changed prio

[Lldb-commits] [PATCH] D96202: [lldb/test] Automatically find debug servers to test

teemperor added a comment. re-adding debugserver deps here: https://reviews.llvm.org/D98196 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D96202/new/ https://reviews.llvm.org/D96202 ___ lldb-commits maili

[Lldb-commits] [PATCH] D96202: [lldb/test] Automatically find debug servers to test

teemperor added inline comments. Comment at: lldb/test/API/CMakeLists.txt:115 -set(LLDB_TEST_SERVER ${debugserver_path}) -add_lldb_test_dependency(debugserver) - elseif(TARGET lldb-server) labath wrote: > teemperor wrote: > > I believe this removed line

[Lldb-commits] [PATCH] D98153: Some FormatEntity.cpp cleanup and unit testing

teemperor requested changes to this revision. teemperor added a comment. This revision now requires changes to proceed. Thanks for cleaning this up! Are the `if (!sc) ...` stuff are missing nullptr checks that affect a normal LLDB session (not sure if we can ever have no SymbolContext) or is thi

[Lldb-commits] [PATCH] D98170: [lldb] Fix error message in IRInterpreter

teemperor added a comment. I would be curious how you found that bug? I believe a lot of these errors are kinda impossible to reach in a normal test setup (I remember I tried writing some tests for this but can't remember finding a good way to get into all the different error branches in IRInte

[Lldb-commits] [PATCH] D98170: [lldb] Fix error message in IRInterpreter

teemperor accepted this revision. teemperor added a comment. This revision is now accepted and ready to land. Herald added a subscriber: JDevlieghere. LGTM. I think the Read/Write methods should return a Status instead of some out parameter, but that seems like a NFC follow-up refactoring (I can

[Lldb-commits] [PATCH] D96202: [lldb/test] Automatically find debug servers to test

teemperor added inline comments. Comment at: lldb/test/API/CMakeLists.txt:115 -set(LLDB_TEST_SERVER ${debugserver_path}) -add_lldb_test_dependency(debugserver) - elseif(TARGET lldb-server) I believe this removed line is causing us to have missing depende

[Lldb-commits] [PATCH] D97760: [lldb][NFC] Delete unused AddressResolverName

This revision was automatically updated to reflect the committed changes. Closed by commit rG820a8466097c: [lldb][NFC] Delete unused AddressResolverName (authored by teemperor). Herald added a subscriber: lldb-commits. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://rev

[Lldb-commits] [PATCH] D97696: [lldb] Enable cross CU decl file test only for x64 Linux

teemperor added a comment. You can just directly land these bot fixes (or you revert the original aptch). Not sure if I'm the right person to review this as I rarely touch the shell tests, but Jonas or Pavel probably know for sure what's the proper way to restrict the test environment for this

[Lldb-commits] [PATCH] D97760: [lldb][NFC] Delete unused AddressResolverName

teemperor created this revision. teemperor added a reviewer: LLDB. teemperor added a project: LLDB. Herald added subscribers: JDevlieghere, mgorny. teemperor requested review of this revision. That's all just dead code that hasn't been changed in years. https://reviews.llvm.org/D97760 Files:

[Lldb-commits] [PATCH] D97586: [mlir][lldb] Fix several gcc warnings in mlir and lldb

teemperor added inline comments. Comment at: lldb/source/Commands/CommandObjectTrace.cpp:119 lldb::TraceSP trace_sp = traceOrErr.get(); - if (m_options.m_verbose) + if (m_options.m_verbose && trace_sp) result.AppendMessageWithFormat("loading trace with p

[Lldb-commits] [PATCH] D97739: Add a progress class that can track and report long running operations that happen in LLDB.

teemperor requested changes to this revision. teemperor added a comment. This revision now requires changes to proceed. Thanks for working on this. I really like the idea and I think this would be really cool to have. I do have some concerns about the way this is implemented though (see inline c

[Lldb-commits] [PATCH] D97586: [mlir][lldb] Fix several gcc warnings in mlir and lldb

teemperor added inline comments. Comment at: lldb/source/Commands/CommandObjectTrace.cpp:119 lldb::TraceSP trace_sp = traceOrErr.get(); - if (m_options.m_verbose) + if (m_options.m_verbose && trace_sp) result.AppendMessageWithFormat("loading trace with p

[Lldb-commits] [PATCH] D95119: Prefer /usr/bin/env xxx over /usr/bin/xxx where xxx = perl, python, awk

This revision was automatically updated to reflect the committed changes. Closed by commit rGa54f160b3a98: Prefer /usr/bin/env xxx over /usr/bin/xxx where xxx = perl, python, awk (authored by haampie, committed by teemperor). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION http

[Lldb-commits] [PATCH] D97249: [lldb] Support debugging utility functions

teemperor accepted this revision. teemperor added a comment. Some small nitpicks about comments, otherwise LGTM I was kinda thinking how we could test this, but all our utility functions are anyway only on macOS and very Obj-C runtime related, so that sounds like a pain to do right... (PS: Als

[Lldb-commits] [PATCH] D96947: [lldb] Prevent double new lines behind errors/warning/messages from LLDB commands

teemperor added inline comments. Comment at: lldb/source/Interpreter/CommandReturnObject.cpp:93 return; - GetOutputStream() << in_string << "\n"; + GetOutputStream() << in_string; + if (!in_string.endswith("\n")) JDevlieghere wrote: > How about rstripping

[Lldb-commits] [PATCH] D96947: [lldb] Prevent double new lines behind errors/warning/messages from LLDB commands

This revision was automatically updated to reflect the committed changes. Closed by commit rG6201017d541f: [lldb] Prevent double new lines behind errors/warning/messages from LLDB… (authored by teemperor). Herald added a subscriber: lldb-commits. Changed prior to commit: https://reviews.llvm.or

[Lldb-commits] [PATCH] D97298: [lldb][NFC] Move trivial ValueObject getters/setters to the header

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG0512b01ebede: [lldb][NFC] Move trivial ValueObject getters/setters to the header (authored by teemperor). Herald added a subscriber: lldb-commits. R

[Lldb-commits] [PATCH] D97298: [lldb][NFC] Move trivial ValueObject getters/setters to the header

teemperor updated this revision to Diff 326052. teemperor added a comment. - Rebased CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97298/new/ https://reviews.llvm.org/D97298 Files: lldb/include/lldb/Core/ValueObject.h lldb/source/Core/ValueObject.cpp Index: lldb/source/Core/ValueO

[Lldb-commits] [PATCH] D97287: [lldb][NFC] Rename the second ValueObjectManager to ValueObjectUpdater and remove the dead code

This revision was automatically updated to reflect the committed changes. Closed by commit rG4631afdeb3c4: [lldb][NFC] Rename the second ValueObjectManager to ValueObjectUpdater and… (authored by teemperor). Herald added a subscriber: lldb-commits. Repository: rG LLVM Github Monorepo CHANGES S

[Lldb-commits] [PATCH] D97300: [lldb] Add asserts that prevent construction of cycles in the decl origin tracking

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG2105912ee0b8: [lldb] Add asserts that prevent construction of cycles in the decl origin… (authored by teemperor). Herald added a subscriber: lldb-com

[Lldb-commits] [PATCH] D97249: [lldb] Support debugging utility functions

teemperor added inline comments. Comment at: lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionSourceCode.cpp:38 const char *ClangExpressionSourceCode::g_expression_prefix = -"#line 1 \"" PREFIX_NAME R"(" +"#line 1 \"" PREFIX_NAME R"("#line 1 #ifndef offsetof -

[Lldb-commits] [PATCH] D97307: [lldb][NFC] Extract ValueObject's expression path parsing into own class

teemperor created this revision. teemperor added a reviewer: LLDB. teemperor added a project: LLDB. Herald added subscribers: JDevlieghere, mgorny. teemperor requested review of this revision. A large chunk of the `ValueObject` implementation is the hand-written expression path parser and nearly

[Lldb-commits] [PATCH] D97298: [lldb][NFC] Move trivial ValueObject getters/setters to the header

teemperor created this revision. teemperor added a reviewer: LLDB. teemperor added a project: LLDB. Herald added a subscriber: JDevlieghere. teemperor requested review of this revision. NFC refactoring that moves the definitions of all the trivial getters/setters to the header file which is what

[Lldb-commits] [PATCH] D97287: [lldb][NFC] Rename the second ValueObjectManager to ValueObjectUpdater and remove the dead code

teemperor created this revision. teemperor added a reviewer: LLDB. teemperor added a project: LLDB. Herald added subscribers: JDevlieghere, mgorny. teemperor requested review of this revision. `ValueObject.h` contains the `ValueObject::ValueObjectManager` type which is just a typedef for the Clus

[Lldb-commits] [PATCH] D97165: [lldb] Add deref support and tests to shared_ptr synthetic

teemperor accepted this revision. teemperor added a comment. This revision is now accepted and ready to land. This LGTM modulo a missing nullptr check. Thanks for fixing this! Also we probably should investigate the `strong=` summary differences. I would have blamed the fake constructors we are

[Lldb-commits] [PATCH] D97205: [lldb][NFC] Don't inherit from UserID in ValueObject

This revision was automatically updated to reflect the committed changes. Closed by commit rGd77e3c6aec29: [lldb][NFC] Don't inherit from UserID in ValueObject (authored by teemperor). Herald added a subscriber: lldb-commits. Changed prior to commit: https://reviews.llvm.org/D97205?vs=325708&id

[Lldb-commits] [PATCH] D97205: [lldb][NFC] Don't inherit from UserID in ValueObject

teemperor updated this revision to Diff 325708. teemperor added a comment. - Rebased. - Add a `GetID()` call where we relied on the `==` overload before. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97205/new/ https://reviews.llvm.org/D97205 Files: lldb/include/lldb/Core/ValueObject

[Lldb-commits] [PATCH] D97199: [lldb][NFC] Cleanup ValueObject construction code

This revision was automatically updated to reflect the committed changes. Closed by commit rG8f63cf5da3c0: [lldb][NFC] Cleanup ValueObject construction code (authored by teemperor). Herald added a subscriber: lldb-commits. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https:/

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