[Lldb-commits] [PATCH] D45977: Always normalize FileSpec paths.

aprantl added a comment. One general question: why is this form of normalization preferred over calling realpath? https://reviews.llvm.org/D45977 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listin

[Lldb-commits] [PATCH] D45977: Always normalize FileSpec paths.

aprantl added inline comments. Comment at: source/Utility/FileSpec.cpp:107 + for (auto i = path.find('/'); i != llvm::StringRef::npos; + i = path.find('/', i + 1)) { +const auto nextChar = safeCharAtIndex(path, i+1); clayborg wrote: > I am fine switchi

[Lldb-commits] [PATCH] D45977: Always normalize FileSpec paths.

aprantl added inline comments. Comment at: source/Utility/FileSpec.cpp:107 + for (auto i = path.find('/'); i != llvm::StringRef::npos; + i = path.find('/', i + 1)) { +const auto nextChar = safeCharAtIndex(path, i+1); clayborg wrote: > clayborg wrote: >

[Lldb-commits] [PATCH] D45977: Always normalize FileSpec paths.

aprantl added a comment. > Also not sure if the LLVM stuff will try to substitute in the current working > directory for "."? No it won't, remove_dots() has no side effects. There's a separate make_absolute() function. That one will cause another copy. Personally, if I have a choice between a

[Lldb-commits] [PATCH] D45977: Always normalize FileSpec paths.

aprantl added a comment. I'll take that back: make_absolute only copies when the path isn't already absolute. https://reviews.llvm.org/D45977 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/l

[Lldb-commits] [PATCH] D45977: Always normalize FileSpec paths.

aprantl added inline comments. Comment at: source/Utility/FileSpec.cpp:241 } + llvm::sys::path::remove_dots(resolved, true, LLVMPathSyntax(syntax)); `// Normalize the path by removing './' and other redundant components.` Comment at: sour

[Lldb-commits] [PATCH] D46083: Move the persistent variable counter into Target

aprantl created this revision. aprantl added a reviewer: jingham. Move the persistent variable counter into Target so it can be shared across multiple language plugins. In a multi-language project it is counterintuitive to have a result variables reuse numbers just because they are using

[Lldb-commits] [PATCH] D46088: Refactor GetNextPersistentVariableName into a non-virtual method

aprantl created this revision. aprantl added a reviewer: jingham. Refactor GetNextPersistentVariableName into a non-virtual method that takes a prefix string. This simplifies the implementation and allows plugins such as the Swift plugin to supply different prefixes for return and error variables.

[Lldb-commits] [PATCH] D46144: Reflow paragraphs in comments.

aprantl created this revision. aprantl added reviewers: clayborg, jingham, jasonmolenda, labath. Herald added subscribers: mgrang, JDevlieghere, kbarton, arichardson, nemanjai, kubamracek, emaste, srhines. Herald added a reviewer: javed.absar. Herald added a reviewer: espindola. This is intended

[Lldb-commits] [PATCH] D46128: Fix expression parser to not accept any type whose basename matches for a type that must exist at root level

aprantl added inline comments. Comment at: packages/Python/lldbsuite/test/lang/cpp/type_lookup/TestTypeLookup.py:41 + +# Make sure we don't accedentally accept structures that exist only +# in namespaces when evaluating expressions with top level types. -

[Lldb-commits] [PATCH] D46220: Remove premature caching of the global variables list in CompileUnit.

aprantl created this revision. aprantl added reviewers: clayborg, jingham. Herald added a subscriber: JDevlieghere. Remove premature caching of the global variables list in CompileUnit. This is fixing a bug where (lldb) target var g_ptr would propulate the global variables list with exactl

[Lldb-commits] [PATCH] D46220: Remove premature caching of the global variables list in CompileUnit.

aprantl added a comment. This also adds test coverage for r224559, which is how I discovered this bug in the first place. https://reviews.llvm.org/D46220 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailma

[Lldb-commits] [PATCH] D46088: Refactor GetNextPersistentVariableName into a non-virtual method

aprantl added a comment. ping https://reviews.llvm.org/D46088 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D46083: Move the persistent variable counter into Target

aprantl added a comment. ping. https://reviews.llvm.org/D46083 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D46144: Reflow paragraphs in comments.

aprantl updated this revision to Diff 144420. aprantl added a comment. Updated the script based on Pavel's feedback: import textwrap import commands import os import sys import re tmp = "%s.tmp"%sys.argv[1] out = open(tmp, "w+") with open(sys.argv[1], "r") as f: header = ""

[Lldb-commits] [PATCH] D46144: Reflow paragraphs in comments.

aprantl added inline comments. Comment at: source/Host/common/MainLoop.cpp:158 // ppoll(2) is not supported on older all android versions. Also, older - // versions android (API <= 19) implemented pselect in a non-atomic way, as a - // combination of pthread_sigmask and sele

[Lldb-commits] [PATCH] D46083: Move the persistent variable counter into Target

aprantl updated this revision to Diff 144628. aprantl added a comment. Thread an is_error flag through for plugins that need it. https://reviews.llvm.org/D46083 Files: include/lldb/Expression/ExpressionVariable.h source/Core/ValueObject.cpp source/Expression/ExpressionVariable.cpp sourc

[Lldb-commits] [PATCH] D46220: Remove premature caching of the global variables list in CompileUnit.

aprantl added a comment. ping. https://reviews.llvm.org/D46220 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D46128: Fix expression parser to not accept any type whose basename matches for a type that must exist at root level

aprantl accepted this revision. aprantl added a comment. This revision is now accepted and ready to land. To aid with reviewing I documented LLDB's current behavior. I don't understand the implementation well enough to comment on it, but I do think the the new behavior is superior over the old o

[Lldb-commits] [PATCH] D46220: Remove premature caching of the global variables list in CompileUnit.

aprantl added inline comments. Comment at: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:4221 if (variable_list_sp.get() == NULL) { variable_list_sp.reset(new VariableList()); } clayborg wrote: > So do we cache

[Lldb-commits] [PATCH] D46083: Move the persistent variable counter into Target

aprantl added a comment. I'm sorry: which inline comment? Did you perhaps forget to hit "Submit"? https://reviews.llvm.org/D46083 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D46088: Refactor GetNextPersistentVariableName into a non-virtual method

aprantl updated this revision to Diff 144649. aprantl added a comment. Address review feedback. https://reviews.llvm.org/D46088 Files: include/lldb/Expression/ExpressionVariable.h source/Core/ValueObject.cpp source/Expression/ExpressionVariable.cpp source/Expression/Materializer.cpp s

[Lldb-commits] [PATCH] D46220: Remove premature caching of the global variables list in CompileUnit.

aprantl added inline comments. Comment at: lldb/trunk/packages/Python/lldbsuite/test/lang/c/global_variables/TestGlobalVariables.py:63-65 +self.runCmd("frame variable --show-types --scope --show-globals --no-args") self.expect( "frame variable --sh

[Lldb-commits] [PATCH] D46362: DWARFExpression: Convert file addresses to load addresses early on

aprantl created this revision. aprantl added a reviewer: clayborg. Herald added a subscriber: JDevlieghere. This is a change that only affects Swift and is NFC for the language plugins on llvm.org. In Swift, we can have global variables with a location such as DW_OP_addr DW_OP_deref. The DWARF

[Lldb-commits] [PATCH] D46334: [CMake] Unify and relayer testing

aprantl added a comment. Is there a way to land a partial version of this patch to un-block the green dragon bots and keep iterating on the check-single target separately? Or do other bots depend on check-single working correctly? I'm really uneasy about the fact that we are loosing signal from

[Lldb-commits] [PATCH] D46334: [CMake] Unify and relayer testing

aprantl added a comment. If that's okay with you I would suggest landing this as is to unblock the bots, and we'll keep working on fixing the check-single target ASAP, too. https://reviews.llvm.org/D46334 ___ lldb-commits mailing list lldb-commits@

[Lldb-commits] [PATCH] D46334: [CMake] Unify and relayer testing

aprantl added a comment. > If by as-is you mean with a big comment in the config file before the > creation of check-single that it is broken, then yes, let's get the bots > unblocked and then fix the issue. Sounds good. Thanks! https://reviews.llvm.org/D46334 _

[Lldb-commits] [PATCH] D46362: DWARFExpression: Convert file addresses to load addresses early on

aprantl updated this revision to Diff 145032. aprantl added a comment. Thanks for the excellent feedback! Addressed comments. https://reviews.llvm.org/D46362 Files: include/lldb/Core/Value.h source/Core/Value.cpp source/Core/ValueObjectVariable.cpp source/Expression/DWARFExpression.cpp

[Lldb-commits] [PATCH] D46362: DWARFExpression: Convert file addresses to load addresses early on

aprantl added a comment. Yes, I must have added a mistake in my last edit before landing this. I reverted that patch for now. Repository: rL LLVM https://reviews.llvm.org/D46362 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://li

[Lldb-commits] [PATCH] D46362: DWARFExpression: Convert file addresses to load addresses early on

aprantl added a comment. As Greg suspected, the problem was that target was null. I rewrote that patch to not use a SymbolContext in the first place. Repository: rL LLVM https://reviews.llvm.org/D46362 ___ lldb-commits mailing list lldb-commits@

[Lldb-commits] [PATCH] D46606: General cleanup to minimize the .debug_types patch

aprantl accepted this revision. aprantl added a comment. This revision is now accepted and ready to land. Looks like this is bringing the interface a *little* closer to the llvm one. Comment at: source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.h:40 + //

[Lldb-commits] [PATCH] D46588: [LLDB][lldb-mi] Add possibility to set breakpoints without selecting a target.

aprantl added a comment. In https://reviews.llvm.org/D46588#1092884, @labath wrote: > Out of curiosity, are there any plans to improve the lldb-mi test reliability > situation? As it stands now, most of the lldb-mi tests are disabled one way > or another due to them being flaky. Thanks for br

[Lldb-commits] [PATCH] D46588: [LLDB][lldb-mi] Add possibility to set breakpoints without selecting a target.

aprantl added a comment. Okay, that's no good. What would you think about writing tests using lit & FileCheck? The lldb-mi tests should not be in the debuginfo category anyway, since they only test the alternative driver. $ cat lldb-mi-demo.input -file-exec-and-symbols a.out -break-insert

[Lldb-commits] [PATCH] D32167: Add support for type units (.debug_types) to LLDB in a way that is compatible with DWARF 5

aprantl added inline comments. Comment at: packages/Python/lldbsuite/test/make/Makefile.rules:197 +ifeq "$(DWARF_TYPE_UNITS)" "YES" + DEBUG_INFO_FLAG ?= -gdwarf-4 +else This shouldn't be necessary on any platform LLDB cares about. DWARF 4 should be the def

[Lldb-commits] [PATCH] D35740: Fix PR33875 by distinguishing between DWO and clang modules

aprantl created this revision. The DWO handling code can get confused by clang modules which also use skeleton CUs to point to the object file with the full debug info. This patch detects whether an object is a "real" DWO or a clang module and prevents LLDB from interpreting clang modules as DW

[Lldb-commits] [PATCH] D35740: Fix PR33875 by distinguishing between DWO and clang modules

aprantl updated this revision to Diff 107734. aprantl added a comment. No with testcase of sorts. https://reviews.llvm.org/D35740 Files: packages/Python/lldbsuite/test/lang/cpp/gmodules/TestWithModuleDebugging.py source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp Index: source/Plugins

[Lldb-commits] [PATCH] D56543: DWARF: Add some support for non-native directory separators

aprantl added inline comments. Comment at: source/Plugins/SymbolFile/DWARF/DWARFUnit.h:255 + void ComputePathStyle(); + I would probably call this DetectPathStyle() since it's more a heuristic? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56543/new/

[Lldb-commits] [PATCH] D56688: Make CompilerType::getBitSize() / getByteSize() return an optional result. (NFC)

aprantl marked an inline comment as done. aprantl added a comment. I changed all the autos back to full types in r351237. Comment at: lldb/trunk/source/Plugins/ABI/MacOSX-i386/ABIMacOSX_i386.cpp:827 CompilerType compiler_type(value->GetCompilerType()); -if (compiler_ty

[Lldb-commits] [PATCH] D56589: Teach the default symbol vendor to respect module.GetSymbolFileFileSpec()

aprantl added a comment. Looks like this test is failing on green dragon: http://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/17221/consoleFull#-97272088254eaf0-7326-4999-85b0-388101f2d404 FAIL: LLDB :: SymbolFile/Breakpad/symtab-macho.test (126 of 1452) TEST 'LLDB

[Lldb-commits] [PATCH] D56798: Change TypeSystem::GetBitSize() to return an optional result.

aprantl created this revision. aprantl added reviewers: labath, zturner, jingham, shafik. Herald added a reviewer: serge-sans-paille. This patch changes the behavior when printing C++ function references: where we previously would get a ``, there is now a ``. It's not clear to me whether this is

[Lldb-commits] [PATCH] D57091: [www] Remove implemented Open Project

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

[Lldb-commits] [PATCH] D57213: [Scalar] Add support for 512-bits values.

aprantl added inline comments. Comment at: lldb/source/Utility/RegisterValue.cpp:159 case 32: +case 64: if (buffer.length % sizeof(uint64_t) == 0) { I'm curious, everything else in this patch refers to 512, how does this fit in?

[Lldb-commits] [PATCH] D57272: Remove unimplemented function

aprantl created this revision. aprantl added reviewers: jasonmolenda, jingham. Looks like this was an unintended sideeffect of r124250. https://reviews.llvm.org/D57272 Files: source/Symbol/Type.cpp Index: source/Symbol/Type.cpp ===

[Lldb-commits] [PATCH] D57273: Make Type::GetByteSize optional

aprantl created this revision. aprantl added reviewers: jingham, labath. This is a continuation of my quest to make the size 0 a supported value. https://reviews.llvm.org/D57273 Files: include/lldb/Symbol/Type.h source/Core/ValueObjectMemory.cpp source/Expression/Materializer.cpp source

[Lldb-commits] [PATCH] D57273: Make Type::GetByteSize optional

aprantl marked an inline comment as done. aprantl added inline comments. Comment at: include/lldb/Symbol/Type.h:221 + uint64_t m_byte_size : 63; + unsigned m_byte_size_has_value : 1; Declaration m_decl; clayborg wrote: > Does this need to be uint64_t to shar

[Lldb-commits] [PATCH] D57273: Make Type::GetByteSize optional

aprantl added a comment. Pavel, I may need you help. I reverted the commit because it broke three PDB-related tests (SymbolFilePDBTests, func-symbols, and typedefs) that need more than just lld in order to run. The fix is probably trivial. We only need to set a breakpoint in `Type.cpp:119` and

[Lldb-commits] [PATCH] D57363: Fix handling of CreateTemplateParameterList when there is an empty pack

aprantl added inline comments. Comment at: packages/Python/lldbsuite/test/expression_command/radar_47565290/TestClassTemplateSpecializationParametersHandling.py:9 +import os +import time +import lldb I don't think these are used Comment at: p

[Lldb-commits] [PATCH] D57363: Fix handling of CreateTemplateParameterList when there is an empty pack

aprantl added inline comments. Comment at: packages/Python/lldbsuite/test/expression_command/radar_47565290/Makefile:1 +LEVEL = ../../make + Could you give the directory a more descriptive name instead of `radar_47565290`? It's fine to mention a rdar:// link in

[Lldb-commits] [PATCH] D57363: Fix handling of CreateTemplateParameterList when there is an empty pack

aprantl added inline comments. Comment at: packages/Python/lldbsuite/test/expression_command/radar_47565290/TestClassTemplateSpecializationParametersHandling.py:23 + +(self.target, self.process, _, bkpt) = lldbutil.run_to_source_breakpoint(self, '// break here', +

[Lldb-commits] [PATCH] D57273: Make Type::GetByteSize optional

aprantl added a comment. Thanks a bunch, Pavel! That is very helpful. Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57273/new/ https://reviews.llvm.org/D57273 ___ lldb-commits mailing list lldb-commits@lists.llvm.org

[Lldb-commits] [PATCH] D57467: Fix use of non-existing variable in crashlog.py

aprantl added a comment. Thanks!! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57467/new/ https://reviews.llvm.org/D57467 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commit

[Lldb-commits] [PATCH] D57781: Fix strlen() of unbound array undefined behavior

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

[Lldb-commits] [PATCH] D58090: [WIP] Deserialize Clang module search path from DWARF

aprantl created this revision. aprantl added reviewers: teemperor, JDevlieghere. This is an unfinished patch I'm working on that Raphael may find interesting. Posting to get early feedback and to avoid stepping on each others' toes. rdar://problem/47970144 https://reviews.llvm.org/D58090 File

[Lldb-commits] [PATCH] D58090: Deserialize Clang module search path from DWARF

aprantl updated this revision to Diff 186571. aprantl added a comment. Herald added a reviewer: serge-sans-paille. Almost final version. There's a silly bug that prevents the C Darwin module to be loaded now that I haven't been able to track down yet. CHANGES SINCE LAST ACTION https://reviews

[Lldb-commits] [PATCH] D58090: Deserialize Clang module search path from DWARF

aprantl added a comment. I think the problem was that I had an incompletely built libcxx in my build directory. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58090/new/ https://reviews.llvm.org/D58090 ___ lldb-commits mailing list lldb-com

[Lldb-commits] [PATCH] D58090: Deserialize Clang module search path from DWARF

aprantl added a comment. I attempted to fix it in r353971, but I'm not sure. Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58090/new/ https://reviews.llvm.org/D58090 ___ lldb-commits mailing list lldb-commits@lists

[Lldb-commits] [PATCH] D58125: Add ability to import std module into expression parser to improve C++ debugging

aprantl added inline comments. Comment at: lldb/packages/Python/lldbsuite/test/expression_command/import-std-module/conflicts/Makefile:3 +USE_LIBCPP := 1 +CXXFLAGS += -std=c++11 -fmodules -glldb -fimplicit-module-maps +CXX_SOURCES := main.cpp Makefile.rules defi

[Lldb-commits] [PATCH] D58309: [Reproducers] Have clang make use of lldb's VFS.

aprantl added inline comments. Comment at: lldb/lit/Reproducer/Modules/Inputs/main.cpp:5 + Foo foo; + // break here. + return 0; This is not very robust, since there is no code on this line. Personally I'd use: ``` void stop() {} int main(int argc, char **a

[Lldb-commits] [PATCH] D58309: [Reproducers] Have clang make use of lldb's VFS.

aprantl added inline comments. Comment at: lldb/lit/Reproducer/Modules/TestModuleCXX.test:17 +# Capture the debug session. +# RUN: %lldb -x -b -s %S/Inputs/ModuleCXX.in --capture %t.repro %t.root/a.out | FileCheck %s --check-prefix CAPTURE +# CAPTURE: (success = 0) -

[Lldb-commits] [PATCH] D58309: [Reproducers] Have clang make use of lldb's VFS.

aprantl accepted this revision. aprantl added a comment. This revision is now accepted and ready to land. LGTM as long as we always set a custom module cache path. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58309/new/ https://reviews.llvm.org/D58309 ___

[Lldb-commits] [PATCH] D58125: Add ability to import std module into expression parser to improve C++ debugging

aprantl added inline comments. Comment at: lldb/packages/Python/lldbsuite/test/expression_command/import-std-module/conflicts/Makefile:3 +USE_LIBCPP := 1 +CXXFLAGS += -std=c++11 -fmodules -glldb -fimplicit-module-maps +CXX_SOURCES := main.cpp teemperor wrote: >

[Lldb-commits] [PATCH] D58405: Make educated guess when constructing module from memory

aprantl added a comment. > a good guess is the triple that the target executable was compiled for. What does this do when the executable has multiple slices, such as a Mach-O universal binary? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58405/new/ https://reviews.llvm.org/D58405

[Lldb-commits] [PATCH] D58405: Merge target triple into module triple when constructing module from memory

aprantl added a comment. Can you think of a way to test this? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58405/new/ https://reviews.llvm.org/D58405 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/

[Lldb-commits] [PATCH] D58405: Merge target triple into module triple when constructing module from memory

aprantl added a comment. Perhaps a unit test? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58405/new/ https://reviews.llvm.org/D58405 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo

[Lldb-commits] [PATCH] D58564: [Reproducers] Add command provider

aprantl added inline comments. Comment at: lldb/include/lldb/Interpreter/CommandInterpreter.h:31 +class CommandProvider; +struct CommandProviderInfo { + static const char *name; Doxygen comments? Comment at: lldb/include/lldb/Interpreter/Comma

[Lldb-commits] [PATCH] D58566: [Reproducers] Add more logging capabilities to reproducer instrumentation

aprantl added inline comments. Comment at: lldb/include/lldb/Utility/ReproducerInstrumentation.h:591 -LLDB_LOG(GetLogIfAllCategoriesSet(LIBLLDB_LOG_API), "Recording ({0}) '{1}'", +#ifndef LLDB_REPRO_INSTR_TRACE +LLDB_LOG(GetLogIfAllCategoriesSet(LIBLLDB_LOG_API), "Reco

[Lldb-commits] [PATCH] D58664: [Utility] Fix ArchSpec.MergeFrom to correctly merge environments

aprantl added a comment. Could you please add more context to the diff? Something like `git diff -U HEAD~1` works fine. Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58664/new/ https://reviews.llvm.org/D58664

[Lldb-commits] [PATCH] D58653: [Utility] Remove Triple{Environment, OS, Vendor}IsUnspecifiedUnknown from ArchSpec

aprantl added a comment. @clayborg I recently ran into a similar issue and I think that perhaps adding explicit `llvm::Triple::Any{Vendor|OS|...}` enumerators to llvm::Triple to make this distinction explicit would be the cleanest solution. CHANGES SINCE LAST ACTION https://reviews.llvm.org/

[Lldb-commits] [PATCH] D58720: Remove unnecessary demangling operation (hopefully NFC)

aprantl created this revision. aprantl added reviewers: sgraenitz, jingham. Herald added a subscriber: erik.pilkington. Herald added a project: LLDB. This extra call to the demangler doesn't affect the performance of C++ because the result is being cached anyway; but I'm working on a patch to the

[Lldb-commits] [PATCH] D58790: Adding test to cover the correct import of SourceLocation pertaining to a built-in during expression parsing

aprantl added inline comments. Comment at: packages/Python/lldbsuite/test/expression_command/import_builtin_fileid/TestImportBuiltinFileID.py:17 + +@skipUnlessDarwin +@add_test_categories(["gmodules"]) The Cocoa framework only exists on macOS, there's a

[Lldb-commits] [PATCH] D58790: Adding test to cover the correct import of SourceLocation pertaining to a built-in during expression parsing

aprantl added inline comments. Comment at: packages/Python/lldbsuite/test/expression_command/import_builtin_fileid/TestImportBuiltinFileID.py:18 +@skipUnlessDarwin +@add_test_categories(["gmodules"]) +def test_import_builtin_fileid(self): aprantl wro

[Lldb-commits] [PATCH] D58838: Remove tautological #ifdefs

aprantl created this revision. aprantl added a reviewer: clayborg. These conditions appear to always be true. @clayborg do you remember the reason why these exist? https://reviews.llvm.org/D58838 Files: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp lldb/source/Target/Executi

[Lldb-commits] [PATCH] D58838: Remove tautological #ifdefs

aprantl added a comment. But I'm sure that `LLDB_CONFIGURATION_BUILD_AND_INTEGRATION` also defines `NDEBUG` so the assert should be a noop, right? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58838/new/ https://reviews.llvm.org/D58838 ___

[Lldb-commits] [PATCH] D58838: Remove tautological #ifdefs

aprantl added a comment. This doesn't apply to DieStack since it isn't an assert, but DieStack is dead code anyway... CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58838/new/ https://reviews.llvm.org/D58838 ___ lldb-commits mailing list ll

[Lldb-commits] [PATCH] D58838: Remove tautological #ifdefs

aprantl added a comment. In D58838#1415637 , @zturner wrote: > Yea it would be nice if we could remove all of the `LLDB_CONFIGURATION_xxx` > macros and just use either the LLVM ones or standard ones such as NDEBUG I generally agree with this, but we nee

[Lldb-commits] [PATCH] D58838: Remove tautological #ifdefs

aprantl added a comment. Also I should note that we can only use LLVM defines that are provided by the Xcode project until we reach the point where Apple builds LLDB with CMake and the CMake-generated Xcode file is good enough to replace the hand-written one. This is a transitional restriction,

[Lldb-commits] [PATCH] D58838: Remove tautological #ifdefs

aprantl updated this revision to Diff 189003. aprantl added a comment. Removed DIEStack from this patch it distracts. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58838/new/ https://reviews.llvm.org/D58838 Files: lldb/source/Target/ExecutionContext.cpp Index: lldb/source/Target/Ex

[Lldb-commits] [PATCH] D58856: Delete dead code

aprantl created this revision. aprantl added reviewers: clayborg, jingham. This isn't used anywhere and since we have version control, I don't see a reason to keep it around. https://reviews.llvm.org/D58856 Files: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp Index: lldb/sou

[Lldb-commits] [PATCH] D58125: Add ability to import std module into expression parser to improve C++ debugging

aprantl added inline comments. Comment at: lldb/include/lldb/Target/Platform.h:283 + /// + /// @param[in] lang + /// The language for which the include directories should be queried. `\param` Comment at: lldb/packages/Python/lldbsuite/te

[Lldb-commits] [PATCH] D58125: Add ability to import std module into expression parser to improve C++ debugging

aprantl added inline comments. Comment at: lldb/packages/Python/lldbsuite/test/expression_command/import-std-module/conflicts/TestStdModuleWithConflicts.py:35 + +self.runCmd("file " + exe, CURRENT_EXECUTABLE_SET) + what's that for? CHANGES SINCE LAST A

[Lldb-commits] [PATCH] D58125: Add ability to import std module into expression parser to improve C++ debugging

aprantl added inline comments. Comment at: lldb/packages/Python/lldbsuite/test/expression_command/import-std-module/conflicts/TestStdModuleWithConflicts.py:35 + +self.runCmd("file " + exe, CURRENT_EXECUTABLE_SET) + teemperor wrote: > aprantl wrote: > > w

[Lldb-commits] [PATCH] D58125: Add ability to import std module into expression parser to improve C++ debugging

aprantl added inline comments. Comment at: lldb/packages/Python/lldbsuite/test/expression_command/import-std-module/conflicts/TestStdModuleWithConflicts.py:35 + +self.runCmd("file " + exe, CURRENT_EXECUTABLE_SET) + teemperor wrote: > aprantl wrote: > > t

[Lldb-commits] [PATCH] D58838: Remove tautological #ifdefs

aprantl added a comment. Ping. As far as I can tell, now this patch should be entirely NFC. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58838/new/ https://reviews.llvm.org/D58838 ___ lldb-commits mailing list lldb-commits@lists.llvm.org h

[Lldb-commits] [PATCH] D58748: [ExpressionParser] Test GetClangResourceDir

aprantl added a comment. Looks mostly good! Comment at: source/Plugins/ExpressionParser/Clang/ClangHost.cpp:45 +static bool DefaultComputeClangDirectory(FileSpec &lldb_shlib_spec, + FileSpec &file_spec, bool verify) { W

[Lldb-commits] [PATCH] D59004: [lldb] Fix DW_OP_addrx uses.

aprantl added a comment. The code looks fine, but this needs a testcase. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59004/new/ https://reviews.llvm.org/D59004 ___ lldb-commits mailing list lldb-comm

[Lldb-commits] [PATCH] D59004: [lldb] Fix DW_OP_addrx uses.

aprantl added a comment. Is there a way to force clang to generate this and/or are existing tests failing because this support is missing? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59004/new/ https://reviews.llvm.org/D59004

[Lldb-commits] [PATCH] D59040: Move ExpressionSourceCode.cpp -> ClangExpressionSourceCode.cpp

aprantl added inline comments. Comment at: source/Plugins/ExpressionParser/Clang/ClangExpressionSourceCode.cpp:136 + else +return; + break; This `else return` doesn't do anything. Is it needed for swift-lldb? Otherwise I'd just delete it. ==

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

aprantl added inline comments. Comment at: lit/Reproducer/Functionalities/TestDataFormatter.test:5 + +# RUN: rm -rf %T/dataformatter +# RUN: %clang -x c++ %S/Inputs/foo.cpp -g -o %t.out same here Comment at: lit/Reproducer/Functionalities/TestS

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

aprantl requested changes to this revision. aprantl added inline comments. This revision now requires changes to proceed. Herald added a subscriber: jdoerfert. Comment at: lit/Reproducer/Functionalities/Inputs/foo.cpp:3 +public: + Foo(int i, double d) : m_i(i), m_d(d){}; + -

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

aprantl added inline comments. Comment at: lldb/lit/Reproducer/Functionalities/TestImagineList.test:8 + +# RUN: rm -rf %T/imagelist +# RUN: rm -rf %t.txt There's still a %T CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55626/new/ https://reviews.llvm.o

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

aprantl accepted this revision. aprantl added inline comments. This revision is now accepted and ready to land. Comment at: lldb/lit/Reproducer/Functionalities/TestDataFormatter.test:6 +# RUN: rm -rf %t.repro +# RUN: %clang -x c++ %S/Inputs/foo.cpp -g -o %t.out +

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

aprantl created this revision. aprantl added a reviewer: davide. Herald added a subscriber: jdoerfert. Herald added a project: LLDB. I find myself extracting m_type and and casting it to call dump() on the raw AST type several times per day, so why not make this a little more convenient! Reposi

[Lldb-commits] [PATCH] D59165: Remove DWARFDIECollection

aprantl added a comment. Whenever I see a GetFooAtIndex() method I expect this to be exposed via the SBAPI, but I assume this isn't the case here? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59165/new/ https://reviews.llvm.org/D59165 ___

[Lldb-commits] [PATCH] D59198: Correctly look up declarations in inline namespaces

aprantl accepted this revision. aprantl added inline comments. This revision is now accepted and ready to land. Comment at: lldb/source/Symbol/ClangASTContext.cpp:10269 +void *opaque_decl_ctx, void *other_opaque_decl_ctx) { + auto decl_ctx = (clang::DeclContext *)opaque_decl

[Lldb-commits] [PATCH] D59200: Fix a crasher in StackFrame::GetValueForVariableExpressionPath()

aprantl added inline comments. Comment at: packages/Python/lldbsuite/test/functionalities/var_path/TestVarPath.py:69 + +def do_test(self): +exe = self.getBuildArtifact("a.out") If I'm not mistaken, you might be able to replace the first 40 lines or s

[Lldb-commits] [PATCH] D59200: Fix a crasher in StackFrame::GetValueForVariableExpressionPath()

aprantl added inline comments. Comment at: packages/Python/lldbsuite/test/functionalities/var_path/TestVarPath.py:101 +# The hit count for the breakpoint should be 1. +self.assertTrue(breakpoint.GetHitCount() == 1) + until here. run_to_source_bre

[Lldb-commits] [PATCH] D58125: Add ability to import std module into expression parser to improve C++ debugging

aprantl added a comment. This looks mostly good now. Comment at: lldb/packages/Python/lldbsuite/test/expression_command/import-std-module/basic/TestImportStdModule.py:19 +@skipIf(oslist=no_match(["linux"])) +@skipIf(debug_info=no_match(["dwarf"])) +def test(self):

[Lldb-commits] [PATCH] D59217: Fix/unify SBType comparison

aprantl added a comment. Interesting find. Thanks! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59217/new/ https://reviews.llvm.org/D59217 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/lis

[Lldb-commits] [PATCH] D58125: Add ability to import std module into expression parser to improve C++ debugging

aprantl accepted this revision. aprantl added inline comments. This revision is now accepted and ready to land. Comment at: lldb/packages/Python/lldbsuite/test/expression_command/import-std-module/basic/TestImportStdModule.py:19 +@skipIf(oslist=no_match(["linux"])) +@ski

[Lldb-commits] [PATCH] D59235: Remove Support for DWARF64

aprantl accepted this revision. aprantl added a comment. This revision is now accepted and ready to land. LGTM, perhaps let's give this a day for others to chime in in case they missed the thread on lldb-dev. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59235/new/ https://reviews.llvm

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