[Lldb-commits] [PATCH] D59102: Add an LLVM-style dump method to CompilerType for extra convenience during debugging

davide added a comment. We should also add it to `SwiftASTContext` downstream. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59102/new/ https://reviews.llvm.org/D59102 ___ lldb-commits mailing list lld

[Lldb-commits] [PATCH] D59159: [third-party] Update pexpect to 4.6.

This revision was not accepted when it landed; it landed in state "Needs Revision". This revision was automatically updated to reflect the committed changes. Closed by commit rLLDB355967: [third-party] Update pexpect to 4.6. (authored by davide, committed by ). Herald added a project: LLDB. Chan

[Lldb-commits] [PATCH] D59580: Use compatible print statements for Python2/3

davide added a comment. Thanks for your interest in the subject of Python! I converted everything I was able to find looking at our test suite, it seems that we lack coverage for these scripts. Did you end up testing them with both py 2 and py 3 or just applied a tool on them? Repository: r

[Lldb-commits] [PATCH] D59580: Use compatible print statements for Python2/3

davide accepted this revision. davide added a comment. This revision is now accepted and ready to land. This looks good to me as long as it doesn't break check-lldb (and from I quick look at the patch I doubt it does). Thanks for helping with this! Repository: rLLDB LLDB CHANGES SINCE LAST A

[Lldb-commits] [PATCH] D59667: Regression test to ensure that we handling importing of anonymous enums correctly

davide accepted this revision. davide added a comment. This revision is now accepted and ready to land. LGTM, just clang format `main.cpp` before committing. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59667/new/ https://reviews.llvm.org/D59667 _

[Lldb-commits] [PATCH] D59719: [ScriptInterpreter] Make sure that PYTHONHOME is right.

davide created this revision. davide added reviewers: jingham, friss, JDevlieghere, aprantl, jasonmolenda. Herald added subscribers: llvm-commits, jdoerfert. Herald added a project: LLVM. For the only version of Python actually supported on Darwin. rdar://problem/40961425 Repository: rL LLVM

[Lldb-commits] [PATCH] D59719: [ScriptInterpreter] Make sure that PYTHONHOME is right.

davide updated this revision to Diff 191958. davide added a comment. Adrian's feedback Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59719/new/ https://reviews.llvm.org/D59719 Files: lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp Ind

[Lldb-commits] [PATCH] D59719: [ScriptInterpreter] Make sure that PYTHONHOME is right.

This revision was automatically updated to reflect the committed changes. Closed by commit rLLDB356816: [ScriptInterpreter] Make sure that PYTHONHOME is right. (authored by davide, committed by ). Herald added a project: LLDB. Changed prior to commit: https://reviews.llvm.org/D59719?vs=191958&i

[Lldb-commits] [PATCH] D59589: Python 2/3 compat: str vs basestring

davide accepted this revision. davide added a comment. This revision is now accepted and ready to land. LGTM. Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59589/new/ https://reviews.llvm.org/D59589 ___ lldb-commit

[Lldb-commits] [PATCH] D59719: [ScriptInterpreter] Make sure that PYTHONHOME is right.

davide added a comment. In D59719#1442181 , @ted wrote: > This doesn't look correct to me - it looks like there are 1 too many #endifs. > I think the one at line 179 should be removed - it should have been replaced > by the #else that is at line 180. U

[Lldb-commits] [PATCH] D59719: [ScriptInterpreter] Make sure that PYTHONHOME is right.

davide added a comment. In D59719#1441254 , @labath wrote: > It sounds to me like you could achieve the same thing by generalizing the > LLDB_PYTHON_HOME logic in LLDBConfig.cmake. This would have the advantage of > centralizing the way we manage python-

[Lldb-commits] [PATCH] D59719: [ScriptInterpreter] Make sure that PYTHONHOME is right.

davide added a comment. In D59719#1441254 , @labath wrote: > It sounds to me like you could achieve the same thing by generalizing the > LLDB_PYTHON_HOME logic in LLDBConfig.cmake. This would have the advantage of > centralizing the way we manage python-

[Lldb-commits] [PATCH] D59831: [CMake] macOS: Find DebugSymbols.framework inside the SDK

davide accepted this revision. davide added a comment. This revision is now accepted and ready to land. LGTM Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59831/new/ https://reviews.llvm.org/D59831 ___ lldb-commits

[Lldb-commits] [PATCH] D59896: [ObjectFileMachO] Disable memory caching for savecore.

davide created this revision. davide added a reviewer: jasonmolenda. Herald added a project: LLVM. Herald added a subscriber: llvm-commits. It's not really useful, and largely increases the footprint. rdar://problem/49293525 Repository: rL LLVM https://reviews.llvm.org/D59896 Files: lldb/

[Lldb-commits] [PATCH] D59896: [ObjectFileMachO] Disable memory caching for savecore.

davide updated this revision to Diff 192496. davide added a comment. try CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59896/new/ https://reviews.llvm.org/D59896 Files: lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp Index: lldb/source/Plugins/ObjectFile/Mach-O/ObjectFile

[Lldb-commits] [PATCH] D59896: [ObjectFileMachO] Disable memory caching for savecore.

davide added a comment. In D59896#1445310 , @jasonmolenda wrote: > Looks good. Removing that printf is good. Could you also remove the > printf("mach_header:...") in the same function? > > It would be nice if include/lldb/Target/Process.h had the decl

[Lldb-commits] [PATCH] D59896: [ObjectFileMachO] Disable memory caching for savecore.

This revision was automatically updated to reflect the committed changes. Closed by commit rL357134: [ObjectFileMachO] Disable memory caching for savecore. (authored by davide, committed by ). Herald added a project: LLVM. Changed prior to commit: https://reviews.llvm.org/D59896?vs=192496&id=19

[Lldb-commits] [PATCH] D59896: [ObjectFileMachO] Disable memory caching for savecore.

davide added a comment. Done. $ git llvm push Pushing 3 commits: d3e193e68a7 [ObjectFileMachO] Disable memory caching for savecore. 10502683510 [ObjectFileMachO] Remove another debugging aid. a619d7b69b9 [Process] Reorder declarations and document ReadMemoryFromInferior. Sendin

[Lldb-commits] [PATCH] D59957: Convert = operators that take object mutexes to the multi-lock version of std::lock

davide added subscribers: labath, davide. davide added a comment. This looks correct to me, but I'm not extremely familiar either, so I'd wait for @labath to sign off. Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59957/new/ https://reviews.llvm.org/D59957

[Lldb-commits] [PATCH] D59911: Don't abort() in lldb_assert and document why.

davide added a comment. My (maybe unpopolar) opinion on the subject is that "soft assertions" are a way to cleanse your conscience of guilt, but they don't work really well in practice. When I started working on lldb, I was a fairly strong proponent of assertions everywhere. My view changed som

[Lldb-commits] [PATCH] D59911: Don't abort() in lldb_assert and document why.

davide added a comment. In D59911#1446787 , @aprantl wrote: > Thanks for summarizing your thoughts, Davide. > > I think that what you wrote is a much better explanation of what I was trying > to say with > > Use these sparingly and only if error handlin

[Lldb-commits] [PATCH] D59911: Don't abort() in lldb_assert and document why.

davide accepted this revision. davide added a comment. LGTM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59911/new/ https://reviews.llvm.org/D59911 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/ma

[Lldb-commits] [PATCH] D60022: [Process] Fix WriteMemory return value

davide added inline comments. Comment at: packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestWriteMemory.py:19-20 +target = self.dbg.CreateTarget('') +if self.TraceOn(): +self.runCmd("log enable gdb-remote packets") +process

[Lldb-commits] [PATCH] D60022: [Process] Fix WriteMemory return value

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

[Lldb-commits] [PATCH] D60092: [Process] Use early returns in Process::WriteMemory (NFC)

davide added a comment. LGTM. Med, what do you think? Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60092/new/ https://reviews.llvm.org/D60092 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists

[Lldb-commits] [PATCH] D60300: [testsuite] Split Objective-C data formatter

davide added a comment. I think this is good regardless for readability, but I would really appreciate if we can collect some numbers to see how effective this change actually is. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60300/new/ https://reviews.llvm.org/D60300 __

[Lldb-commits] [PATCH] D60300: [testsuite] Split Objective-C data formatter

davide accepted this revision. davide added a comment. This revision is now accepted and ready to land. great! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60300/new/ https://reviews.llvm.org/D60300 ___ lldb-commits mailing list lldb-commi

[Lldb-commits] [PATCH] D60300: [testsuite] Split Objective-C data formatter

davide added a comment. In D60300#1456472 , @clayborg wrote: > In D60300#1456468 , @JDevlieghere > wrote: > > > In D60300#1455790 , @davide wrote: > > > > > I think this is

[Lldb-commits] [PATCH] D60300: [testsuite] Split Objective-C data formatter

davide added a comment. In D60300#1456491 , @davide wrote: > In D60300#1456472 , @clayborg wrote: > > > In D60300#1456468 , @JDevlieghere > > wrote: > > > > > In D60300#1455

[Lldb-commits] [PATCH] D60654: Fix UNPREDICTABLE check in EmulateInstructionARM::EmulateADDRegShift

davide accepted this revision. davide added a comment. This revision is now accepted and ready to land. Add an unittest maybe? otherwise, seems to match the ISA so LGTM Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60654/new/ https://reviews.llvm.org/D60654

[Lldb-commits] [PATCH] D60780: [tools] Only build lldb-instr and lldb-vscode if asked.

davide created this revision. davide added reviewers: JDevlieghere, friss. Herald added subscribers: lldb-commits, mgorny. Herald added a project: LLDB. Saves some build times, and they're not part of the usual developer workflow. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D

[Lldb-commits] [PATCH] D60780: [tools] Only build lldb-instr and lldb-vscode if asked.

davide added a comment. OK, on it. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60780/new/ https://reviews.llvm.org/D60780 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.

[Lldb-commits] [PATCH] D60780: [tools] Only build lldb-instr and lldb-vscode if asked.

davide updated this revision to Diff 195443. davide added a comment. updated. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60780/new/ https://reviews.llvm.org/D60780 Files: lldb/cmake/modules/AddLLDB.cmake lldb/lit/lit.site.cfg.py.in lldb/l

[Lldb-commits] [PATCH] D60780: [tools] Only build lldb-instr and lldb-vscode if asked.

davide updated this revision to Diff 195454. davide added a comment. Jonas' comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60780/new/ https://reviews.llvm.org/D60780 Files: lldb/cmake/modules/AddLLDB.cmake lldb/lit/CMakeLists.txt lld

[Lldb-commits] [PATCH] D60780: [tools] Only build lldb-instr and lldb-vscode if asked.

This revision was automatically updated to reflect the committed changes. Closed by commit rLLDB358528: [tools] Make vscode and lldb-instr optional. (authored by davide, committed by ). Changed prior to commit: https://reviews.llvm.org/D60780?vs=195454&id=195459#toc Repository: rLLDB LLDB C

[Lldb-commits] [PATCH] D76697: [lldb] Replace debug-only assert in AppleObjCTypeEncodingParser::BuildObjCObjectPointerType with lldbassert

davide added a comment. LGTM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76697/new/ https://reviews.llvm.org/D76697 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D73938: [Host.mm] Check for the right macro instead of inlining it

davide accepted this revision. davide added a comment. This revision is now accepted and ready to land. LGTM =) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73938/new/ https://reviews.llvm.org/D73938 ___ lldb-commits mailing list lldb-comm

[Lldb-commits] [PATCH] D73921: Assert that a subprogram should have a name when parsing DWARF

davide added a comment. In D73921#1868286 , @JDevlieghere wrote: > In D73921#1868228 , @jingham wrote: > > > In D73921#1855961 , @davide wrote: > > > > > DWARFASTParserClang

[Lldb-commits] [PATCH] D75496: [lldb][NFC] Remove some commented out code in TypeSystemClang

davide added a comment. LGTM. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75496/new/ https://reviews.llvm.org/D75496 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D75496: [lldb][NFC] Remove some commented out code in TypeSystemClang

davide added a comment. @teemperor quite honestly you touched this code enough lately that you can commit such trivial changes without asking for review. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75496/new/ https://reviews.llvm.org/D75496

[Lldb-commits] [PATCH] D76697: [lldb] Remove Debug-only assert in AppleObjCTypeEncodingParser::BuildObjCObjectPointerType

davide added a comment. Please don't do this. I've seen that assertion triggering. If anything, you might want to make a `lldbassert`. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76697/new/ https://reviews.llvm.org/D76697 ___ lldb-commit

[Lldb-commits] [PATCH] D76697: [lldb] Remove Debug-only assert in AppleObjCTypeEncodingParser::BuildObjCObjectPointerType

davide added a comment. Also, this was asserting already -- so it's not introducing any new `assert()`. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76697/new/ https://reviews.llvm.org/D76697 ___ lldb-commits mailing list lldb-commits@list

[Lldb-commits] [PATCH] D76697: [lldb] Remove Debug-only assert in AppleObjCTypeEncodingParser::BuildObjCObjectPointerType

davide added a comment. I don't recall the details. Soft assertions. LLDB provides lldb_assert() as a soft alternative to cover the middle ground of situations that indicate a recoverable bug in LLDB. In a Debug configuration lldb_assert() behaves like assert(). In a Release configuration it

[Lldb-commits] [PATCH] D76697: [lldb] Remove Debug-only assert in AppleObjCTypeEncodingParser::BuildObjCObjectPointerType

davide added a comment. In D76697#1939731 , @JDevlieghere wrote: > According to the comment there is at least one valid reason for this code > path to trigger. An `lldb_assert` will encourage users to file bugs and I > think we have enough real issues t

[Lldb-commits] [PATCH] D76687: [lldb][NFC] Always update m_cache_{hits/misses} in FormatCache

davide added a comment. Testcase? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76687/new/ https://reviews.llvm.org/D76687 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.o

[Lldb-commits] [PATCH] D76730: [RNBRemote/arm64] Remove a couple of unused functions

davide created this revision. davide added reviewers: jasonmolenda, LLDB. Herald added a subscriber: kristof.beyls. Jason, before submitting this I wanted to run it past you. https://reviews.llvm.org/D76730 Files: lldb/tools/debugserver/source/RNBRemote.cpp Index: lldb/tools/debugserver/sour

[Lldb-commits] [PATCH] D77173: [PATCH] [debugserver/ARM64] Make sure watchpoints hit are attributed correctly.

davide created this revision. davide added a reviewer: jasonmolenda. davide added a project: LLDB. Herald added subscribers: danielkiss, kristof.beyls. >From e330facaea0c3780734a6a061134551662fb9d74 Mon Sep 17 00:00:00 2001 From: Davide Italiano Date: Tue, 31 Mar 2020 13:55:36 -0700 Subj

[Lldb-commits] [PATCH] D77698: [DWARF] Assign the correct scope to constant variables

davide added a comment. See also https://bugs.llvm.org/show_bug.cgi?id=45471 CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77698/new/ https://reviews.llvm.org/D77698 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.ll

[Lldb-commits] [PATCH] D77698: [DWARF] Assign the correct scope to constant variables

davide created this revision. davide added reviewers: jingham, labath. Herald added subscribers: arphaman, aprantl. davide added a comment. davide added a reviewer: friss. See also https://bugs.llvm.org/show_bug.cgi?id=45471 SymbolFileDWARF::ParseVariableDIE consider all constant variables as "s

[Lldb-commits] [PATCH] D77698: [DWARF] Assign the correct scope to constant variables

davide updated this revision to Diff 256063. davide added a comment. Herald added a reviewer: jdoerfert. Added test, updated comment. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77698/new/ https://reviews.llvm.org/D77698 Files: lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEnt

[Lldb-commits] [PATCH] D77698: [DWARF] Assign the correct scope to constant variables

davide closed this revision. davide added a comment. commit d51b38f1b3a34c2a8e1869af6434ebd743ce7a5e <https://reviews.llvm.org/rGd51b38f1b3a34c2a8e1869af6434ebd743ce7a5e> (HEAD -> master, origin/master, origin/HEAD) Author: Davide Italiano Date: Wed Apr 8 11:06:00 2020 -0700 [D

[Lldb-commits] [PATCH] D77698: [DWARF] Assign the correct scope to constant variables

davide marked an inline comment as done. davide added inline comments. Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp:1029 + return false; + +case DW_TAG_compile_unit: aprantl wrote: > To save everyone the trouble: > ``` > 0x000

[Lldb-commits] [PATCH] D84263: [debugserver/Apple Silicon] Handoff connections when attaching to translated processes

davide created this revision. davide added a reviewer: jasonmolenda. When we detect a process that the native debugserver cannot handle, handoff the connection fd to the translated debugserver. https://reviews.llvm.org/D84263 Files: lldb/tools/debugserver/source/DNB.cpp lldb/tools/debugser

[Lldb-commits] [PATCH] D84263: [debugserver/Apple Silicon] Handoff connections when attaching to translated processes

davide added a comment. yeah, I think it's reasonable. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84263/new/ https://reviews.llvm.org/D84263 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman

[Lldb-commits] [PATCH] D84272: Add checks for ValueObjectSP in Cocoa summary providers

davide requested changes to this revision. davide added a comment. This revision now requires changes to proceed. why? Do you have a testcase? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84272/new/ https://reviews.llvm.org/D84272 ___ lldb

[Lldb-commits] [PATCH] D84263: [debugserver/Apple Silicon] Handoff connections when attaching to translated processes

davide updated this revision to Diff 282073. davide added a comment. Added the check that Jason requested. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84263/new/ https://reviews.llvm.org/D84263 Files: lldb/tools/debugserver/source/DNB.cpp lldb/tools/debugserver/source/debugserver

[Lldb-commits] [PATCH] D84263: [debugserver/Apple Silicon] Handoff connections when attaching to translated processes

davide accepted this revision. davide added a comment. This revision is now accepted and ready to land. Reviewed by Jason privately. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84263/new/ https://reviews.llvm.org/D84263 ___ lldb-commits mai

[Lldb-commits] [PATCH] D84263: [debugserver/Apple Silicon] Handoff connections when attaching to translated processes

davide closed this revision. davide added a comment. https://github.com/llvm/llvm-project/commit/57605758b5de3726eec1d6e587de1003af1ab5b7 CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84263/new/ https://reviews.llvm.org/D84263 ___ lldb-commit

[Lldb-commits] [PATCH] D85365: [lldb] Modify the `skipIfRemote` decorator so we can skip all PExpect tests.

davide accepted this revision. davide added a comment. This revision is now accepted and ready to land. I assume we're still allowing to put the decorator on a test-by-test basis, and that seems the case from what I see. If so, LG. Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://r

[Lldb-commits] [PATCH] D85049: Unify the code that updates the ArchSpec after finding a fat binary with how it is done for a lean binary

davide added inline comments. Comment at: lldb/source/Target/TargetList.cpp:110-111 +// architecture so that the platform matching can be more accurate. +if (!platform_arch.TripleOSWasSpecified() || +!platform_arch.TripleVendorWasSpecified()) { + prefer_platf

[Lldb-commits] [PATCH] D85376: Fix how ValueObjectChild handles bit-fields stored in a Scalar in UpdateValue()

davide requested changes to this revision. davide added inline comments. This revision now requires changes to proceed. Comment at: lldb/test/API/functionalities/data-formatter/valueobj-pass-by-reg/Makefile:2 +EXE := a.out +CFLAGS := -O1 + This is fundamentally

[Lldb-commits] [PATCH] D85376: Fix how ValueObjectChild handles bit-fields stored in a Scalar in UpdateValue()

davide added inline comments. Comment at: lldb/test/API/functionalities/data-formatter/valueobj-pass-by-reg/Makefile:2 +EXE := a.out +CFLAGS := -O1 + davide wrote: > This is fundamentally a no-go. Depending on the optimization pipeline passes > this in a regist

[Lldb-commits] [PATCH] D85388: [lldb] Fix bug in skipIfRosetta decorator

davide accepted this revision. davide added a comment. This revision is now accepted and ready to land. yes, this makes sense. We could refine the check in future. Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85388/new/ https://reviews.llvm.org/D85388 ___

[Lldb-commits] [PATCH] D85396: Fix a small memory leak in VectorType.cpp and BlockPointer.cpp

davide accepted this revision. davide added a comment. This revision is now accepted and ready to land. This is correct to the best of my understanding. Thank you Jason. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85396/new/ https://reviews.llvm.

[Lldb-commits] [PATCH] D85396: Fix a small memory leak in VectorType.cpp and BlockPointer.cpp

davide added a comment. In D85396#2198668 , @tschuett wrote: > auto_ptr is deprecated since C++11 and will be removed in C++17. @jasonmolenda you might want to use `std::unique_ptr` instead -- probably LLVM won't migrate to 17 for a while, but still. R

[Lldb-commits] [PATCH] D86436: [lldb] Fix Type::GetByteSize for pointer types

davide accepted this revision. davide added a comment. This revision is now accepted and ready to land. I came to the same conclusion when analyzing https://bugs.llvm.org/show_bug.cgi?id=47257 (but you beat me to the punch). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION htt

[Lldb-commits] [PATCH] D86436: [lldb] Fix Type::GetByteSize for pointer types

davide added inline comments. Comment at: lldb/source/Symbol/Type.cpp:378 m_byte_size_has_value = true; +return m_byte_size; } shafik wrote: > Wouldn't it be better to turn `m_byte_size` into an `Optional`? As this fix > shows this having

[Lldb-commits] [PATCH] D86436: [lldb] Fix Type::GetByteSize for pointer types

davide added a subscriber: teemperor. davide added a comment. @labath something we noticed when finding this (and related bugs) is that `frame var` carries a decent diagnostic (int *) l_125 = and the expression parser returns just returns something not particularly useful: (lldb) p l_125

[Lldb-commits] [PATCH] D86436: [lldb] Fix Type::GetByteSize for pointer types

davide added inline comments. Comment at: lldb/source/Symbol/Type.cpp:378 m_byte_size_has_value = true; +return m_byte_size; } shafik wrote: > labath wrote: > > davide wrote: > > > shafik wrote: > > > > Wouldn't it be better to turn `m_byt

[Lldb-commits] [PATCH] D82813: [Apple Silicon] Rewrite part of the Rosetta support to be confined in Apple specific files

davide closed this revision. davide added a comment. Somebody will pick this up, probably in a different form. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82813/new/ https://reviews.llvm.org/D82813 ___ lldb-commits mailing list lldb-commits

[Lldb-commits] [PATCH] D68179: [lldb] Fix JSON parser to allow empty arrays

davide requested changes to this revision. davide added a comment. This revision now requires changes to proceed. This needs a test. You can either write a python one as the ones in `test/testcases` or a lit-style based as the ones in `lit/`. CHANGES SINCE LAST ACTION https://reviews.llvm.org

[Lldb-commits] [PATCH] D68674: Remove the is_mangled flag from Mangled and Symbol

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

[Lldb-commits] [PATCH] D68679: [CMake] Add a cache for iOS.

davide created this revision. davide added reviewers: friss, jasonmolenda. Herald added a subscriber: mgorny. Herald added a project: LLDB. rdar://problem/55916729 Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D68679 Files: lldb/cmake/caches/Apple-lldb-iOS.cmake Index: lld

[Lldb-commits] [PATCH] D69016: [lldb] move more things from python to cmake

davide added a comment. What are you trying to accomplish here? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69016/new/ https://reviews.llvm.org/D69016 ___ lldb-commits mailing list lldb-commits@lists

[Lldb-commits] [PATCH] D69523: [debugserver] Detect arch from LLVM_DEFAULT_TARGET_TRIPLE

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

[Lldb-commits] [PATCH] D69714: [lldb] Provide a getter for m_materializer_up in LLVMUserExpression instead of relying on it being accessible.

davide accepted this revision. davide added a comment. This revision is now accepted and ready to land. LGTM. Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69714/new/ https://reviews.llvm.org/D69714 ___ lldb-commit

[Lldb-commits] [PATCH] D70070: [lldb][NFC] Move LLVM RTTI implementation from enum to static ID variable

davide added a comment. LGTM. Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70070/new/ https://reviews.llvm.org/D70070 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailma

[Lldb-commits] [PATCH] D70074: [lldb][NFC] Remove unused CompilerType::IsPossibleCPlusPlusDynamicType

davide accepted this revision. davide added a comment. This revision is now accepted and ready to land. Nice! Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70074/new/ https://reviews.llvm.org/D70074 ___ lldb-commit

[Lldb-commits] [PATCH] D68179: [lldb] Fix JSON parser to allow empty arrays

davide accepted this revision. davide added a comment. This revision is now accepted and ready to land. Thanks for adding the tests. I took a look at the patch and it seems fine to me. Do you need somebody to commit this for you? @jingham or @labath may want to have another look. CHANGES SINCE

[Lldb-commits] [PATCH] D68179: [lldb] Fix JSON parser to allow empty arrays

davide added a comment. In D68179#1745614 , @tetsuo-cpp wrote: > Thanks for looking at this. I will need someone to commit it for me. > However, I've been having issues with the test suite on my MacBook. > `check-lldb-unit` works for me but `check-llvm`

[Lldb-commits] [PATCH] D70393: [lldb] Fix NSURL data formatter truncation issue in Swift

davide added a comment. While I review this, can you create a corresponding change to `swift.org` which tests this? [I don't think we have a way of triggering this from pure Obj-C, but I may be wrong]. Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70393/new/

[Lldb-commits] [PATCH] D70393: [lldb] Fix NSURL data formatter truncation issue in Swift

davide added a comment. Thanks again for your contribution! This change broke one test. Can you please investigate? RESULT: FAILED (0 passes, 3 failures, 0 errors, 1 skipped, 0 expected failures, 0 unexpected successes) Testing Time: 316.58s **

[Lldb-commits] [PATCH] D70409: [Docs] Add reproducer documentation.

davide accepted this revision. davide added a comment. This revision is now accepted and ready to land. Thanks for writing this up! Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70409/new/ https://reviews.llvm.org/D70409 _

[Lldb-commits] [PATCH] D65469: Remove `bugreport` command

davide added a comment. I wanted to remove it a while ago, but Jason told me he found this useful, so I would wait for his opinion. Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65469/new/ https://reviews.llvm.org/D65469

[Lldb-commits] [PATCH] D65469: Remove `bugreport` command

davide added a comment. Other than that, I have no objections. Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65469/new/ https://reviews.llvm.org/D65469 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http

[Lldb-commits] [PATCH] D65534: [clang] Change FileManager to use llvm::ErrorOr instead of null on failure

davide added a comment. Really on the lldb side, Jonas is the right person to review this patch. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65534/new/ https://reviews.llvm.org/D65534 ___ lldb-commit

[Lldb-commits] [PATCH] D65534: [clang] Change FileManager to use llvm::ErrorOr instead of null on failure

davide added a comment. [and Raphael for the clang vendor bits] Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65534/new/ https://reviews.llvm.org/D65534 ___ lldb-commits mailing list lldb-commits@lists

[Lldb-commits] [PATCH] D65547: [Reproducers] Force replay in synchronous mode.

davide accepted this revision. davide added a comment. This revision is now accepted and ready to land. Thanks! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65547/new/ https://reviews.llvm.org/D65547 ___ lldb-commits mailing list lldb-comm

[Lldb-commits] [PATCH] D65781: [CompilerType] Remove an unused function.

davide created this revision. davide added reviewers: teemperor, JDevlieghere, xiaobai, compnerd, friss. Herald added a project: LLDB. This simplifies the interface, as I'm trying to understand how we can upstream swift support. rdar://problem/36377967 Repository: rG LLVM Github Monorepo htt

[Lldb-commits] [PATCH] D65781: [CompilerType] Remove an unused function.

This revision was automatically updated to reflect the committed changes. Closed by commit rL367946: [CompilerType] Remove an unused function. (authored by davide, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llv

[Lldb-commits] [PATCH] D65782: [CompilerType] Simplify the interface a bit more..

davide created this revision. davide added reviewers: shafik, JDevlieghere, alex, compnerd, teemperor. Herald added a project: LLDB. .. removing IsMeaninglessWithoutTypeResolution(). I'm fairly confident this was introduced to support swift, where static types [without dynamic counterpart] don't c

[Lldb-commits] [PATCH] D65782: [CompilerType] Simplify the interface a bit more..

This revision was automatically updated to reflect the committed changes. Closed by commit rL367957: [CompilerType] Simplify the interface a bit more.. (authored by davide, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit: https://re

[Lldb-commits] [PATCH] D65782: [CompilerType] Simplify the interface a bit more..

davide added a comment. In D65782#1615959 , @alex wrote: > I was marked as a review on this, but I suspect you intended to get some > other Alex :-) yes, sorry. My bad. Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65782

[Lldb-commits] [PATCH] D65874: [lldb][CMake] Disable modules in Xcode projects

davide accepted this revision. davide added a comment. This revision is now accepted and ready to land. You might want to check with Argyrios, maybe file a rdar against SourceKit. Otherwise, this is fine to me. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.ll

[Lldb-commits] [PATCH] D65872: [lldb][NFC] Check in test case for testing virtual function calls in pointers and references.

davide accepted this revision. davide added a comment. This revision is now accepted and ready to land. I seem to remember there was a bug where you run the expression twice and the second time it crashes/it's not evaluated correctly [at least this is what I remember talking to Lang ~18 months a

[Lldb-commits] [PATCH] D65942: Disallow implicit conversion from pointers to bool in llvm::toStringRef

davide added a comment. I think this is definitely a good idea but Chandler or DBlaikie might want to take a look at this one too. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65942/new/ https://reviews.llvm.org/D65942 ___ lldb-commits ma

[Lldb-commits] [PATCH] D65874: [lldb][CMake] Disable modules in Xcode projects

davide added a comment. Did you manage to file a rdar for this issue? If so, can you put the link here for future archeology? Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65874/new/ https://reviews.llvm.org/D65874 _

[Lldb-commits] [PATCH] D66093: [Symbol] GetTypeBitAlign() should return None in case of failure.

davide created this revision. davide added reviewers: xiaobai, compnerd. Herald added a project: LLDB. davide added a reviewer: aprantl. And not `zero`. This is the last API needed to be converted to an Optional. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D66093 Files: ll

[Lldb-commits] [PATCH] D66093: [Symbol] GetTypeBitAlign() should return None in case of failure.

davide added a comment. More than error handling, this allows us to distinguish with a type with zero alignment (if possible at all?) and an error getting the alignment from ASTContext/ Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66093/new/ htt

[Lldb-commits] [PATCH] D66093: [Symbol] GetTypeBitAlign() should return None in case of failure.

davide marked an inline comment as done. davide added inline comments. Comment at: lldb/source/Expression/Materializer.cpp:803 + + size_t byte_align = (*opt_bit_align + 7) / 8; aprantl wrote: > does this also need to be 7ull? I'm going to move this to a fu

<    5   6   7   8   9   10   11   12   >