teemperor created this revision.
We always print two error messages when we hit an unknown command. As the
function with
one error message unconditionally calls the other, we can just remove that
error message.
Fixes https://bugs.llvm.org/show_bug.cgi?id=38312
https://reviews.llvm.org/D49831
teemperor updated this revision to Diff 157370.
teemperor added a comment.
(Added small refactoring I forgot to add to the diff).
https://reviews.llvm.org/D49322
Files:
include/lldb/Utility/CompletionRequest.h
source/Commands/CommandCompletions.cpp
teemperor updated this revision to Diff 157368.
teemperor added a comment.
- Addressed Davide's comments.
https://reviews.llvm.org/D49322
Files:
include/lldb/Utility/CompletionRequest.h
source/Commands/CommandCompletions.cpp
source/Commands/CommandObjectCommands.cpp
teemperor marked an inline comment as done.
teemperor added inline comments.
Comment at: source/Commands/CommandObjectMultiword.cpp:378
return proxy_command->HandleArgumentCompletion(request,
opt_element_vector);
- request.GetMatches().Clear();
return 0;
Author: davide
Date: Wed Jul 25 14:18:20 2018
New Revision: 337963
URL: http://llvm.org/viewvc/llvm-project?rev=337963=rev
Log:
Revert "[DataFormatters] Add formatter for C++17 std::optional."
I forgot to git add some files. I'm going to recommit the correct
version at once soon.
Removed:
This revision was automatically updated to reflect the committed changes.
Closed by commit rL337959: [DataFormatters] Add formatter for C++17
std::optional. (authored by davide, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
Author: davide
Date: Wed Jul 25 13:46:29 2018
New Revision: 337959
URL: http://llvm.org/viewvc/llvm-project?rev=337959=rev
Log:
[DataFormatters] Add formatter for C++17 std::optional.
Patch by Shafik Yaghmour.
Differential Revision: https://reviews.llvm.org/D49271
Added:
teemperor added inline comments.
Comment at: source/Core/Highlighter.cpp:34
+ // Calculate how many bytes we have written.
+ return m_prefix.size() + value.size() + m_suffix.size();
+}
labath wrote:
> This isn't correct, as you're not writing m_prefix, but
davide added inline comments.
Comment at: source/Commands/CommandCompletions.cpp:251-261
request.SetWordComplete(false);
StandardTildeExpressionResolver Resolver;
- return DiskDirectories(request.GetCursorArgumentPrefix(),
- request.GetMatches(),
apolyakov added inline comments.
Comment at: lit/lit.cfg:59
-debugserver = lit.util.which('debugserver', lldb_tools_dir)
+if platform.system() in ['Darwin']:
+debugserver = lit.util.which('debugserver', lldb_tools_dir)
Do we have `debugserver` only on
shafik updated this revision to Diff 157337.
shafik added a comment.
Adding additional comments
https://reviews.llvm.org/D49271
Files:
lldb.xcodeproj/project.pbxproj
packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/optional/Makefile
teemperor updated this revision to Diff 157320.
teemperor marked 8 inline comments as done.
teemperor added a comment.
- Addressed Pavel's comments. Also cleaned up some more includes.
Yes, knowing the language we currently display would be nice, but I didn't see
a good way to reliably get this
apolyakov updated this revision to Diff 157310.
apolyakov added a comment.
Moved test from bash to python, removed unnecessary
`Target::AppendImageSearchPath`.
https://reviews.llvm.org/D49739
Files:
include/lldb/API/SBTarget.h
lit/lit.cfg
lit/tools/lldb-mi/target/inputs/main.c
EugeneBi added a comment.
In https://reviews.llvm.org/D49685#1174770, @labath wrote:
> Could you also add a test for this?
I never ran LLDB tests, not sure where they are and what they are.
Also, how would you test that? I know now my open core dump works, but I cannot
share it.
keith added inline comments.
Comment at: cmake/modules/LLDBFramework.cmake:21
add_custom_command(TARGET lldb-framework POST_BUILD
COMMAND ${CMAKE_COMMAND} -E copy_directory
${CMAKE_CURRENT_BINARY_DIR}/FrameworkHeaders $/Headers
COMMAND ${CMAKE_COMMAND} -E
lemo added inline comments.
Comment at: source/Plugins/Process/minidump/RegisterContextMinidump_ARM.cpp:51
+ reg_fpscr,
+ reg_d0, reg_d1, reg_d2, reg_d3, reg_d4, reg_d5, reg_d6, reg_d7,
+ reg_d8, reg_d9, reg_d10, reg_d11, reg_d12, reg_d13, reg_d14, reg_d15,
xiaobai added a comment.
If I understand correctly, this is putting the headers directly into the
framework? That's a pretty good idea :D
In https://reviews.llvm.org/D49779#1174659, @keith wrote:
> It seems like if this was a common occurrence, it would've been fixed
> earlier, so I'm
teemperor added a comment.
Sounds reasonable. It seems both arcanist and patch can't handle the patch
file, so let's just see how it goes post-commit.
https://reviews.llvm.org/D49740
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
Author: jdevlieghere
Date: Wed Jul 25 08:20:15 2018
New Revision: 337932
URL: http://llvm.org/viewvc/llvm-project?rev=337932=rev
Log:
[ProcessGDBRemote] handle result from ConnectToDebugserver
We ignored the result from ConnectToDebugserver, causing certain errors
(like a failed handshake) not
This revision was automatically updated to reflect the committed changes.
Closed by commit rL337931: Use LLVMs new ItaniumPartialDemangler in LLDB
(authored by stefan.graenitz, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
Author: stefan.graenitz
Date: Wed Jul 25 08:19:04 2018
New Revision: 337931
URL: http://llvm.org/viewvc/llvm-project?rev=337931=rev
Log:
Use LLVM's new ItaniumPartialDemangler in LLDB
Summary:
Replace the existing combination of FastDemangle and the fallback to
llvm::itaniumDemangle() with
JDevlieghere accepted this revision.
JDevlieghere added a comment.
This revision is now accepted and ready to land.
Given these numbers I think we can land this patch as is and figure out the IDP
stuff on the mailing list and/or a future patch. LGTM. Thanks Stefan!
aleksandr.urakov added a subscriber: zturner.
aleksandr.urakov added a comment.
Yes, I have understood that, thank you. It may be not trivial in a case of
multiple nested unnamed unions/structs, but it's solvable for clang-emitted
PDBs. But I'm not sure about cl-emitted PDBs too.
Can we leave
zturner added a comment.
Wouldn’t the location of the unnamed struct be the location of its first
member? We already print the offsets of the individual members, so that
part is solvable (although that code is horrendously complex). The second
part is figuring out how to correlate the member back
Wouldn’t the location of the unnamed struct be the location of its first
member? We already print the offsets of the individual members, so that
part is solvable (although that code is horrendously complex). The second
part is figuring out how to correlate the member back to the unnamed struct
it
Author: labath
Date: Wed Jul 25 04:35:28 2018
New Revision: 337908
URL: http://llvm.org/viewvc/llvm-project?rev=337908=rev
Log:
Fix PythonString::GetString for >=python-3.7
The return value of PyUnicode_AsUTF8AndSize is now "const char *".
Thanks to Brett Neumeier for testing the patch out on
aleksandr.urakov added a comment.
In https://reviews.llvm.org/D49410#1174787, @zturner wrote:
> When you have a DIA interface for struct S, can you just call
> findChildren()? Will that enumerate tge unnamed struct?
I have checked this again with `cl` and `clang-cl` (but I have enumerated all
labath added a comment.
In https://reviews.llvm.org/D49740#1173655, @teemperor wrote:
> Did you test that with lldb's C++ modules? If not I can test it locally.
I didn't, but I don't expect there to be any issues, as I've made sure that the
moved files don't include anything outside of
labath added a comment.
In https://reviews.llvm.org/D49739#1174810, @apolyakov wrote:
> Thanks, I used this `lldb-server gdbserver --pipe 0 localhost:0` and got a
> port chosen by debugserver. But to let lldb-mi know about this port we need
> an additional script, is python a good choice for
apolyakov added a comment.
Thanks, I used this `lldb-server gdbserver --pipe 0 localhost:0` and got a port
chosen by debugserver. But to let lldb-mi know about this port we need an
additional script, is python a good choice for that?
https://reviews.llvm.org/D49739
labath added a comment.
In https://reviews.llvm.org/D49739#1174769, @apolyakov wrote:
> `packages/Python/lldbsuite/test/tools/lldb-mi/signal/TestMiSignal.py` test
> contains `port = 12000 + random.randint(0, 3999)`.
Ok, that's bad. I don't know why the other lldb-mi tests are flaky, but these
When you have a DIA interface for struct S, can you just call
findChildren()? Will that enumerate tge unnamed struct?
The fact that pdbutil doesn’t is only an indication of how the printing
code behaves, you shouldn’t interpret anything about what information is
available from it
On Wed, Jul 25,
zturner added a subscriber: asmith.
zturner added a comment.
When you have a DIA interface for struct S, can you just call
findChildren()? Will that enumerate tge unnamed struct?
The fact that pdbutil doesn’t is only an indication of how the printing
code behaves, you shouldn’t interpret anything
labath added a comment.
Could you also add a test for this?
https://reviews.llvm.org/D49685
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
apolyakov added a comment.
`packages/Python/lldbsuite/test/tools/lldb-mi/signal/TestMiSignal.py` test
contains `port = 12000 + random.randint(0, 3999)`.
https://reviews.llvm.org/D49739
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
labath added a comment.
In https://reviews.llvm.org/D49739#1174744, @apolyakov wrote:
> What do you think about running tests with a hardcoded port number(as it's
> done in a web-services). Doing this, we get rid of additional scripts and
> os-specific things. AFAIK, debugserver even has its
apolyakov added a comment.
What do you think about running tests with a hardcoded port number(as it's done
in a web-services). Doing this, we get rid of additional scripts and
os-specific things. AFAIK, debugserver even has its own default port.
P.S. As I saw in the lldb-mi python tests, port
labath added a reviewer: xiaobai.
labath added a comment.
Adding Alex, as he was doing a lot of framework work lately.
https://reviews.llvm.org/D49779
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
labath added a comment.
The patch is bigger than ideal for a single change, but I like the way it is
structured. Thank you for abstracting the clang specifics.
The one high-level question that came to mind when looking this over is whether
we really need to do all this file name matching to
aleksandr.urakov added inline comments.
Comment at: lit/SymbolFile/PDB/Inputs/ClassLayoutTest.cpp:37
+ };
+ union { // Test unnamed union. MSVC treats it as `int a; float b;`
+int a;
aleksandr.urakov wrote:
> aleksandr.urakov wrote:
> > Hui wrote:
> > >
sgraenitz marked an inline comment as done.
sgraenitz added a comment.
I did a little investigation of the performance impact of not reusing the IPD.
For this I used the unit test `PartialDemangleTest.TestNameChopping`, where I
added an outer loop:
TEST(PartialDemangleTest, TestNameChopping)
labath added inline comments.
Comment at: source/Plugins/Process/minidump/ProcessMinidump.cpp:179
+auto platform_sp = target.GetPlatform();
+if (platform_sp && !platform_sp->IsCompatibleArchitecture(arch, false,
nullptr)) {
+ ArchSpec platform_arch;
aleksandr.urakov added inline comments.
Comment at: lit/SymbolFile/PDB/Inputs/ClassLayoutTest.cpp:37
+ };
+ union { // Test unnamed union. MSVC treats it as `int a; float b;`
+int a;
aleksandr.urakov wrote:
> Hui wrote:
> > aleksandr.urakov wrote:
> > >
keith added a comment.
It seems like if this was a common occurrence, it would've been fixed earlier,
so I'm wondering if there's a difference in the way I'm building lldb that
causes this. Using cmake:
cmake ../llvm -G Ninja -DCMAKE_BUILD_TYPE=Debug
-DCMAKE_EXPORT_COMPILE_COMMANDS=YES
keith created this revision.
keith added reviewers: kastiglione, beanz.
Herald added a subscriber: mgorny.
Previously the framework-header-fix script would change the sources
before they were copied, leading to unnecessary rebuilds on repeat
`ninja lldb` invocations. This runs the script on the
45 matches
Mail list logo
