[Lldb-commits] [PATCH] D51959: Add compatability version to liblldb in framework builds

teemperor added a comment. Nitpick: compatAbility->compatIbility LGTM otherwise, but don’t know frameworks good enough to accept. https://reviews.llvm.org/D51959 ___ lldb-commits mailing list lldb-commits@lists.llvm.org

[Lldb-commits] [lldb] r342008 - [MIPS] Fix signed overflow in DADDIU emulation

Author: vedantk Date: Tue Sep 11 16:04:05 2018 New Revision: 342008 URL: http://llvm.org/viewvc/llvm-project?rev=342008=rev Log: [MIPS] Fix signed overflow in DADDIU emulation This fixes a signed integer overflow diagnostic reported by ubsan. rdar://44353380 Modified:

[Lldb-commits] [PATCH] D51896: Refactoring std::function formatter to move core functionality into CPPLanguageRuntime

This revision was automatically updated to reflect the committed changes. Closed by commit rL341991: Refactoring std::function formatter to move core functionality into… (authored by shafik, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit:

[Lldb-commits] [PATCH] D51830: Add a way to make scripted breakpoints

jingham added a comment. I addressed the individual concerns in the comments. There were a couple of overall points: 1. Could we avoid adding AddLocation by having the callback return a list of addresses to set breakpoints. I actually like directly saying "Set me a location there" and I

[Lldb-commits] [PATCH] D51896: Refactoring std::function formatter to move core functionality into CPPLanguageRuntime

shafik updated this revision to Diff 164962. shafik added a comment. Removing change that was meant for the next PR https://reviews.llvm.org/D51896 Files: include/lldb/Target/CPPLanguageRuntime.h source/Plugins/Language/CPlusPlus/LibCxx.cpp source/Target/CPPLanguageRuntime.cpp Index:

[Lldb-commits] [PATCH] D51930: Add a basic test for 'memory region'

davide accepted this revision. davide added a comment. This revision is now accepted and ready to land. Yeah, sure :) Repository: rLLDB LLDB https://reviews.llvm.org/D51930 ___ lldb-commits mailing list lldb-commits@lists.llvm.org

[Lldb-commits] [PATCH] D51930: Add a basic test for 'memory region'

vsk added a comment. This LGTM. Davide? Repository: rLLDB LLDB https://reviews.llvm.org/D51930 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D51930: Add a basic test for 'memory region'

vsk added a comment. I think it'd be useful to test the driver output specifically. The kind of testing lldb-test facilitates might not be a good fit here (too low-level). Repository: rLLDB LLDB https://reviews.llvm.org/D51930 ___ lldb-commits

[Lldb-commits] [PATCH] D51935: [LLDB] - Improve reporting source lines and variables (improved DWARF5 support).

clayborg requested changes to this revision. clayborg added a comment. This revision now requires changes to proceed. Nice patch! A few minor fixes, see inlined comments. Comment at: source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.cpp:78 +uint32_t

Re: [Lldb-commits] [PATCH] D51162: [PDB] Restore AST from PDB symbols

A visitor would work, but unless we need this frequently it might be overkill. If we do need it frequently then it would be very helpful though. For now since we just have this one use case I think a switch statement on the tag is the simplest. You can group all same cases together and use the

[Lldb-commits] [PATCH] D51934: [target] Change target create's behavior wrt loading dependent files.

clayborg added a comment. Or add a new option that is --dependents that takes the enum and leave the --no-dependents there for compatability Repository: rLLDB LLDB https://reviews.llvm.org/D51934 ___ lldb-commits mailing list

[Lldb-commits] [PATCH] D51934: [target] Change target create's behavior wrt loading dependent files.

clayborg added a comment. I would rather not change the argument to be required if we can help it. That might break existing scripts or command files. I think if -d is specified without a value it should default to "no". Can we make the value optional? Repository: rLLDB LLDB

[Lldb-commits] [PATCH] D50681: Remove manual byte counting from internal Stream methods.

JDevlieghere accepted this revision as: JDevlieghere. JDevlieghere added a comment. This revision is now accepted and ready to land. Neat, LGTM! https://reviews.llvm.org/D50681 ___ lldb-commits mailing list lldb-commits@lists.llvm.org

[Lldb-commits] [PATCH] D51175: Add support for descriptions with command completions.

JDevlieghere added a comment. I don't know the completion API well enough to accept, but this looks like a nice improvement! Comment at: source/Utility/CompletionRequest.cpp:79 + // Add the completion if we haven't seen the same value before. + if

[Lldb-commits] [PATCH] D51445: Remove undefined behavior around the use of StateType

This revision was automatically updated to reflect the committed changes. Closed by commit rLLDB341957: Remove undefined behavior around the use of StateType (authored by shafik, committed by ). Herald added a subscriber: abidh. Repository: rLLDB LLDB https://reviews.llvm.org/D51445 Files:

[Lldb-commits] [PATCH] D51445: Remove undefined behavior around the use of StateType

This revision was automatically updated to reflect the committed changes. Closed by commit rL341957: Remove undefined behavior around the use of StateType (authored by shafik, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit:

[Lldb-commits] [PATCH] D50809: Remove byte counting from SourceManager

JDevlieghere accepted this revision. JDevlieghere added a comment. This revision is now accepted and ready to land. Maybe mention that this is NFC in the commit msg? Otherwise this LGTM. Repository: rLLDB LLDB https://reviews.llvm.org/D50809 ___

[Lldb-commits] [lldb] r341957 - Remove undefined behavior around the use of StateType

Author: shafik Date: Tue Sep 11 09:08:05 2018 New Revision: 341957 URL: http://llvm.org/viewvc/llvm-project?rev=341957=rev Log: Remove undefined behavior around the use of StateType rdar://problem/43530233 Patch by Shafik Yaghmour. Differential Revision: https://reviews.llvm.org/D51445

[Lldb-commits] [PATCH] D51935: [LLDB] - Improve reporting source lines and variables (improved DWARF5 support).

grimar created this revision. grimar added a reviewer: LLDB. Herald added subscribers: JDevlieghere, arichardson, aprantl, emaste. Herald added a reviewer: espindola. grimar edited the summary of this revision. This patch improves the support of DWARF5 by lldb. Imagine we have the following

[Lldb-commits] [PATCH] D51934: [target] Change target create's behavior wrt loading dependent files.

JDevlieghere created this revision. JDevlieghere added reviewers: jasonmolenda, jingham, LLDB. JDevlieghere added a dependency: D51859: [NFC] Turn "load dependent files" boolean into an enum . JDevlieghere edited the summary of this revision. When creating a target, lldb loads all dependent

[Lldb-commits] [PATCH] D51930: Add a basic test for 'memory region'

davide added a comment. Yeah, that's the revision I had in mind. Can't you extend `lldb-test` to do what you need? Repository: rLLDB LLDB https://reviews.llvm.org/D51930 ___ lldb-commits mailing list lldb-commits@lists.llvm.org

Re: [Lldb-commits] [PATCH] D51162: [PDB] Restore AST from PDB symbols

Yes, you are right, we can just to consider some cases important for us in the function, but I thought to solve the problem in a more general way. I think that the key problem is that we lose some type info when we get a `PDBSymbol`. So we need to dyn_cast the symbol multiple times or to analyse

[Lldb-commits] [PATCH] D51830: Add a way to make scripted breakpoints

clayborg added a comment. See inlined comments. Cool stuff. Comment at: include/lldb/API/SBBreakpoint.h:26-27 + SBBreakpoint(const lldb::BreakpointSP _sp); + ~SBBreakpoint(); Why does this need to be public? Comment at:

[Lldb-commits] [PATCH] D51162: [PDB] Restore AST from PDB symbols

llvm-commits added a subscriber: labath. llvm-commits added a comment. That’s fine - F7177739: msg-17534-333.txt Repository: rL LLVM https://reviews.llvm.org/D51162 ___ lldb-commits mailing list

Re: [Lldb-commits] [PATCH] D51162: [PDB] Restore AST from PDB symbols

On Tue, Sep 11, 2018 at 4:28 AM Aleksandr Urakov via Phabricator < revi...@reviews.llvm.org> wrote: > aleksandr.urakov added inline comments. > > > > Comment at: lldb/trunk/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp:289 > + return GetClassOrFunctionParent(*lexical_parent); >

[Lldb-commits] [PATCH] D51930: Add a basic test for 'memory region'

teemperor added a subscriber: vsk. teemperor added a comment. @davide Maybe git is failing me, but I can only find https://reviews.llvm.org/D47508 but that doesn't seem related. Let's ping @vsk Repository: rLLDB LLDB https://reviews.llvm.org/D51930

Re: [Lldb-commits] [PATCH] D51162: [PDB] Restore AST from PDB symbols

That’s fine On Tue, Sep 11, 2018 at 7:11 AM Aleksandr Urakov via Phabricator < revi...@reviews.llvm.org> wrote: > aleksandr.urakov added a comment. > > @stella.stamenova @ted Fixed with https://reviews.llvm.org/rL341942, > thanks again! > @zturner I'll rewrite `GetClassOrFunctionParent` and will

[Lldb-commits] [PATCH] D51557: Replace uses of LazyBool with LazyBool template

teemperor marked 4 inline comments as done. teemperor added a comment. (Phabricator didn't send my draft, sorry for the delay). I have more similar patches coming up for the other uses of LazyBool, so I would prefer finishing the implementation in LLDB and then afterwards asking if the ADT

Re: [Lldb-commits] [lldb] r341683 - Add input files to the "prepare swig bindings" step.

Already done (see recent commits). On Tue, Sep 11, 2018 at 7:22 AM Greg Clayton via lldb-commits wrote: > > Is the space in the string for for SBHostOS.cpp ok? Might want to remove it > just in case: > > + " $(SRCROOT)/source/API/SBHostOS.cpp", > > On Sep 7, 2018, at 11:10 AM, Jim Ingham via

[Lldb-commits] [PATCH] D51930: Add a basic test for 'memory region'

davide requested changes to this revision. davide added a comment. This revision now requires changes to proceed. I thought Vedant added an harness utility to test memory a bit ago. Have you looked into whether that's extensible for your purposes, Raphael? Repository: rLLDB LLDB

[Lldb-commits] [PATCH] D51175: Add support for descriptions with command completions.

teemperor added a comment. Herald added a subscriber: lldb-commits. ping :) Repository: rLLDB LLDB https://reviews.llvm.org/D51175 ___ lldb-commits mailing list lldb-commits@lists.llvm.org

Re: [Lldb-commits] [lldb] r341683 - Add input files to the "prepare swig bindings" step.

Is the space in the string for for SBHostOS.cpp ok? Might want to remove it just in case: + " $(SRCROOT)/source/API/SBHostOS.cpp", > On Sep 7, 2018, at 11:10 AM, Jim Ingham via lldb-commits > wrote: > > Author: jingham > Date: Fri Sep 7 11:10:26 2018 > New

[Lldb-commits] [PATCH] D51930: Add a basic test for 'memory region'

teemperor created this revision. teemperor added a reviewer: LLDB. Herald added a subscriber: lldb-commits. The 'memory region' command is at the moment not tested at all by our test suite. This patch just adds a basic test that at least provides some basic testing. Repository: rLLDB LLDB

[Lldb-commits] [PATCH] D51162: [PDB] Restore AST from PDB symbols

aleksandr.urakov added a comment. @stella.stamenova @ted Fixed with https://reviews.llvm.org/rL341942, thanks again! @zturner I'll rewrite `GetClassOrFunctionParent` and will create a different review for that, ok? Repository: rL LLVM https://reviews.llvm.org/D51162

[Lldb-commits] [lldb] r341942 - [PDB] Fix problems after rL341782

Author: aleksandr.urakov Date: Tue Sep 11 07:03:12 2018 New Revision: 341942 URL: http://llvm.org/viewvc/llvm-project?rev=341942=rev Log: [PDB] Fix problems after rL341782 Summary: This commit fixes following problems after rL341782: - Broken SymbolFilePDBTests - Warning on comparison of

[Lldb-commits] [PATCH] D51602: Print the correct error when our DynamicCheckerFunctions fail to install

This revision was automatically updated to reflect the committed changes. Closed by commit rL341940: Print the correct error when our DynamicCheckerFunctions fail to install (authored by teemperor, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit:

[Lldb-commits] [PATCH] D51602: Print the correct error when our DynamicCheckerFunctions fail to install

This revision was automatically updated to reflect the committed changes. Closed by commit rLLDB341940: Print the correct error when our DynamicCheckerFunctions fail to install (authored by teemperor, committed by ). Repository: rL LLVM https://reviews.llvm.org/D51602 Files:

[Lldb-commits] [lldb] r341940 - Print the correct error when our DynamicCheckerFunctions fail to install

Author: teemperor Date: Tue Sep 11 06:59:47 2018 New Revision: 341940 URL: http://llvm.org/viewvc/llvm-project?rev=341940=rev Log: Print the correct error when our DynamicCheckerFunctions fail to install Summary: The check is inverted here: If we have error messages, we should print those

[Lldb-commits] [lldb] r341931 - [NFC] Fix compiler warning in TestArmv7Disassembly.cpp

Author: teemperor Date: Tue Sep 11 05:45:22 2018 New Revision: 341931 URL: http://llvm.org/viewvc/llvm-project?rev=341931=rev Log: [NFC] Fix compiler warning in TestArmv7Disassembly.cpp The warning is comparison of integers of different signs: 'const int' and 'const unsigned long' and

[Lldb-commits] [PATCH] D51162: [PDB] Restore AST from PDB symbols

aleksandr.urakov added a comment. In https://reviews.llvm.org/D51162#1229112, @stella.stamenova wrote: > This change is causing three of the symbol file PDB tests to fail: > > lldb-Unit :: > SymbolFile/PDB/release/SymbolFilePDBTests.exe/SymbolFilePDBTests.TestClassInNamespace > lldb-Unit ::

[Lldb-commits] [PATCH] D51162: [PDB] Restore AST from PDB symbols

aleksandr.urakov added inline comments. Comment at: lldb/trunk/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp:289 + return GetClassOrFunctionParent(*lexical_parent); +} + zturner wrote: > However, this is only for symbols (not types). So we can exclude

[Lldb-commits] [PATCH] D51162: [PDB] Restore AST from PDB symbols

aleksandr.urakov added inline comments. Comment at: lldb/trunk/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp:268-269 + +std::unique_ptr +GetClassOrFunctionParent(const llvm::pdb::PDBSymbol ) { + const IPDBSession = symbol.getSession(); zturner wrote: > All

[Lldb-commits] [PATCH] D51162: [PDB] Restore AST from PDB symbols

aleksandr.urakov added a comment. In https://reviews.llvm.org/D51162#1229616, @ted wrote: > The "auto from = 0" should be "size_t from = 0", since auto can't determine > the correct type. Yes, I've missed that because MSVC doesn't emit such a warning. Thanks for catching that! Repository: