[Lldb-commits] [lldb] r350682 - [CMakeLists] Sort tools/CMakeLists.txt

Author: jdevlieghere Date: Tue Jan 8 16:31:30 2019 New Revision: 350682 URL: http://llvm.org/viewvc/llvm-project?rev=350682=rev Log: [CMakeLists] Sort tools/CMakeLists.txt Modified: lldb/trunk/tools/CMakeLists.txt Modified: lldb/trunk/tools/CMakeLists.txt URL:

[Lldb-commits] [PATCH] D56462: Change SymbolFile::ParseTypes to ParseTypesForCompileUnit

zturner created this revision. zturner added reviewers: clayborg, labath, davide, jingham. Herald added a subscriber: JDevlieghere. The function `SymbolFile::ParseTypes` previously accepted a `SymbolContext`. This makes it extremely difficult to implement faithfully, because you have to

[Lldb-commits] [PATCH] D56461: [NativePDB] Add support for parsing typedefs and make lldb-test work with the native reader.

zturner created this revision. zturner added reviewers: aleksandr.urakov, amccarth, lemo, labath. Herald added a subscriber: aprantl. Typedefs are represented as `S_UDT` records in the globals stream. This creates a strange situation where "types" are actually represented as "symbols", so they

[Lldb-commits] [lldb] r350679 - Change std::sort to llvm::sort to detect non-determinism.

Author: jdevlieghere Date: Tue Jan 8 15:25:06 2019 New Revision: 350679 URL: http://llvm.org/viewvc/llvm-project?rev=350679=rev Log: Change std::sort to llvm::sort to detect non-determinism. LLVM added wrappers to std::sort (r327219) that randomly shuffle the container before sorting. The goal

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

clayborg added a comment. In D56425#1350253 , @JDevlieghere wrote: > In D56425#1350236 , @JDevlieghere > wrote: > > > In D56425#1350234 , @clayborg > > wrote: > > > > >

[Lldb-commits] [PATCH] D56458: Fix unused private field warning.

This revision was not accepted when it landed; it landed in state "Needs Review". This revision was automatically updated to reflect the committed changes. Closed by commit rL350675: Fix unused private field warning. (authored by teemperor, committed by ). Herald added a subscriber:

[Lldb-commits] [lldb] r350675 - Fix unused private field warning.

Author: teemperor Date: Tue Jan 8 14:55:02 2019 New Revision: 350675 URL: http://llvm.org/viewvc/llvm-project?rev=350675=rev Log: Fix unused private field warning. Summary: The member is private and unused if HAVE_LIBCOMPRESSION is undefined, which triggers Clang's -Wunused-private-field

[Lldb-commits] [PATCH] D56230: [gdb-remote] Use lldb's portable Host::GetEnvironment() instead of getenv

Hui added inline comments. Comment at: source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp:1138 + // Double quote the string. + ::snprintf(arg_cstr, sizeof(arg_cstr), "--log-channels=\"%s\"", + env_debugserver_log_channels.c_str());

[Lldb-commits] [PATCH] D56418: Change lldb-test to use ParseAllDebugSymbols instead of ParseDeclsForContext

zturner marked an inline comment as done. zturner added inline comments. Comment at: lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp:318 + + TypeSystem *type_system = GetTypeSystemForLanguage(lldb::eLanguageTypeC_plus_plus); + if (!type_system) amccarth

[Lldb-commits] [PATCH] D56458: Fix unused private field warning.

teemperor created this revision. Herald added a subscriber: lldb-commits. The member is private and unused if HAVE_LIBCOMPRESSION is undefined, which triggers Clang's -Wunused-private-field warning. Repository: rLLDB LLDB https://reviews.llvm.org/D56458 Files:

[Lldb-commits] [PATCH] D56418: Change lldb-test to use ParseAllDebugSymbols instead of ParseDeclsForContext

amccarth added inline comments. Comment at: lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp:318 + + TypeSystem *type_system = GetTypeSystemForLanguage(lldb::eLanguageTypeC_plus_plus); + if (!type_system) Is it appropriate to hard code that language type

[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

[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=180735#toc Repository:

[Lldb-commits] [lldb] r350659 - [BreakpointList] Simplify/modernize BreakpointList (NFC)

Author: jdevlieghere Date: Tue Jan 8 14:07:42 2019 New Revision: 350659 URL: http://llvm.org/viewvc/llvm-project?rev=350659=rev Log: [BreakpointList] Simplify/modernize BreakpointList (NFC) I was looking at the code in BreakpointList.cpp and found it deserved a quick cleanup. - Use

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

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

clayborg accepted this revision. clayborg added a comment. 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 would need to insert it sorted, then we can get better than O(n) performance on

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

aprantl accepted this revision. aprantl added inline comments. This revision is now accepted and ready to land. Comment at: source/Breakpoint/BreakpointList.cpp:49 + m_breakpoints.begin(), m_breakpoints.end(), + [&](const BreakpointSP ) { return bp->GetID() ==

[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 _sp : m_breakpoints)

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

[Lldb-commits] [lldb] r350652 - [PdbAstBuilder] Remove unused functions

Author: jdevlieghere Date: Tue Jan 8 12:58:54 2019 New Revision: 350652 URL: http://llvm.org/viewvc/llvm-project?rev=350652=rev Log: [PdbAstBuilder] Remove unused functions PdbAstBuilder.cpp:273:20: warning: unused function 'GetParentUniqueName' [-Wunused-function] PdbAstBuilder.cpp:267:13:

[Lldb-commits] [lldb] r350651 - Convert to LLDB coding style (NFC)

Author: adrian Date: Tue Jan 8 12:48:40 2019 New Revision: 350651 URL: http://llvm.org/viewvc/llvm-project?rev=350651=rev Log: Convert to LLDB coding style (NFC) Modified: lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp Modified:

[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

[Lldb-commits] [PATCH] D56443: [CMake] In standalone builds, LLVM_BINARY_DIR should point to LLVM's binary directory

labath accepted this revision. labath added a comment. This revision is now accepted and ready to land. Makes sense to me. Comment at: CMakeLists.txt:117 # For now check that the include directory exists. -set(cxx_dir "${LLVM_DIR}/../../../include/c++") +

[Lldb-commits] [PATCH] D56443: [CMake] In standalone builds, LLVM_BINARY_DIR should point to LLVM's binary directory

sgraenitz updated this revision to Diff 180683. sgraenitz added a comment. Use LLVM_BINARY_DIR when checking for existance of libc++ include directory CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56443/new/ https://reviews.llvm.org/D56443 Files: CMakeLists.txt

[Lldb-commits] [PATCH] D56399: [CMake] Fix standalone builds: workaround the cxx target not getting imported yet (unlike clang target)

sgraenitz updated this revision to Diff 180679. sgraenitz added a comment. Update documentation: libc++ should be checked out when building for macOS CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56399/new/ https://reviews.llvm.org/D56399 Files: CMakeLists.txt www/build.html

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

sgraenitz marked 7 inline comments as done. sgraenitz 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!

[Lldb-commits] [PATCH] D55998: ELF: create "container" sections from PT_LOAD segments

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

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

clayborg added inline comments. Comment at: source/Breakpoint/BreakpointList.cpp:22 +Target::eBroadcastBitBreakpointChanged, +new Breakpoint::BreakpointEventData(event, bp)); +} If we don't pass a "BreakpointSP& sp", then use std::move?

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

clayborg added a comment. Second vote from me to switch to a std::vector since we have a function get breakpoint at index. Other than that, looks good. Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56425/new/ https://reviews.llvm.org/D56425

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

aprantl added inline comments. Comment at: source/Breakpoint/BreakpointList.cpp:18 +static void NotifyChange(BreakpointSP bp, BreakpointEventType event) { + if (bp->GetTarget().EventTypeHasListeners( + Target::eBroadcastBitBreakpointChanged)) perhaps:

[Lldb-commits] [PATCH] D56443: [CMake] In standalone builds, LLVM_BINARY_DIR should point to LLVM's binary directory

sgraenitz created this revision. sgraenitz added reviewers: zturner, labath, clayborg, JDevlieghere, stella.stamenova. Herald added a subscriber: mgorny. Herald added a reviewer: serge-sans-paille. In standalone builds `LLVM_BINARY_DIR` was equal to `LLDB_BINARY_DIR` so far. This is

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

sgraenitz updated this revision to Diff 180670. sgraenitz added a comment. Remove remove unused `LLDB_HAVE_LLD` and `ENABLE_SHARED` in lit/CMakeLists.txt CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56440/new/ https://reviews.llvm.org/D56440 Files: CMakeLists.txt

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

sgraenitz updated this revision to Diff 180662. sgraenitz added a comment. In test/CMakeLists.txt pass on LLDB_TEST_COMPILER_USED instead of LLDB_TEST_COMPILER CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56440/new/ https://reviews.llvm.org/D56440 Files: CMakeLists.txt

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

sgraenitz created this revision. sgraenitz added reviewers: labath, zturner, ted, JDevlieghere, stella.stamenova. Herald added a subscriber: mgorny. Following up from D56400 : - add `LLDB_TEST_COMPILER` setting, while still supporting `LLDB_TEST_C/CXX_COMPILER`

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

sgraenitz updated this revision to Diff 180643. sgraenitz added a comment. Act on FIXME's in subsequent commits and remove comments here CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56400/new/ https://reviews.llvm.org/D56400 Files: CMakeLists.txt lit/CMakeLists.txt Index:

[Lldb-commits] [PATCH] D55998: ELF: create "container" sections from PT_LOAD segments

labath added a comment. Greg, what do you think about the current version of this patch. Joerg, I propose to check the patch in in the current form, as this solves existing issues in the lldb's ELF representation and improves the overall consistency of lldb's representation of object files.

[Lldb-commits] [PATCH] D56196: ProcessLaunchInfo: Remove Target reference

This revision was automatically updated to reflect the committed changes. Closed by commit rL350617: ProcessLaunchInfo: Remove Target reference (authored by labath, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM CHANGES SINCE LAST ACTION

[Lldb-commits] [lldb] r350617 - ProcessLaunchInfo: Remove Target reference

Author: labath Date: Tue Jan 8 03:55:19 2019 New Revision: 350617 URL: http://llvm.org/viewvc/llvm-project?rev=350617=rev Log: ProcessLaunchInfo: Remove Target reference Summary: The target was being used in FinalizeFileActions to provide default values for stdin/out/err. Also, most of the

Re: [Lldb-commits] [PATCH] D56418: Change lldb-test to use ParseAllDebugSymbols instead of ParseDeclsForContext

On 08/01/2019 11:45, Zachary Turner wrote: The same plugin is being used in both cases, the patch only touches the nonnative PDB reader. The followup changes to the plugin itself are necessary because the results were not identical (another reason i want to de-support ParseDeclsForContext at

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

sgraenitz 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! labath wrote: >

Re: [Lldb-commits] [PATCH] D56418: Change lldb-test to use ParseAllDebugSymbols instead of ParseDeclsForContext

The same plugin is being used in both cases, the patch only touches the nonnative PDB reader. The followup changes to the plugin itself are necessary because the results were not identical (another reason i want to de-support ParseDeclsForContext at global acope - having 2 implementations of the

[Lldb-commits] [PATCH] D56418: Change lldb-test to use ParseAllDebugSymbols instead of ParseDeclsForContext

labath accepted this revision. labath added a comment. This revision is now accepted and ready to land. The lldb-test change itself looks fine. I don't feel like I know the full impact of the proposed follow-up changes (but they sounds reasonable). Comment at:

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

labath added a reviewer: stella.stamenova. labath added a comment. This looks fine to me. I recommend including Stella in changes relating to this, as I recall some subtleties here wrt. multi-config (e.g., MSVC) generators. Comment at: CMakeLists.txt:43

[Lldb-commits] [PATCH] D56232: [lldb-server] Add remote platform capabilities for Windows

labath added a comment. In D56232#1348514 , @zturner wrote: > Is that even necessary? If a platform is not remote aware, `IsHost()` will > always just return `true` by definition. So we could put all of this in the > existing `Platform` base class.

[Lldb-commits] [PATCH] D56233: [lldb-server] Add initial support for building lldb-server on Windows

labath added inline comments. Comment at: include/lldb/Host/windows/PosixApi.h:78-79 +#define WNOHANG 1 +#define WUNTRACED 2 + zturner wrote: > krytarowski wrote: > > I think that these symbols should not be leaked here in the first place. > In general we