[Lldb-commits] [PATCH] D52516: [lldbinline] Set directory attribute on test-specific classes

[Davide Italiano via Phabricator via lldb-commits]

davide accepted this revision.
davide added a comment.

LGTM.


https://reviews.llvm.org/D52516



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D51874: Fix buildbot regression by rL339929: NameError: global name 'test_directory' is not defined

[Davide Italiano via Phabricator via lldb-commits]

davide accepted this revision.
davide added a comment.
This revision is now accepted and ready to land.

I think this looks fine. Vedant, thoughts?


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D51874



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D52851: Adding support to step into the callable wrapped by libc++ std::function

[Davide Italiano via Phabricator via lldb-commits]

davide added a comment.

Some basic comments. Haven't looked at the implementation very closely, I'll do 
that probably tomorrow. Thanks for working on this!




Comment at: include/lldb/Target/CPPLanguageRuntime.h:59-60
 
+  lldb::ThreadPlanSP GetStepThroughTrampolinePlan(Thread &thread,
+  bool stop_others);
+

Please add a doxygen comment to this function. (I understand the surrounding 
ones are not commented, but we should start somewhere)/



Comment at: 
packages/Python/lldbsuite/test/lang/cpp/std-function-step-into-callable/TestStdFunctionStepIntoCallable.py:1-3
+"""
+Test lldb data formatter subsystem.
+"""

This comment seems a little outdated.



Comment at: 
packages/Python/lldbsuite/test/lang/cpp/std-function-step-into-callable/TestStdFunctionStepIntoCallable.py:8-9
+
+import os
+import time
+import lldb

I'm not sure you need these.



Comment at: 
packages/Python/lldbsuite/test/lang/cpp/std-function-step-into-callable/TestStdFunctionStepIntoCallable.py:27-28
+
+@add_test_categories(["libc++"])
+def test(self):
+"""Test that std::function as defined by libc++ is correctly printed 
by LLDB"""

Is this affected by the debug info variant used? (i.e. 
dSYM/gmodules/DWARF/dwo). 
If not, this could be a `NO_DEBUG_INFO` test



Comment at: 
packages/Python/lldbsuite/test/lang/cpp/std-function-step-into-callable/TestStdFunctionStepIntoCallable.py:56-73
+self.assertEqual( 
self.thread.GetFrameAtIndex(0).GetLineEntry().GetLine(), 13 ) ;
+self.process.Continue()
+
+self.thread.StepInto()
+self.assertEqual( 
self.thread.GetFrameAtIndex(0).GetLineEntry().GetLine(), 28 ) ;
+self.process.Continue()
+

All these lines check hardcoded make me a little nervous, as they're really 
fragile. Can you make them parametric at least?
so that if somebody slides down the test case they're still valid (i.e. base + 
offset)



Comment at: 
packages/Python/lldbsuite/test/lang/cpp/std-function-step-into-callable/main.cpp:1-9
+//===-- main.cpp --*- C++ 
-*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//

You don't need a license for the test case, I think.



Comment at: source/Target/ThreadPlanStepThrough.cpp:99-105
+
+CPPLanguageRuntime *cpp_runtime =
+m_thread.GetProcess()->GetCPPLanguageRuntime();
+
+if (cpp_runtime)
+  m_sub_plan_sp =
+  cpp_runtime->GetStepThroughTrampolinePlan(m_thread, m_stop_others);

Can you add a comment here?


https://reviews.llvm.org/D52851



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D52678: DWARFExpression: Resolve file addresses in the linked module

[Davide Italiano via Phabricator via lldb-commits]

davide added inline comments.



Comment at: 
packages/Python/lldbsuite/test/functionalities/target_var/globals.ll:1
+source_filename = "globals.c"
+target datalayout = "e-m:o-i64:64-f80:128-n8:16:32:64-S128"

vsk wrote:
> Should we check in bitcode instead? That might make it easier to avoid 
> breakage when the textual IR format changes.
But also will be more painful to regenerate in case, e.g. add a verifier check 
where this breaks. 
I think it's a tradeoff anyway, I and Adrian discussed this briefly and agreed 
this is a decent middle ground (on one extreme, one might check ASM directly, 
but that seems even harder to handle).

This is honestly based on my experience of having to regenerate broken bitcode 
files, happy to be convinced otherwise if you have arguments :)


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D52678



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D50304: [lldb] CommandObjectThreadUntil should set breakpoint at either on exact or the nearest subsequent line number but not on all the subsequent line numbers

[Davide Italiano via Phabricator via lldb-commits]

davide added a comment.

I'll commit this for you, but I might ask if you can try adding a test first?


https://reviews.llvm.org/D50304



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D53010: Add an alias "var" for "frame var" and "vo" for "frame var -O"

[Davide Italiano via Phabricator via lldb-commits]

davide accepted this revision.
davide added a comment.
This revision is now accepted and ready to land.

LG. If you can add a line to an existing inline test, I think it would be 
excellent.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D53010



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D52851: Adding support to step into the callable wrapped by libc++ std::function

[Davide Italiano via Phabricator via lldb-commits]

davide accepted this revision.
davide added a comment.

LGTM too. Thanks for taking the time to address the comments.


https://reviews.llvm.org/D52851



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D53530: Fix (and improve) the support for C99 variable length array types

[Davide Italiano via Phabricator via lldb-commits]

davide added inline comments.



Comment at: 
source/Plugins/LanguageRuntime/CPlusPlus/ItaniumABI/ItaniumABILanguageRuntime.cpp:213-229
+  // Handle variables with incomplete array types.
+  auto *type = in_value.GetCompilerType().GetOpaqueQualType();
+  auto qual_type = clang::QualType::getFromOpaquePtr(type).getCanonicalType();
+  if (qual_type->getTypeClass() == clang::Type::IncompleteArray) {
+if (auto variable = in_value.GetVariable()) {
+  auto *lldb_type = variable->GetType();
+  auto *symbol_file = lldb_type->GetSymbolFile();

clayborg wrote:
> We must abstract this via the TypeSystem stuff and make this happen via the 
> CompilerType interface. What happens when "in_value" is a swift type or other 
> type from the type system? Crash
My understanding is that it is not possible today to get here with a swift 
type. In fact, `GetDynamicTypeAndAddress` in swift will be always called on a 
`SwiftLanguageRuntime`. 

That said, I'm not necessarily advocating against the abstraction, just 
pointing out that this is just a theoretical concern, and given this is an 
internal API in the debugger, it could be possibly enforced by an assertion to 
make sure people don't change the status quo accidentally.


https://reviews.llvm.org/D53530



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D53788: [FileSystem] Remove GetByteSize() from FileSpec

[Davide Italiano via Phabricator via lldb-commits]

davide accepted this revision.
davide added a comment.
This revision is now accepted and ready to land.

LGTM


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D53788



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D53834: [FileSystem] Remove ResolveExecutableLocation() from FileSpec

[Davide Italiano via Phabricator via lldb-commits]

davide accepted this revision.
davide added a comment.
This revision is now accepted and ready to land.

LGTM, thanks.


https://reviews.llvm.org/D53834



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D54003: Refactor ClangASTContext::AddEnumerationValueToEnumerationType() to remove redundant parameter which can be calculated from other parameter

[Davide Italiano via Phabricator via lldb-commits]

davide added inline comments.



Comment at: 
packages/Python/lldbsuite/test/expression_command/radar_43822994/TestScopedEnumType.py:42
+## integral is not implicitly convertible to a scoped enum
+value = frame.EvaluateExpression("1 == Foo::FooBar")
+self.assertTrue(value.IsValid())

This clearly looks like can be an inline test (and probably would make the 
thing more readable, IMHO).


https://reviews.llvm.org/D54003



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D53368: [Symbol] Search symbols with name and type in a symbol file

[Davide Italiano via Phabricator via lldb-commits]

davide added a comment.

This broke MacOS. I'm going to revert this. To reproduce, just run `ninja 
check-lldb` with your patches.
Please let me know if you need other informations.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D53368



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D53368: [Symbol] Search symbols with name and type in a symbol file

[Davide Italiano via Phabricator via lldb-commits]

davide added a comment.

In https://reviews.llvm.org/D53368#1286361, @aleksandr.urakov wrote:

> Thanks for catching that! Unfortunately, I have no access to MacOS, can you 
> provide some more info about failure, please?


Unfortunately the bot logs are gone. When I originally looked at them they 
weren't particularly informative, so I'm afraid the only way would be that of 
trying to reproduce this thing on a real machine.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D53368



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D54056: Add SetAllowJIT (the SBExpressionOptions equivalent of "expression --allow-jit")

[Davide Italiano via Phabricator via lldb-commits]

davide added a comment.

In https://reviews.llvm.org/D54056#1286620, @jankratochvil wrote:

> It broke the testsuite for me:
>
>   $ time 
> PYTHONPATH=$PWD/lib64/python2.7/site-packages:$PWD/lib64/python2.7/site-packages/lldb
>  ../llvm-git/tools/lldb/test/dotest.py --executable $PWD/bin/lldb -C 
> $PWD/bin/clang -t ../llvm-git/tools/lldb/packages/Python/lldbsuite/test/ 
>   WARNING:root:No valid FileCheck executable; some tests may fail...
>   WARNING:root:(Double-check the --filecheck argument to dotest.py)
>   LLDB library dir: /home/jkratoch/redhat/llvm-git-build-release-clang/bin
>   LLDB import library dir: 
> /home/jkratoch/redhat/llvm-git-build-release-clang/bin
>   lldb version 8.0.0
> clang revision 6974b990e13dfb4190a6dffdcc8bac9edbd1cde5
> llvm revision 7fad5fb0d0d32beea4e95e239cc065a850733358
>   Libc++ tests will not be run because: Unable to find libc++ installation
>   Skipping following debug info categories: ['dsym', 'gmodules']
>   Traceback (most recent call last):
> File "../llvm-git/tools/lldb/test/dotest.py", line 7, in 
>   lldbsuite.test.run_suite()
> File 
> "/home/jkratoch/redhat/llvm-git/tools/lldb/packages/Python/lldbsuite/test/dotest.py",
>  line 1323, in run_suite
>   visit('Test', dirpath, filenames)
> File 
> "/home/jkratoch/redhat/llvm-git/tools/lldb/packages/Python/lldbsuite/test/dotest.py",
>  line 965, in visit
>   raise Exception("Found multiple tests with the name %s" % name)
>   Exception: Found multiple tests with the name TestSampleTest.py
>
>
> As really there are now:
>
>   $ find -name TestSampleTest.py
>   
> ./packages/Python/lldbsuite/test/expression_command/dont_allow_jit/TestSampleTest.py
>   ./packages/Python/lldbsuite/test/sample_test/TestSampleTest.py
>


I don't think anybody will look at this until Monday, so if you want a quick 
fix you might consider renaming the test yourself.

-

Davide


Repository:
  rL LLVM

https://reviews.llvm.org/D54056



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D54057: Remove Go debugger plugin

[Davide Italiano via Phabricator via lldb-commits]

davide accepted this revision.
davide added a comment.

LGTM. Thanks for picking up the slack.


https://reviews.llvm.org/D54057



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D54059: Remove Java debugger plugin

[Davide Italiano via Phabricator via lldb-commits]

davide added inline comments.



Comment at: tools/lldb-test/SystemInitializerTest.cpp:194
   SystemRuntimeMacOSX::Initialize();
   RenderScriptRuntime::Initialize();
-  JavaLanguageRuntime::Initialize();

Aside, do you know why we have renderscript support? maybe that should go away 
too.


https://reviews.llvm.org/D54059



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D51566: Add a relocation to ObjectFileELF::ApplyRelocations and a test

[Davide Italiano via Phabricator via lldb-commits]

davide added a comment.

@lanza Hey Nathan, it looks like your test exposed an UB in 
`ELFObjectFileReader`. May I ask you to take a look?

- TEST 'lldb-Unit :: 
ObjectFile/ELF/./ObjectFileELFTests/ObjectFileELFTest.TestAARCH64Relocations' 
FAILED 

Note: Google Test filter = ObjectFileELFTest.TestAARCH64Relocations
[==] Running 1 test from 1 test case.
[--] Global test environment set-up.
[--] 1 test from ObjectFileELFTest
[ RUN  ] ObjectFileELFTest.TestAARCH64Relocations
/Users/buildslave/jenkins/workspace/lldb-sanitized/llvm/tools/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp:2741:11:
 runtime error: store to misaligned address 0x61f01526 for type 'uint32_t' 
(aka 'unsigned int'), which requires 4 byte alignment
0x61f01526: note: pointer points here
 00 00 04 00 00 00  00 00 08 01 00 00 00 00  0c 00 00 00 00 00 00 00  00 00 00 
00 00 00 00 00  00 00

  ^ 

SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior 
/Users/buildslave/jenkins/workspace/lldb-sanitized/llvm/tools/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp:2741:11
 in


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D51566



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D51566: Add a relocation to ObjectFileELF::ApplyRelocations and a test

[Davide Italiano via Phabricator via lldb-commits]

davide added a comment.

In https://reviews.llvm.org/D51566#1288792, @davide wrote:

> @lanza Hey Nathan, it looks like your test exposed an UB in 
> `ELFObjectFileReader`. May I ask you to take a look?
>
> - TEST 'lldb-Unit :: 
> ObjectFile/ELF/./ObjectFileELFTests/ObjectFileELFTest.TestAARCH64Relocations' 
> FAILED  Note: Google Test filter = 
> ObjectFileELFTest.TestAARCH64Relocations [==] Running 1 test from 1 
> test case. [--] Global test environment set-up. [--] 1 test 
> from ObjectFileELFTest [ RUN  ] ObjectFileELFTest.TestAARCH64Relocations 
> /Users/buildslave/jenkins/workspace/lldb-sanitized/llvm/tools/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp:2741:11:
>  runtime error: store to misaligned address 0x61f01526 for type 
> 'uint32_t' (aka 'unsigned int'), which requires 4 byte alignment 
> 0x61f01526: note: pointer points here 00 00 04 00 00 00  00 00 08 01 00 
> 00 00 00  0c 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  00 00 ^ SUMMARY: 
> UndefinedBehaviorSanitizer: undefined-behavior 
> /Users/buildslave/jenkins/workspace/lldb-sanitized/llvm/tools/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp:2741:11
>  in




In https://reviews.llvm.org/D51566#1288792, @davide wrote:

> @lanza Hey Nathan, it looks like your test exposed an UB in 
> `ELFObjectFileReader`. May I ask you to take a look?
>
>    TEST 'lldb-Unit :: 
> ObjectFile/ELF/./ObjectFileELFTests/ObjectFileELFTest.TestAARCH64Relocations' 
> FAILED 
>   Note: Google Test filter = ObjectFileELFTest.TestAARCH64Relocations
>   [==] Running 1 test from 1 test case.
>   [--] Global test environment set-up.
>   [--] 1 test from ObjectFileELFTest
>   [ RUN  ] ObjectFileELFTest.TestAARCH64Relocations
>   
> /Users/buildslave/jenkins/workspace/lldb-sanitized/llvm/tools/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp:2741:11:
>  runtime error: store to misaligned address 0x61f01526 for type 
> 'uint32_t' (aka 'unsigned int'), which requires 4 byte alignment
>   0x61f01526: note: pointer points here
>00 00 04 00 00 00  00 00 08 01 00 00 00 00  0c 00 00 00 00 00 00 00  00 00 
> 00 00 00 00 00 00  00 00
>^ 
>   SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior 
> /Users/buildslave/jenkins/workspace/lldb-sanitized/llvm/tools/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp:2741:11
>  in 
>  
>


Went ahead and fixed this in r346244. A post commit review would be appreciated.

-

Davide


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D51566



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D44060: [lldb] Fix "code requires global destructor" warning in g_architecture_mutex

[Davide Italiano via Phabricator via lldb-commits]

davide accepted this revision.
davide added a comment.
This revision is now accepted and ready to land.

LGTM.


https://reviews.llvm.org/D44060



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D44073: [lldb] Refactor ObjC/NSException.cpp (cleanup, avoid code duplication). NFC.

[Davide Italiano via Phabricator via lldb-commits]

davide accepted this revision.
davide added a comment.
This revision is now accepted and ready to land.

Sounds good.


https://reviews.llvm.org/D44073



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D44081: [lldb] Add synthetic frontend for _NSCallStackArray

[Davide Italiano via Phabricator via lldb-commits]

davide requested changes to this revision.
davide added inline comments.
This revision now requires changes to proceed.



Comment at: source/Plugins/Language/ObjC/NSArray.cpp:446-451
+  } else if (class_name == g_NSCallStackArray) {
+Status error;
+value = process_sp->ReadUnsignedIntegerFromMemory(
+valobj_addr + 2 * ptr_size, ptr_size, 0, error);
+if (error.Fail())
+  return false;

Can you add a comment here?
this also seems exactly the same as `NSArrayCF`, any chance we can avoid the 
duplication?


https://reviews.llvm.org/D44081



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D43884: [lldb] Extract more fields from NSException values

[Davide Italiano via Phabricator via lldb-commits]

davide requested changes to this revision.
davide added inline comments.
This revision now requires changes to proceed.



Comment at: source/Plugins/Language/ObjC/NSException.cpp:161-168
+static ConstString g___name("name");
+static ConstString g___reason("reason");
 static ConstString g___userInfo("userInfo");
-if (name == g___userInfo)
-  return 0;
+static ConstString g___reserved("reserved");
+if (name == g___name) return 0;
+if (name == g___reason) return 1;
+if (name == g___userInfo) return 2;

can you clang-format this and add a comment? Thanks.


https://reviews.llvm.org/D43884



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D43884: [lldb] Extract more fields from NSException values

[Davide Italiano via Phabricator via lldb-commits]

davide accepted this revision.
davide added a comment.
This revision is now accepted and ready to land.

LGTM. Thanks.


https://reviews.llvm.org/D43884



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D54431: [lldb] Add "ninja" to svn:ignore

[Davide Italiano via Phabricator via lldb-commits]

davide accepted this revision.
davide added a comment.
This revision is now accepted and ready to land.

This is trivial. I don't know why we didn't do this before.


https://reviews.llvm.org/D54431



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D44081: [lldb] Add synthetic frontend for _NSCallStackArray

[Davide Italiano via Phabricator via lldb-commits]

davide accepted this revision.
davide added a comment.
This revision is now accepted and ready to land.

Thanks for addressing my comments.


https://reviews.llvm.org/D44081



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D54602: Use a shared module cache directory for LLDB.

[Davide Italiano via Phabricator via lldb-commits]

davide accepted this revision.
davide added a comment.

lgtm


https://reviews.llvm.org/D54602



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D54680: Don't use lldb -O in lit tests

[Davide Italiano via Phabricator via lldb-commits]

davide requested changes to this revision.
davide added a comment.
This revision now requires changes to proceed.

Is there a way of fixing this that doesn't require scattering the test between 
two files?


https://reviews.llvm.org/D54680



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D54680: Don't use lldb -O in lit tests

[Davide Italiano via Phabricator via lldb-commits]

davide accepted this revision.
davide added a comment.
This revision is now accepted and ready to land.

I don't have any other great ideas either and I'd say the proposed alternative 
is not worth the complexity.


https://reviews.llvm.org/D54680



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D54863: [ASTImporter] Set MustBuildLookupTable on PrimaryContext

[Davide Italiano via Phabricator via lldb-commits]

davide added a comment.

In D54863#1306947 , @martong wrote:

> We have a change in Clang/ASTImporter which causes an LLDB assertion, unless 
> this patch is applied.
>  The related change is https://reviews.llvm.org/D53655
>
> In that patch we change the importer to properly import the redecl chains of 
> `RecordDecl`s. Consequently the `to_tag_decl` here in LLDB may not always be 
> the PrimaryContext anymore, thus we have to explicitly use the primary 
> context.


Can you write an lldb test for this?


Repository:
  rLLDB LLDB

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D54863/new/

https://reviews.llvm.org/D54863



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D54692: [Driver] Use libOption with tablegen.

[Davide Italiano via Phabricator via lldb-commits]

davide added a comment.

In D54692#1308071 , @clayborg wrote:

> Would be great to see old and new output like Zach suggested. Is there a 
> reason we need to use TableGen? Other command line tools just use llvm:🆑:opt 
> stuff. Seems a bit obtuse to use TableGen?


I'm afraid this is not true.  lld uses TableGen and that was a big success. See 
https://github.com/llvm-mirror/lld/blob/master/ELF/Options.td (considering that 
linkers have to handle all the annoying cmdline compatibility with gcc, e.g. 
options which take single or double underscores).

FWIW, I'm in favour of this change.


Repository:
  rLLDB LLDB

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D54692/new/

https://reviews.llvm.org/D54692



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D54863: [ASTImporter] Set MustBuildLookupTable on PrimaryContext

[Davide Italiano via Phabricator via lldb-commits]

davide added a comment.

In D54863#1308176 , @martong wrote:

> > Can you write an lldb test for this?
>
> There is already an existing test for that:
>
>   2: test_expr_dwarf (TestSharedLib.SharedLibTestCase)
>  Test that types work when defined in a shared library and 
> forward-declared in the main executable ... python: 
> ../../git/llvm/tools/clang/include/clang/AST/DeclBase.h:2298: void 
> clang::DeclContext::setMustBuildLookupTable(): Assertion `this == 
> getPrimaryContext() && "should only be called on primary context"' failed.
>   


Cool, thanks. Is this failing only on linux? Do you happen to have a buildbot 
link that shows the failure?

Thanks!


Repository:
  rLLDB LLDB

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D54863/new/

https://reviews.llvm.org/D54863



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D54616: [Reproducers] Improve reproducer API and add unit tests.

[Davide Italiano via Phabricator via lldb-commits]

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/D54616/new/

https://reviews.llvm.org/D54616



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D55240: [FileSystem] Migrate CommandCompletions

[Davide Italiano via Phabricator via lldb-commits]

davide added a comment.

No objections from me, but I would appreciate if @labath can take a look.


Repository:
  rLLDB LLDB

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55240/new/

https://reviews.llvm.org/D55240



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D55383: Implement basic DidAttach for DynamicLoaderWindowsDYLD for use with ds2

[Davide Italiano via Phabricator via lldb-commits]

davide added subscribers: zturner, labath, davide.
davide added a comment.

You would recommend getting a post-commit review from @labath or @zturner


Repository:
  rLLDB LLDB

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55383/new/

https://reviews.llvm.org/D55383



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D55776: Fix lldb's macosx/heap.py cstr command

[Davide Italiano via Phabricator via lldb-commits]

davide accepted this revision.
davide added a comment.

commit 44b5014ce138fa293238afeaa53cfd4c2f5b12d2 (HEAD -> master)
Author: Davide Italiano 
Date:   Mon Dec 17 10:23:44 2018 -0800

  Fix lldb's macosx/heap.py cstr command.
  
  


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55776/new/

https://reviews.llvm.org/D55776



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D56014: [lldb] - Fix crash when listing the history with the key up.

[Davide Italiano via Phabricator via lldb-commits]

davide added a comment.

testcase?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56014/new/

https://reviews.llvm.org/D56014



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D56170: RangeMap.h: merge RangeDataArray and RangeDataVector

[Davide Italiano via Phabricator via lldb-commits]

davide added inline comments.



Comment at: unittests/Core/RangeMapTest.cpp:52-54
+  // However, this does not always succeed.
+  // TODO: This should probably return the range (0, 40) as well.
+  EXPECT_THAT(Map.FindEntryThatContains(35), nullptr);

I think this is a bug in the implementation.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56170/new/

https://reviews.llvm.org/D56170



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D56170: RangeMap.h: merge RangeDataArray and RangeDataVector

[Davide Italiano via Phabricator via lldb-commits]

davide accepted this revision.
davide added a comment.
This revision is now accepted and ready to land.

LG


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56170/new/

https://reviews.llvm.org/D56170



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D56389: [CMake] Fix standalone builds: make dependency to LLVM's `count` conditional

[Davide Italiano via Phabricator via lldb-commits]

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/D56389/new/

https://reviews.llvm.org/D56389



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D45628: [LLDB] Support GNU-style compressed debug sections (.zdebug)

[Davide Italiano via Phabricator via lldb-commits]

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 rL332162: [LLDB] Support GNU-style compressed debug sections 
(.zdebug) (authored by davide, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D45628?vs=145939&id=146448#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D45628

Files:
  lldb/trunk/packages/Python/lldbsuite/test/linux/compressed-debug-info/Makefile
  
lldb/trunk/packages/Python/lldbsuite/test/linux/compressed-debug-info/TestCompressedDebugInfo.py
  lldb/trunk/packages/Python/lldbsuite/test/linux/compressed-debug-info/a.c
  lldb/trunk/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp

Index: lldb/trunk/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
===
--- lldb/trunk/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
+++ lldb/trunk/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
@@ -1757,6 +1757,63 @@
   return 0;
 }
 
+static SectionType getSectionType(llvm::StringRef section_name) {
+  if (!section_name.startswith(".")) {
+return eSectionTypeOther;
+  }
+
+  // MISSING? .gnu_debugdata - "mini debuginfo / MiniDebugInfo" section,
+  // http://sourceware.org/gdb/onlinedocs/gdb/MiniDebugInfo.html
+  // MISSING? .debug-index -
+  // http://src.chromium.org/viewvc/chrome/trunk/src/build/gdb-add-index?pathrev=144644
+  return llvm::StringSwitch(section_name.drop_front(1))
+  .Case("text", eSectionTypeCode)
+  .Case("data", eSectionTypeData)
+  .Case("bss", eSectionTypeZeroFill)
+  .Case("tdata", eSectionTypeData)
+  .Case("tbss", eSectionTypeZeroFill)
+  // Abbreviations used in the .debug_info section
+  .Case("debug_abbrev", eSectionTypeDWARFDebugAbbrev)
+  .Case("debug_abbrev.dwo", eSectionTypeDWARFDebugAbbrev)
+  .Case("debug_addr", eSectionTypeDWARFDebugAddr)
+  // Lookup table for mapping addresses to compilation units
+  .Case("debug_aranges", eSectionTypeDWARFDebugAranges)
+  .Case("debug_cu_index", eSectionTypeDWARFDebugCuIndex)
+  // Call frame information
+  .Case("debug_frame", eSectionTypeDWARFDebugFrame)
+  // The core DWARF information section
+  .Case("debug_info", eSectionTypeDWARFDebugInfo)
+  .Case("debug_info.dwo", eSectionTypeDWARFDebugInfo)
+  // Line number information
+  .Case("debug_line", eSectionTypeDWARFDebugLine)
+  .Case("debug_line.dwo", eSectionTypeDWARFDebugLine)
+  // Location lists used in DW_AT_location attributes
+  .Case("debug_loc", eSectionTypeDWARFDebugLoc)
+  .Case("debug_loc.dwo", eSectionTypeDWARFDebugLoc)
+  // Macro information
+  .Case("debug_macinfo", eSectionTypeDWARFDebugMacInfo)
+  .Case("debug_macro", eSectionTypeDWARFDebugMacro)
+  .Case("debug_macro.dwo", eSectionTypeDWARFDebugMacro)
+  // Lookup table for mapping object and function names to compilation units
+  .Case("debug_pubnames", eSectionTypeDWARFDebugPubNames)
+  // Lookup table for mapping type names to compilation units
+  .Case("debug_pubtypes", eSectionTypeDWARFDebugPubTypes)
+  // Address ranges used in DW_AT_ranges attributes
+  .Case("debug_ranges", eSectionTypeDWARFDebugRanges)
+  // String table used in .debug_info
+  .Case("debug_str", eSectionTypeDWARFDebugStr)
+  .Case("debug_str.dwo", eSectionTypeDWARFDebugStr)
+  .Case("debug_str_offsets", eSectionTypeDWARFDebugStrOffsets)
+  .Case("debug_str_offsets.dwo", eSectionTypeDWARFDebugStrOffsets)
+  .Case("debug_types", eSectionTypeDWARFDebugTypes)
+  .Case("gnu_debugaltlink", eSectionTypeDWARFGNUDebugAltLink)
+  .Case("eh_frame", eSectionTypeEHFrame)
+  .Case("ARM.exidx", eSectionTypeARMexidx)
+  .Case("ARM.extab", eSectionTypeARMextab)
+  .Case("gosymtab", eSectionTypeGoSymtab)
+  .Default(eSectionTypeOther);
+}
+
 void ObjectFileELF::CreateSections(SectionList &unified_section_list) {
   if (!m_sections_ap.get() && ParseSectionHeaders()) {
 m_sections_ap.reset(new SectionList());
@@ -1771,137 +1828,14 @@
  I != m_section_headers.end(); ++I) {
   const ELFSectionHeaderInfo &header = *I;
 
-  ConstString &name = I->section_name;
+  llvm::StringRef section_name = I->section_name.GetStringRef();
   const uint64_t file_size =
   header.sh_type == SHT_NOBITS ? 0 : header.sh_size;
   const uint64_t vm_size = header.sh_flags & SHF_ALLOC ? header.sh_size : 0;
 
-  static ConstString g_sect_name_text(".text");
-  static ConstString g_sect_name_data(".data");
-  static ConstString g_sect_name_bss(".bss");
-  static ConstString g_sect_name_tdata(".tdata");
-  static ConstString g_sect_name_tbss(".tbss");
-  static ConstString g_sect_name_dwarf_debug_abbrev(".debug_abbrev");
-  static ConstString g_sect_name_dwarf_debug_addr(".debug_addr");
-  static ConstString g_sect_name_dwarf_debug_

[Lldb-commits] [PATCH] D47014: Fix _NSCFBoolean data formatter.

[Davide Italiano via Phabricator via lldb-commits]

davide accepted this revision.
davide added a comment.
This revision is now accepted and ready to land.

LGTM


Repository:
  rL LLVM

https://reviews.llvm.org/D47014



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D47062: Suggest lldb-dotest to reproduce a failure.

[Davide Italiano via Phabricator via lldb-commits]

davide added a comment.

Can you commit the whitespace fixes separately?


Repository:
  rL LLVM

https://reviews.llvm.org/D47062



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D47232: Break dependency from Expression -> Commands

[Davide Italiano via Phabricator via lldb-commits]

davide accepted this revision.
davide added a comment.
This revision is now accepted and ready to land.

Looks fine to me, but let's wait for Pavel. Are you back working on lldb ? :)


https://reviews.llvm.org/D47232



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D48084: [FileSpec] Delegate common operations to llvm::sys::path

[Davide Italiano via Phabricator via lldb-commits]

davide accepted this revision.
davide added a comment.

LGTM with a nit.




Comment at: source/Utility/FileSpec.cpp:18-21
+#include "llvm/ADT/Triple.h"  // for Triple
+#include "llvm/ADT/Twine.h"   // for Twine
+#include "llvm/Support/ErrorOr.h" // for ErrorOr
 #include "llvm/Support/FileSystem.h"

This comments are really too trivial to be useful. Can you remove them?


Repository:
  rL LLVM

https://reviews.llvm.org/D48084



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D48096: Disable warnings for the generated LLDB wrapper source

[Davide Italiano via Phabricator via lldb-commits]

davide accepted this revision.
davide added a comment.
This revision is now accepted and ready to land.

Unfortunate, but this seems reasonable. lg.


https://reviews.llvm.org/D48096



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D48114: Add dataformatter for NSDecimalNumber

[Davide Italiano via Phabricator via lldb-commits]

davide accepted this revision.
davide added a comment.
This revision is now accepted and ready to land.

I didn't check whether the representation was correct (although it looks lo). 
The structure of the patch looks fine to me, thanks for adding the comments :)


Repository:
  rL LLVM

https://reviews.llvm.org/D48114



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D48450: [DataFormatter] Add CFDictionary data formatter

[Davide Italiano via Phabricator via lldb-commits]

davide added a comment.

LGTM.


Repository:
  rL LLVM

https://reviews.llvm.org/D48450



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D48500: [DWARFASTParser] Remove special cases for `llvm-gcc`

[Davide Italiano via Phabricator via lldb-commits]

davide created this revision.
davide added reviewers: friss, aprantl, clayborg, labath.
Herald added a subscriber: JDevlieghere.

To the best of my understanding modern compilers handle all these cases 
correctly. So, I think this is basically dead code.


https://reviews.llvm.org/D48500

Files:
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp


Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -307,14 +307,7 @@
 decl.SetColumn(form_value.Unsigned());
 break;
   case DW_AT_name:
-
 type_name_cstr = form_value.AsCString();
-// Work around a bug in llvm-gcc where they give a name to a
-// reference type which doesn't include the "&"...
-if (tag == DW_TAG_reference_type) {
-  if (strchr(type_name_cstr, '&') == NULL)
-type_name_cstr = NULL;
-}
 if (type_name_cstr)
   type_name_const_str.SetCString(type_name_cstr);
 break;
@@ -558,16 +551,9 @@
 if (attributes.ExtractFormValueAtIndex(i, form_value)) {
   switch (attr) {
   case DW_AT_decl_file:
-if (die.GetCU()->DW_AT_decl_file_attributes_are_invalid()) {
-  // llvm-gcc outputs invalid DW_AT_decl_file attributes that
-  // always point to the compile unit file, so we clear this
-  // invalid value so that we can still unique types
-  // efficiently.
-  decl.SetFile(FileSpec("", false));
-} else
-  decl.SetFile(
-  sc.comp_unit->GetSupportFiles().GetFileSpecAtIndex(
-  form_value.Unsigned()));
+decl.SetFile(
+   sc.comp_unit->GetSupportFiles().GetFileSpecAtIndex(
+  form_value.Unsigned()));
 break;
 
   case DW_AT_decl_line:
@@ -2977,15 +2963,6 @@
 class_language == eLanguageTypeObjC_plus_plus)
   accessibility = eAccessNone;
 
-if (member_idx == 0 && !is_artificial && name &&
-(strstr(name, "_vptr$") == name)) {
-  // Not all compilers will mark the vtable pointer member as
-  // artificial (llvm-gcc). We can't have the virtual members in our
-  // classes otherwise it throws off all child offsets since we end up
-  // having and extra pointer sized member in our class layouts.
-  is_artificial = true;
-}
-
 // Handle static members
 if (is_external && member_byte_offset == UINT32_MAX) {
   Type *var_type = die.ResolveTypeUID(DIERef(encoding_form));


Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -307,14 +307,7 @@
 decl.SetColumn(form_value.Unsigned());
 break;
   case DW_AT_name:
-
 type_name_cstr = form_value.AsCString();
-// Work around a bug in llvm-gcc where they give a name to a
-// reference type which doesn't include the "&"...
-if (tag == DW_TAG_reference_type) {
-  if (strchr(type_name_cstr, '&') == NULL)
-type_name_cstr = NULL;
-}
 if (type_name_cstr)
   type_name_const_str.SetCString(type_name_cstr);
 break;
@@ -558,16 +551,9 @@
 if (attributes.ExtractFormValueAtIndex(i, form_value)) {
   switch (attr) {
   case DW_AT_decl_file:
-if (die.GetCU()->DW_AT_decl_file_attributes_are_invalid()) {
-  // llvm-gcc outputs invalid DW_AT_decl_file attributes that
-  // always point to the compile unit file, so we clear this
-  // invalid value so that we can still unique types
-  // efficiently.
-  decl.SetFile(FileSpec("", false));
-} else
-  decl.SetFile(
-  sc.comp_unit->GetSupportFiles().GetFileSpecAtIndex(
-  form_value.Unsigned()));
+decl.SetFile(
+   sc.comp_unit->GetSupportFiles().GetFileSpecAtIndex(
+  form_value.Unsigned()));
 break;
 
   case DW_AT_decl_line:
@@ -2977,15 +2963,6 @@
 class_language == eLanguageTypeObjC_plus_plus)
   accessibility = eAccessNone;
 
-if (member_idx == 0 && !is_arti

[Lldb-commits] [PATCH] D48500: [DWARFASTParser] Remove special cases for `llvm-gcc`

[Davide Italiano via Phabricator via lldb-commits]

davide added a comment.

I think it's fair and correct supporting everything we can, but I guess 
`llvm-gcc` is largely dead at this point.


https://reviews.llvm.org/D48500



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D48704: [LLDB] Fix for "Bug 37950: ExecutionContext::GetByteOrder() always returns endian::InlHostByteOrder()"

[Davide Italiano via Phabricator via lldb-commits]

davide requested changes to this revision.
davide added a comment.
This revision now requires changes to proceed.

Can you write a unittest for this? Thanks.


https://reviews.llvm.org/D48704



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D49110: [testsuite] Implement a category to skip libstdcxx tests

[Davide Italiano via Phabricator via lldb-commits]

davide created this revision.
davide added a reviewer: labath.
Herald added a subscriber: krytarowski.

On systems where it's not supported.
As far as I understand Linux is the only systems which now ships with libstdcxx 
(maybe NetBSD?, but I'm not entirely sure of the state of lldb on the platform).
We could make this more fine grained looking for the header as we do for 
libcxx. This is a little tricky as there's no such thing as 
`/usr/include/c++/v1`, but libstdcxx encodes the version number in the path 
(i.e.  `/usr/include/c++/5.4`). I guess we might match a regex, but it seems 
fragile to me.

@labath I hope this is more in line with what you had in mind, thanks for your 
feedback!


https://reviews.llvm.org/D49110

Files:
  lldb/packages/Python/lldbsuite/test/dotest.py
  
lldb/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libstdcpp/iterator/TestDataFormatterStdIterator.py
  
lldb/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libstdcpp/list/TestDataFormatterStdList.py
  
lldb/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libstdcpp/map/TestDataFormatterStdMap.py
  
lldb/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libstdcpp/smart_ptr/TestDataFormatterStdSmartPtr.py
  
lldb/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libstdcpp/string/TestDataFormatterStdString.py
  
lldb/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libstdcpp/tuple/TestDataFormatterStdTuple.py
  
lldb/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libstdcpp/unique_ptr/TestDataFormatterStdUniquePtr.py
  
lldb/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libstdcpp/vbool/TestDataFormatterStdVBool.py
  
lldb/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libstdcpp/vector/TestDataFormatterStdVector.py
  lldb/packages/Python/lldbsuite/test/test_categories.py

Index: lldb/packages/Python/lldbsuite/test/test_categories.py
===
--- lldb/packages/Python/lldbsuite/test/test_categories.py
+++ lldb/packages/Python/lldbsuite/test/test_categories.py
@@ -26,6 +26,7 @@
 'gmodules': 'Tests that can be run with -gmodules debug information',
 'expression': 'Tests related to the expression parser',
 'libc++': 'Test for libc++ data formatters',
+'libstdcxx': 'Test for libstdcxx data formatters',
 'objc': 'Tests related to the Objective-C programming language support',
 'pyapi': 'Tests related to the Python API',
 'basic_process': 'Basic process execution sniff tests.',
Index: lldb/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libstdcpp/vector/TestDataFormatterStdVector.py
===
--- lldb/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libstdcpp/vector/TestDataFormatterStdVector.py
+++ lldb/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libstdcpp/vector/TestDataFormatterStdVector.py
@@ -23,9 +23,7 @@
 # Find the line number to break at.
 self.line = line_number('main.cpp', '// Set break point at this line.')
 
-@skipIfFreeBSD
-@skipIfWindows  # libstdcpp not ported to Windows
-@skipIfwatchOS  # libstdcpp not ported to watchos
+@add_test_categories(["libstdcxx"])
 def test_with_run_command(self):
 """Test that that file and class static variables display correctly."""
 self.build()
Index: lldb/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libstdcpp/vbool/TestDataFormatterStdVBool.py
===
--- lldb/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libstdcpp/vbool/TestDataFormatterStdVBool.py
+++ lldb/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libstdcpp/vbool/TestDataFormatterStdVBool.py
@@ -23,12 +23,7 @@
 # Find the line number to break at.
 self.line = line_number('main.cpp', '// Set break point at this line.')
 
-@expectedFailureAll(
-oslist=['freebsd'],
-bugnumber='llvm.org/pr20548 fails to build on lab.llvm.org buildbot')
-@skipIfWindows  # libstdcpp not ported to Windows.
-@skipIfDarwin
-@skipIfwatchOS  # libstdcpp not ported to watchos
+@add_test_categories(["libstdcxx"])
 def test_with_run_command(self):
 """Test that that file and class static variables display correctly."""
 self.build()
Index: lldb/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libstdcpp/unique_ptr/TestDataFormatterStdUniquePtr.py
===
--- l

[Lldb-commits] [PATCH] D49110: [testsuite] Implement a category to skip libstdcxx tests

[Davide Italiano via Phabricator via lldb-commits]

davide added a comment.

@labath this is breaking several bots for us internally so I would really love 
to get this in.
As this seems to be reasonably close to what you ask, I might consider checking 
this in before end of day (PST) today and we can iterate from there.


https://reviews.llvm.org/D49110



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D49110: [testsuite] Implement a category to skip libstdcxx tests

[Davide Italiano via Phabricator via lldb-commits]

davide added a comment.

Thank you Pavel!


https://reviews.llvm.org/D49110



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D49110: [testsuite] Implement a category to skip libstdcxx tests

[Davide Italiano via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL336724: [testsuite] Implement a category to skip libstdcxx 
tests (authored by davide, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D49110?vs=154737&id=154871#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D49110

Files:
  lldb/trunk/packages/Python/lldbsuite/test/dotest.py
  
lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libstdcpp/iterator/TestDataFormatterStdIterator.py
  
lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libstdcpp/list/TestDataFormatterStdList.py
  
lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libstdcpp/map/TestDataFormatterStdMap.py
  
lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libstdcpp/smart_ptr/TestDataFormatterStdSmartPtr.py
  
lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libstdcpp/string/TestDataFormatterStdString.py
  
lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libstdcpp/tuple/TestDataFormatterStdTuple.py
  
lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libstdcpp/unique_ptr/TestDataFormatterStdUniquePtr.py
  
lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libstdcpp/vbool/TestDataFormatterStdVBool.py
  
lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libstdcpp/vector/TestDataFormatterStdVector.py
  lldb/trunk/packages/Python/lldbsuite/test/test_categories.py

Index: lldb/trunk/packages/Python/lldbsuite/test/dotest.py
===
--- lldb/trunk/packages/Python/lldbsuite/test/dotest.py
+++ lldb/trunk/packages/Python/lldbsuite/test/dotest.py
@@ -1104,6 +1104,23 @@
 print("Libc++ tests will not be run because: " + reason)
 configuration.skipCategories.append("libc++")
 
+def canRunLibstdcxxTests():
+from lldbsuite.test import lldbplatformutil
+
+platform = lldbplatformutil.getPlatform()
+if platform == "linux":
+  return True, "libstdcxx always present"
+return False, "Don't know how to build with libstdcxx on %s" % platform
+
+def checkLibstdcxxSupport():
+result, reason = canRunLibstdcxxTests()
+if result:
+return # libstdcxx supported
+if "libstdcxx" in configuration.categoriesList:
+return # libstdcxx category explicitly requested, let it run.
+print("libstdcxx tests will not be run because: " + reason)
+configuration.skipCategories.append("libstdcxx")
+
 def checkDebugInfoSupport():
 import lldb
 
@@ -1228,6 +1245,7 @@
 target_platform = lldb.DBG.GetSelectedPlatform().GetTriple().split('-')[2]
 
 checkLibcxxSupport()
+checkLibstdcxxSupport()
 checkDebugInfoSupport()
 
 # Don't do debugserver tests on anything except OS X.
Index: lldb/trunk/packages/Python/lldbsuite/test/test_categories.py
===
--- lldb/trunk/packages/Python/lldbsuite/test/test_categories.py
+++ lldb/trunk/packages/Python/lldbsuite/test/test_categories.py
@@ -26,6 +26,7 @@
 'gmodules': 'Tests that can be run with -gmodules debug information',
 'expression': 'Tests related to the expression parser',
 'libc++': 'Test for libc++ data formatters',
+'libstdcxx': 'Test for libstdcxx data formatters',
 'objc': 'Tests related to the Objective-C programming language support',
 'pyapi': 'Tests related to the Python API',
 'basic_process': 'Basic process execution sniff tests.',
Index: lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libstdcpp/vector/TestDataFormatterStdVector.py
===
--- lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libstdcpp/vector/TestDataFormatterStdVector.py
+++ lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libstdcpp/vector/TestDataFormatterStdVector.py
@@ -23,9 +23,7 @@
 # Find the line number to break at.
 self.line = line_number('main.cpp', '// Set break point at this line.')
 
-@skipIfFreeBSD
-@skipIfWindows  # libstdcpp not ported to Windows
-@skipIfwatchOS  # libstdcpp not ported to watchos
+@add_test_categories(["libstdcxx"])
 def test_with_run_command(self):
 """Test that that file and class static variables display correctly."""
 self.build()
Index: lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libstdcpp/vbool/TestDataFormatterStdVBool.py
===

[Lldb-commits] [PATCH] D49155: [IRInterpreter] Fix misevaluation of interpretation expressions with `urem`

[Davide Italiano via Phabricator via lldb-commits]

davide created this revision.
davide added reviewers: jingham, jasonmolenda, friss.

`Scalar::MakeUnsigned` was implemented incorrectly so it didn't really change 
the sign of the type (leaving signed types signed).
This showed up as a misevaluation when IR-interpreting `urem` but it's likely 
to arise in other contexts.

This commit fixes the definition, and adds a test to make sure this won't 
regress in future (hopefully).

Fixes rdar://problem/42038760 and LLVM PR38076


https://reviews.llvm.org/D49155

Files:
  lldb/packages/Python/lldbsuite/test/expression_command/rdar42038760/Makefile
  
lldb/packages/Python/lldbsuite/test/expression_command/rdar42038760/TestScalarURem.py
  lldb/packages/Python/lldbsuite/test/expression_command/rdar42038760/main.c
  lldb/source/Core/Scalar.cpp


Index: lldb/source/Core/Scalar.cpp
===
--- lldb/source/Core/Scalar.cpp
+++ lldb/source/Core/Scalar.cpp
@@ -1184,38 +1184,38 @@
   case e_void:
 break;
   case e_sint:
+m_type = e_uint;
 success = true;
 break;
   case e_uint:
-m_type = e_uint;
 success = true;
 break;
   case e_slong:
+m_type = e_ulong;
 success = true;
 break;
   case e_ulong:
-m_type = e_ulong;
 success = true;
 break;
   case e_slonglong:
+m_type = e_ulonglong;
 success = true;
 break;
   case e_ulonglong:
-m_type = e_ulonglong;
 success = true;
 break;
   case e_sint128:
+m_type = e_uint128;
 success = true;
 break;
   case e_uint128:
-m_type = e_uint128;
 success = true;
 break;
   case e_sint256:
+m_type = e_uint256;
 success = true;
 break;
   case e_uint256:
-m_type = e_uint256;
 success = true;
 break;
   case e_float:
Index: 
lldb/packages/Python/lldbsuite/test/expression_command/rdar42038760/main.c
===
--- /dev/null
+++ lldb/packages/Python/lldbsuite/test/expression_command/rdar42038760/main.c
@@ -0,0 +1,19 @@
+// Make sure we IR-interpret the expression correctly.
+
+typedef unsigned int uint32_t;
+struct S0 {
+  signed f2;
+};
+static g_463 = 0x1561983AL;
+void func_1(void)
+{
+  struct S0 l_19;
+  l_19.f2 = 419;
+  uint32_t l_4037 = 4294967295UL;
+  l_19.f2 = g_463; //%self.expect("expr ((l_4037 % (-(g_463))) | l_19.f2)", 
substrs=['(unsigned int) $0 = 358717883'])
+}
+int main()
+{
+  func_1();
+  return 0;
+}
Index: 
lldb/packages/Python/lldbsuite/test/expression_command/rdar42038760/TestScalarURem.py
===
--- /dev/null
+++ 
lldb/packages/Python/lldbsuite/test/expression_command/rdar42038760/TestScalarURem.py
@@ -0,0 +1,4 @@
+from lldbsuite.test import lldbinline
+from lldbsuite.test import decorators
+
+lldbinline.MakeInlineTest(__file__, globals(), None)
Index: 
lldb/packages/Python/lldbsuite/test/expression_command/rdar42038760/Makefile
===
--- /dev/null
+++ lldb/packages/Python/lldbsuite/test/expression_command/rdar42038760/Makefile
@@ -0,0 +1,3 @@
+LEVEL = ../../make
+C_SOURCES := main.c
+include $(LEVEL)/Makefile.rules


Index: lldb/source/Core/Scalar.cpp
===
--- lldb/source/Core/Scalar.cpp
+++ lldb/source/Core/Scalar.cpp
@@ -1184,38 +1184,38 @@
   case e_void:
 break;
   case e_sint:
+m_type = e_uint;
 success = true;
 break;
   case e_uint:
-m_type = e_uint;
 success = true;
 break;
   case e_slong:
+m_type = e_ulong;
 success = true;
 break;
   case e_ulong:
-m_type = e_ulong;
 success = true;
 break;
   case e_slonglong:
+m_type = e_ulonglong;
 success = true;
 break;
   case e_ulonglong:
-m_type = e_ulonglong;
 success = true;
 break;
   case e_sint128:
+m_type = e_uint128;
 success = true;
 break;
   case e_uint128:
-m_type = e_uint128;
 success = true;
 break;
   case e_sint256:
+m_type = e_uint256;
 success = true;
 break;
   case e_uint256:
-m_type = e_uint256;
 success = true;
 break;
   case e_float:
Index: lldb/packages/Python/lldbsuite/test/expression_command/rdar42038760/main.c
===
--- /dev/null
+++ lldb/packages/Python/lldbsuite/test/expression_command/rdar42038760/main.c
@@ -0,0 +1,19 @@
+// Make sure we IR-interpret the expression correctly.
+
+typedef unsigned int uint32_t;
+struct S0 {
+  signed f2;
+};
+static g_463 = 0x1561983AL;
+void func_1(void)
+{
+  struct S0 l_19;
+  l_19.f2 = 419;
+  uint32_t l_4037 = 4294967295UL;
+  l_19.f2 = g_463; //%self.expect("expr ((l_4037 % (-(g_463))) | l_19.f2)", substrs=['(unsigned int) $0 = 358717883'])
+}
+int main()
+{
+  func_1();
+  return 0;
+}
Index: lldb/packages/Python/lldbsuite/test/expression_command/rdar42038760/TestScalarURem.py

[Lldb-commits] [PATCH] D49155: [IRInterpreter] Fix misevaluation of interpretation expressions with `urem`

[Davide Italiano via Phabricator via lldb-commits]

davide accepted this revision.
davide added a comment.
This revision is now accepted and ready to land.

Ack'ed by Jim offline.


https://reviews.llvm.org/D49155



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D49155: [IRInterpreter] Fix misevaluation of interpretation expressions with `urem`

[Davide Italiano via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL336872: [IRInterpreter] Fix misevaluation of interpretation 
expressions with `urem`. (authored by davide, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D49155?vs=154884&id=155096#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D49155

Files:
  
lldb/trunk/packages/Python/lldbsuite/test/expression_command/rdar42038760/Makefile
  
lldb/trunk/packages/Python/lldbsuite/test/expression_command/rdar42038760/TestScalarURem.py
  
lldb/trunk/packages/Python/lldbsuite/test/expression_command/rdar42038760/main.c
  lldb/trunk/source/Core/Scalar.cpp


Index: lldb/trunk/source/Core/Scalar.cpp
===
--- lldb/trunk/source/Core/Scalar.cpp
+++ lldb/trunk/source/Core/Scalar.cpp
@@ -1184,38 +1184,38 @@
   case e_void:
 break;
   case e_sint:
+m_type = e_uint;
 success = true;
 break;
   case e_uint:
-m_type = e_uint;
 success = true;
 break;
   case e_slong:
+m_type = e_ulong;
 success = true;
 break;
   case e_ulong:
-m_type = e_ulong;
 success = true;
 break;
   case e_slonglong:
+m_type = e_ulonglong;
 success = true;
 break;
   case e_ulonglong:
-m_type = e_ulonglong;
 success = true;
 break;
   case e_sint128:
+m_type = e_uint128;
 success = true;
 break;
   case e_uint128:
-m_type = e_uint128;
 success = true;
 break;
   case e_sint256:
+m_type = e_uint256;
 success = true;
 break;
   case e_uint256:
-m_type = e_uint256;
 success = true;
 break;
   case e_float:
Index: 
lldb/trunk/packages/Python/lldbsuite/test/expression_command/rdar42038760/Makefile
===
--- 
lldb/trunk/packages/Python/lldbsuite/test/expression_command/rdar42038760/Makefile
+++ 
lldb/trunk/packages/Python/lldbsuite/test/expression_command/rdar42038760/Makefile
@@ -0,0 +1,3 @@
+LEVEL = ../../make
+C_SOURCES := main.c
+include $(LEVEL)/Makefile.rules
Index: 
lldb/trunk/packages/Python/lldbsuite/test/expression_command/rdar42038760/main.c
===
--- 
lldb/trunk/packages/Python/lldbsuite/test/expression_command/rdar42038760/main.c
+++ 
lldb/trunk/packages/Python/lldbsuite/test/expression_command/rdar42038760/main.c
@@ -0,0 +1,19 @@
+// Make sure we IR-interpret the expression correctly.
+
+typedef unsigned int uint32_t;
+struct S0 {
+  signed f2;
+};
+static g_463 = 0x1561983AL;
+void func_1(void)
+{
+  struct S0 l_19;
+  l_19.f2 = 419;
+  uint32_t l_4037 = 4294967295UL;
+  l_19.f2 = g_463; //%self.expect("expr ((l_4037 % (-(g_463))) | l_19.f2)", 
substrs=['(unsigned int) $0 = 358717883'])
+}
+int main()
+{
+  func_1();
+  return 0;
+}
Index: 
lldb/trunk/packages/Python/lldbsuite/test/expression_command/rdar42038760/TestScalarURem.py
===
--- 
lldb/trunk/packages/Python/lldbsuite/test/expression_command/rdar42038760/TestScalarURem.py
+++ 
lldb/trunk/packages/Python/lldbsuite/test/expression_command/rdar42038760/TestScalarURem.py
@@ -0,0 +1,4 @@
+from lldbsuite.test import lldbinline
+from lldbsuite.test import decorators
+
+lldbinline.MakeInlineTest(__file__, globals(), None)


Index: lldb/trunk/source/Core/Scalar.cpp
===
--- lldb/trunk/source/Core/Scalar.cpp
+++ lldb/trunk/source/Core/Scalar.cpp
@@ -1184,38 +1184,38 @@
   case e_void:
 break;
   case e_sint:
+m_type = e_uint;
 success = true;
 break;
   case e_uint:
-m_type = e_uint;
 success = true;
 break;
   case e_slong:
+m_type = e_ulong;
 success = true;
 break;
   case e_ulong:
-m_type = e_ulong;
 success = true;
 break;
   case e_slonglong:
+m_type = e_ulonglong;
 success = true;
 break;
   case e_ulonglong:
-m_type = e_ulonglong;
 success = true;
 break;
   case e_sint128:
+m_type = e_uint128;
 success = true;
 break;
   case e_uint128:
-m_type = e_uint128;
 success = true;
 break;
   case e_sint256:
+m_type = e_uint256;
 success = true;
 break;
   case e_uint256:
-m_type = e_uint256;
 success = true;
 break;
   case e_float:
Index: lldb/trunk/packages/Python/lldbsuite/test/expression_command/rdar42038760/Makefile
===
--- lldb/trunk/packages/Python/lldbsuite/test/expression_command/rdar42038760/Makefile
+++ lldb/trunk/packages/Python/lldbsuite/test/expression_command/rdar42038760/Makefile
@@ -0,0 +1,3 @@
+LEVEL = ../../make
+C_SOURCES := main.c
+include $(LEVEL)/Makefile.rules
Index: lldb/trunk/packages/Python/lldbsuite/test/expression_command/rdar42038760/main.c

[Lldb-commits] [PATCH] D49271: Adding libc++ formattors for std::optional

[Davide Italiano via Phabricator via lldb-commits]

davide added a comment.

Some comments. Jonas looked at many formatters recently so he's in a good 
position to take a look.




Comment at: 
packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/optional/Makefile:9
+CXXFLAGS += -std=c++17
+#CFLAGS += -std=c++17

commented out code?



Comment at: 
packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/optional/TestDataFormatterLibcxxOptional.py:21-22
+@add_test_categories(["libc++"])
+##@skipIf(debug_info="gmodules",
+##bugnumber="https://bugs.llvm.org/show_bug.cgi?id=36048";)
+def test_with_run_command(self):

ditto



Comment at: 
packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/optional/main.cpp:14-38
+optional_int number ;
+
+printf( "%d\n", number.has_value() ) ; // break here
+
+number = 42 ;
+
+printf( "%d\n", *number ) ; // break here

You don't really need to se all these breakpoints.
You just need set one on the return and print all the types.
Also, an `lldbInline` python test here should suffice and it's probably more 
readable (grep for lldbInline).



Comment at: source/Plugins/Language/CPlusPlus/CMakeLists.txt:14-15
   LibCxxTuple.cpp
+  LibCxxOptional.cpp
   LibCxxUnorderedMap.cpp
   LibCxxVector.cpp

Please sort this.



Comment at: source/Plugins/Language/CPlusPlus/LibCxx.cpp:48-51
+  const char *engaged_as_cstring = engaged_sp->GetValueAsUnsigned(0) == 1 ? 
"true" : "false" ;
+
+  stream.Printf(" engaged=%s",  engaged_as_cstring );
+

Can you use StringRef?



Comment at: source/Plugins/Language/CPlusPlus/LibCxx.h:30-33
+bool LibcxxOptionalSummaryProvider(
+ValueObject &valobj, Stream &stream,
+const TypeSummaryOptions
+&options); // libc++ std::optional<>

Please clang-format this patch.



Comment at: source/Plugins/Language/CPlusPlus/LibCxxOptional.cpp:59
+
+  //ValueObjectSP val_sp( 
m_backend.GetChildMemberWithName(ConstString("__val_"), true));
+  ValueObjectSP val_sp( 
m_backend.GetChildMemberWithName(ConstString("__engaged_") , 
true)->GetParent()->GetChildAtIndex(0,true)->GetChildMemberWithName(ConstString("__val_"),
 true) );

Please remove this commented out code.



Comment at: source/Plugins/Language/CPlusPlus/LibCxxOptional.cpp:63
+  if (!val_sp) {
+  fprintf( stderr, "no __val_!\n" ) ;
+return ValueObjectSP();

if you want to log diagnostics, you might consider using the `LOG` facility and 
then add these to the `data formatters` channel.


https://reviews.llvm.org/D49271



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D49271: Adding libc++ formattors for std::optional

[Davide Italiano via Phabricator via lldb-commits]

davide added a comment.

This is getting really close. Please try the `lldbInline` test format and 
revert the unrelated bits and I'll take another look.
 Thanks!




Comment at: source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp:971-972
 g_formatters.push_back([](lldb_private::ValueObject &valobj,
-  lldb::DynamicValueType,
-  FormatManager &
-  fmt_mgr) -> SyntheticChildren::SharedPointer 
{
+  lldb::DynamicValueType, FormatManager &fmt_mgr)
+   -> SyntheticChildren::SharedPointer {
   static CXXSyntheticChildren::SharedPointer formatter_sp(

this looks unrelated?



Comment at: source/Plugins/Language/CPlusPlus/LibCxx.cpp:179-223
+  CompilerType pair_type(
+  __i_->GetCompilerType().GetTypeTemplateArgument(0));
+  std::string name;
+  uint64_t bit_offset_ptr;
+  uint32_t bitfield_bit_size_ptr;
+  bool is_bitfield_ptr;
+  pair_type = pair_type.GetFieldAtIndex(

this looks like .. unrelated?


https://reviews.llvm.org/D49271



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D49271: Adding libc++ formattors for std::optional

[Davide Italiano via Phabricator via lldb-commits]

davide requested changes to this revision.
davide added a comment.
This revision now requires changes to proceed.

Probably last round of comments. Thanks for your patience!




Comment at: 
packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/optional/Makefile:7
+include $(LEVEL)/Makefile.rules
+CXXFLAGS += -O0
+CXXFLAGS += -std=c++17

you don't need this, it's implicit (`-O0`)



Comment at: 
packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/optional/TestDataFormatterLibcxxOptional.py:8
+
+lldbinline.MakeInlineTest(__file__, globals(), [no_debug_info_test])

I do wonder if you need a decorator to indicate that this is a libcxx only test 
(and skip everywhere the library isn't supported).



Comment at: source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp:496
+  "libc++ std::optional synthetic children",
+  ConstString("^std::__(ndk)?1::optional<.+>(( )?&)?$"),
+  stl_synth_flags, true);

I'm a little nervous about this regex, but I think it's fine and I'll let Jim 
take another look in case I missed something.



Comment at: source/Plugins/Language/CPlusPlus/LibCxx.cpp:42-51
+  ValueObjectSP engaged_sp(
+  valobj_sp->GetChildMemberWithName(ConstString("__engaged_"), true));
+
+  if (!engaged_sp)
+return false;
+
+  llvm::StringRef engaged_as_cstring(

This is really obscure to somebody who doesn't understand the type internally. 
Can you add a comment explaining the structure? (here or in the synthetic)


https://reviews.llvm.org/D49271



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D48351: Move dumping code out of RegisterValue class

[Davide Italiano via Phabricator via lldb-commits]

davide added a comment.

I guess this is fine. @jingham?


https://reviews.llvm.org/D48351



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D49612: Use LLVM's new ItaniumPartialDemangler in LLDB

[Davide Italiano via Phabricator via lldb-commits]

davide added a comment.

Thanks for doing this.
We may consider doing some A-B testing between the two demanglers.
A strategy that worked very well for similar purposes was that of running `nm` 
on a large executable (e.g. clang or lldb itself) and see whether we demangle 
in the same exact way and measure the time needed for demangling.


https://reviews.llvm.org/D49612



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D49695: [cmake] Remove unused ${LLDB_PLUGINS} dependency from our Objective-C++ CMake config

[Davide Italiano via Phabricator via lldb-commits]

davide accepted this revision.
davide added a comment.
This revision is now accepted and ready to land.

lg


https://reviews.llvm.org/D49695



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D49271: Adding libc++ formattors for std::optional

[Davide Italiano via Phabricator via lldb-commits]

davide accepted this revision.
davide added a comment.
This revision is now accepted and ready to land.

This is good, but please add a comment explaining the type before committing.


https://reviews.llvm.org/D49271



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D49708: Added unit test for StreamTee

[Davide Italiano via Phabricator via lldb-commits]

davide accepted this revision.
davide added a comment.
This revision is now accepted and ready to land.

lg. You probably don't need pre-commit reviews for adding tests. This is 
obvious goodness.


https://reviews.llvm.org/D49708



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D49322: Narrow the CompletionRequest API to being append-only.

[Davide Italiano via Phabricator via lldb-commits]

davide added inline comments.



Comment at: source/Commands/CommandCompletions.cpp:251-261
   request.SetWordComplete(false);
   StandardTildeExpressionResolver Resolver;
-  return DiskDirectories(request.GetCursorArgumentPrefix(),
- request.GetMatches(), Resolver);
+  StringList matches;
+  int result =
+  DiskDirectories(request.GetCursorArgumentPrefix(), matches, Resolver);
+  request.AddCompletions(matches);
+

Can you please merge these two functions? They're pretty much the same.



Comment at: source/Commands/CommandObjectMultiword.cpp:378
 return proxy_command->HandleArgumentCompletion(request, 
opt_element_vector);
-  request.GetMatches().Clear();
   return 0;

I'm not sure not calling `Clear` here is correct.



Comment at: source/Interpreter/Options.cpp:731-741
-  // The options definitions table has duplicates because of the
-  // way the grouping information is stored, so only add once.
-  bool duplicate = false;
-  for (size_t k = 0; k < request.GetMatches().GetSize(); k++) {
-if (request.GetMatches().GetStringAtIndex(k) == full_name) {
-  duplicate = true;
-  break;

Why did you remove this code?


https://reviews.llvm.org/D49322



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D49271: Adding libc++ formattors for std::optional

[Davide Italiano via Phabricator via lldb-commits]

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:
  https://reviews.llvm.org/D49271?vs=157337&id=157352#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D49271

Files:
  
lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/optional/Makefile
  
lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/optional/TestDataFormatterLibcxxOptional.py
  
lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/optional/main.cpp
  lldb/trunk/source/Plugins/Language/CPlusPlus/LibCxxOptional.cpp

Index: lldb/trunk/source/Plugins/Language/CPlusPlus/LibCxxOptional.cpp
===
--- lldb/trunk/source/Plugins/Language/CPlusPlus/LibCxxOptional.cpp
+++ lldb/trunk/source/Plugins/Language/CPlusPlus/LibCxxOptional.cpp
@@ -0,0 +1,85 @@
+//===-- LibCxxOptional.cpp --*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "LibCxx.h"
+#include "lldb/DataFormatters/FormattersHelpers.h"
+
+using namespace lldb;
+using namespace lldb_private;
+
+namespace {
+
+class OptionalFrontEnd : public SyntheticChildrenFrontEnd {
+public:
+  OptionalFrontEnd(ValueObject &valobj) : SyntheticChildrenFrontEnd(valobj) {
+Update();
+  }
+
+  size_t GetIndexOfChildWithName(const ConstString &name) override {
+return formatters::ExtractIndexFromString(name.GetCString());
+  }
+
+  bool MightHaveChildren() override { return true; }
+  bool Update() override;
+  size_t CalculateNumChildren() override { return m_size; }
+  ValueObjectSP GetChildAtIndex(size_t idx) override;
+
+private:
+  size_t m_size = 0;
+  ValueObjectSP m_base_sp;
+};
+} // namespace
+
+bool OptionalFrontEnd::Update() {
+  ValueObjectSP engaged_sp(
+  m_backend.GetChildMemberWithName(ConstString("__engaged_"), true));
+
+  if (!engaged_sp)
+return false;
+
+  // __engaged_ is a bool flag and is true if the optional contains a value.
+  // Converting it to unsigned gives us a size of 1 if it contains a value
+  // and 0 if not.
+  m_size = engaged_sp->GetValueAsUnsigned(0);
+
+  return false;
+}
+
+ValueObjectSP OptionalFrontEnd::GetChildAtIndex(size_t idx) {
+  if (idx >= m_size)
+return ValueObjectSP();
+
+  // __val_ contains the underlying value of an optional if it has one.
+  // Currently because it is part of an anonymous union GetChildMemberWithName()
+  // does not peer through and find it unless we are at the parent itself.
+  // We can obtain the parent through __engaged_.
+  ValueObjectSP val_sp(
+  m_backend.GetChildMemberWithName(ConstString("__engaged_"), true)
+  ->GetParent()
+  ->GetChildAtIndex(0, true)
+  ->GetChildMemberWithName(ConstString("__val_"), true));
+
+  if (!val_sp)
+return ValueObjectSP();
+
+  CompilerType holder_type = val_sp->GetCompilerType();
+
+  if (!holder_type)
+return ValueObjectSP();
+
+  return val_sp->Clone(ConstString(llvm::formatv("Value").str()));
+}
+
+SyntheticChildrenFrontEnd *
+formatters::LibcxxOptionalFrontEndCreator(CXXSyntheticChildren *,
+  lldb::ValueObjectSP valobj_sp) {
+  if (valobj_sp)
+return new OptionalFrontEnd(*valobj_sp);
+  return nullptr;
+}
Index: lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/optional/Makefile
===
--- lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/optional/Makefile
+++ lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/optional/Makefile
@@ -0,0 +1,7 @@
+LEVEL = ../../../../../make
+
+CXX_SOURCES := main.cpp
+
+USE_LIBCPP := 1
+include $(LEVEL)/Makefile.rules
+CXXFLAGS += -std=c++17
Index: lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/optional/main.cpp
===
--- lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/optional/main.cpp
+++ lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/optional/main.cpp
@@ -0,0 +1,27 @@
+#include 
+#include 
+#include 
+#include 
+
+using int_vect = std::vector ;
+using optional_int = std::optional ;
+using optional_int_vect = std::optional ;
+using optional_string = std::optional ;

[Lldb-commits] [PATCH] D49866: Fix duplicate suggestions after an ambiguous command

[Davide Italiano via Phabricator via lldb-commits]

davide accepted this revision.
davide added a comment.
This revision is now accepted and ready to land.

LG. thanks for improving the interface, I think all these cleanups are really 
good.


https://reviews.llvm.org/D49866



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D49322: Narrow the CompletionRequest API to being append-only.

[Davide Italiano via Phabricator via lldb-commits]

davide accepted this revision.
davide added a comment.
This revision is now accepted and ready to land.

Although I'm not entirely sure whether this is safe (as in, it doesn't break 
anything), it's probably not worth to block this further. My understanding is 
that Raphael did a bunch of testing and he's willing to follow-up on eventual 
regressions. So, given this passes the test suite, I think it's OK for this 
patch to go in.


https://reviews.llvm.org/D49322



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D49271: Adding libc++ formattors for std::optional

[Davide Italiano via Phabricator via lldb-commits]

davide added a comment.

Your latest update doesn't contain CMakeList.txt files and the Xcode project 
changes. That's why it failed to apply. Please upload a correct version and 
I'll commit it for you.


Repository:
  rL LLVM

https://reviews.llvm.org/D49271



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D49271: Adding libc++ formattors for std::optional

[Davide Italiano via Phabricator via lldb-commits]

davide added a comment.

Recommitted:

  a11963323fc Recommit [DataFormatters] Add formatter for C++17 std::optional.

Authentication realm: https://llvm.org:443 LLVM Subversion repository
Password for 'davide': ***

Sendinglldb/trunk/lldb.xcodeproj/project.pbxproj
Adding 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/optional
Adding 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/optional/Makefile
Adding 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/optional/TestDataFormatterLibcxxOptional.py
Adding 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/optional/main.cpp
Sendinglldb/trunk/source/Plugins/Language/CPlusPlus/CMakeLists.txt
Sending
lldb/trunk/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
Sendinglldb/trunk/source/Plugins/Language/CPlusPlus/LibCxx.cpp
Sendinglldb/trunk/source/Plugins/Language/CPlusPlus/LibCxx.h
Adding lldb/trunk/source/Plugins/Language/CPlusPlus/LibCxxOptional.cpp
Transmitting file data .done
Committing transaction...
Committed revision 338156.
Committed a11963323fc to svn.


Repository:
  rL LLVM

https://reviews.llvm.org/D49271



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D49271: Adding libc++ formattors for std::optional

[Davide Italiano via Phabricator via lldb-commits]

davide added a comment.

I'm afraid I had to revert this again. It broke an ubuntu lldb-cmake bot which 
builds with clang-3.5 (which has no `-std=c++17` support flag, as 17 wasn't 
there yet).

I'd recommend to update this patch with another decorator to skip places where 
`-std=c++17` is not available doesn't work, and then we'll try again.


Repository:
  rL LLVM

https://reviews.llvm.org/D49271



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D49271: Adding libc++ formattors for std::optional

[Davide Italiano via Phabricator via lldb-commits]

davide added a comment.

Bot logs for completeness:

  Session logs for test failures/errors/unexpected successes will go into 
directory 'logs-clang-3.5-i386'
  Command invoked: 
/lldb-buildbot/lldbSlave/buildWorkingDir/scripts/../llvm/tools/lldb/test/dotest.py
 -A i386 -C clang-3.5 -v -s logs-clang-3.5-i386 -u CXXFLAGS -u CFLAGS --env 
ARCHIVER=ar --env OBJCOPY=objcopy --executable 
/lldb-buildbot/lldbSlave/buildWorkingDir/scripts/../build/bin/lldb --channel 
gdb-remote packets --channel lldb all --skip-category watchpoint 
--skip-category lldb-mi --results-port 43584 -S fnmac --inferior -p 
TestDataFormatterLibcxxOptional.py 
/lldb-buildbot/lldbSlave/buildWorkingDir/llvm/tools/lldb/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/optional
 --event-add-entries worker_index=4:int
  
  Configuration: arch=i386 compiler=/usr/bin/clang-3.5
  --
  Collected 4 tests
  
  1: test_with_run_command_dsym 
(TestDataFormatterLibcxxOptional.LibcxxOptionalDataFormatterTestCase)
 Test that that file and class static variables display correctly. ... 
skipped 'test case does not fall in any category of interest for this run'
  2: test_with_run_command_dwarf 
(TestDataFormatterLibcxxOptional.LibcxxOptionalDataFormatterTestCase)
 Test that that file and class static variables display correctly. ... 3: 
test_with_run_command_dwo 
(TestDataFormatterLibcxxOptional.LibcxxOptionalDataFormatterTestCase)
 Test that that file and class static variables display correctly. ... 4: 
test_with_run_command_gmodules 
(TestDataFormatterLibcxxOptional.LibcxxOptionalDataFormatterTestCase)
 Test that that file and class static variables display correctly. ... 
skipped 'test case does not fall in any category of interest for this run'
  
  ==
  ERROR: test_with_run_command_dwarf 
(TestDataFormatterLibcxxOptional.LibcxxOptionalDataFormatterTestCase)
 Test that that file and class static variables display correctly.
  --
  Error when building test subject.
  
  Build Command:
  make 
VPATH=/lldb-buildbot/lldbSlave/buildWorkingDir/llvm/tools/lldb/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/optional
 -C 
/lldb-buildbot/lldbSlave/buildWorkingDir/scripts/lldb-test-build.noindex/functionalities/data-formatter/data-formatter-stl/libcxx/optional/TestDataFormatterLibcxxOptional.test_with_run_command_dwarf
 -I 
/lldb-buildbot/lldbSlave/buildWorkingDir/llvm/tools/lldb/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/optional
 -f 
/lldb-buildbot/lldbSlave/buildWorkingDir/llvm/tools/lldb/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/optional/Makefile
 MAKE_DSYM=NO ARCH=i386 CC="/usr/bin/clang-3.5" 
  
  Build Command Output:
  error: invalid value 'c++17' in '-std=c++17'
  error: invalid value 'c++17' in '-std=c++17'
  make: *** [main.o] Error 1


Repository:
  rL LLVM

https://reviews.llvm.org/D49271



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D50026: Remove Stream::UnitTest

[Davide Italiano via Phabricator via lldb-commits]

davide accepted this revision.
davide added a comment.
This revision is now accepted and ready to land.

LGTM.


https://reviews.llvm.org/D50026



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D48500: [DWARFASTParser] Remove special cases for `llvm-gcc`

[Davide Italiano via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL338638: [DWARFASTParser] Remove special cases for `llvm-gcc` 
(authored by davide, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D48500?vs=152523&id=158633#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D48500

Files:
  lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp


Index: lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
===
--- lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -307,14 +307,7 @@
 decl.SetColumn(form_value.Unsigned());
 break;
   case DW_AT_name:
-
 type_name_cstr = form_value.AsCString();
-// Work around a bug in llvm-gcc where they give a name to a
-// reference type which doesn't include the "&"...
-if (tag == DW_TAG_reference_type) {
-  if (strchr(type_name_cstr, '&') == NULL)
-type_name_cstr = NULL;
-}
 if (type_name_cstr)
   type_name_const_str.SetCString(type_name_cstr);
 break;
@@ -558,16 +551,9 @@
 if (attributes.ExtractFormValueAtIndex(i, form_value)) {
   switch (attr) {
   case DW_AT_decl_file:
-if (die.GetCU()->DW_AT_decl_file_attributes_are_invalid()) {
-  // llvm-gcc outputs invalid DW_AT_decl_file attributes that
-  // always point to the compile unit file, so we clear this
-  // invalid value so that we can still unique types
-  // efficiently.
-  decl.SetFile(FileSpec("", false));
-} else
-  decl.SetFile(
-  sc.comp_unit->GetSupportFiles().GetFileSpecAtIndex(
-  form_value.Unsigned()));
+decl.SetFile(
+   sc.comp_unit->GetSupportFiles().GetFileSpecAtIndex(
+  form_value.Unsigned()));
 break;
 
   case DW_AT_decl_line:
@@ -2977,15 +2963,6 @@
 class_language == eLanguageTypeObjC_plus_plus)
   accessibility = eAccessNone;
 
-if (member_idx == 0 && !is_artificial && name &&
-(strstr(name, "_vptr$") == name)) {
-  // Not all compilers will mark the vtable pointer member as
-  // artificial (llvm-gcc). We can't have the virtual members in our
-  // classes otherwise it throws off all child offsets since we end up
-  // having and extra pointer sized member in our class layouts.
-  is_artificial = true;
-}
-
 // Handle static members
 if (is_external && member_byte_offset == UINT32_MAX) {
   Type *var_type = die.ResolveTypeUID(DIERef(encoding_form));


Index: lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
===
--- lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -307,14 +307,7 @@
 decl.SetColumn(form_value.Unsigned());
 break;
   case DW_AT_name:
-
 type_name_cstr = form_value.AsCString();
-// Work around a bug in llvm-gcc where they give a name to a
-// reference type which doesn't include the "&"...
-if (tag == DW_TAG_reference_type) {
-  if (strchr(type_name_cstr, '&') == NULL)
-type_name_cstr = NULL;
-}
 if (type_name_cstr)
   type_name_const_str.SetCString(type_name_cstr);
 break;
@@ -558,16 +551,9 @@
 if (attributes.ExtractFormValueAtIndex(i, form_value)) {
   switch (attr) {
   case DW_AT_decl_file:
-if (die.GetCU()->DW_AT_decl_file_attributes_are_invalid()) {
-  // llvm-gcc outputs invalid DW_AT_decl_file attributes that
-  // always point to the compile unit file, so we clear this
-  // invalid value so that we can still unique types
-  // efficiently.
-  decl.SetFile(FileSpec("", false));
-} else
-  decl.SetFile(
-  sc.comp_unit->GetSupportFiles().GetFileSpecAtIndex(
-  form_value.Unsigned()));
+decl.SetFile(
+   sc.comp_unit->GetSupportFiles().GetFileSpecAtIndex(
+  form_value.Unsigned()));
 break;
 
   case DW_AT_decl_line:
@@ -2977,15 +2963,6 @@
 cla

[Lldb-commits] [PATCH] D50192: Fix: ClangHighlighter.cpp should not be in CopyFiles, but in lldb-core targets

[Davide Italiano via Phabricator via lldb-commits]

davide added a comment.

My understanding is that @t.p.northover just committed the same patch.


https://reviews.llvm.org/D50192



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D50317: Remove duplicated code in CommandObjectQuit

[Davide Italiano via Phabricator via lldb-commits]

davide accepted this revision.
davide added a comment.
This revision is now accepted and ready to land.

Sure.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D50317



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D50271: [IRMemoryMap] Shrink Allocation make it move-only (NFC)

[Davide Italiano via Phabricator via lldb-commits]

davide accepted this revision.
davide added a comment.
This revision is now accepted and ready to land.

Looks reasonable to me.


https://reviews.llvm.org/D50271



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D50677: Remove manual byte counting from Opcode::Dump

[Davide Italiano via Phabricator via lldb-commits]

davide accepted this revision.
davide added a comment.
This revision is now accepted and ready to land.

LGTM.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D50677



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D51319: Use a RAII guard to control access to DisassemblerLLVMC.

[Davide Italiano via Phabricator via lldb-commits]

davide accepted this revision.
davide added a comment.
This revision is now accepted and ready to land.

LGTM if Vedant is happy with this.


https://reviews.llvm.org/D51319



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D51374: [LLDB] Fix script to work with GNU sed

[Davide Italiano via Phabricator via lldb-commits]

davide added a comment.

LGTM


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D51374



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

[Davide Italiano via Phabricator via lldb-commits]

davide added inline comments.



Comment at: include/lldb/DataFormatters/ValueObjectPrinter.h:146
+
+#define LLDB_VOP ValueObjectPrinter
+  LazyBoolMember m_should_print;

can you typedef?


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D51557



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

[Davide Italiano via Phabricator via lldb-commits]

davide added a comment.

I agree.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D51557



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D51816: Fix raw address breakpoints not resolving

[Davide Italiano via Phabricator via lldb-commits]

davide added a comment.

Can you add a test for this behavior?


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D51816



___
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'

[Davide Italiano via Phabricator via lldb-commits]

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

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'

[Davide Italiano via Phabricator via lldb-commits]

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
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

[Davide Italiano via Phabricator via lldb-commits]

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
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D51999: build: add libedit to include paths

[Davide Italiano via Phabricator via lldb-commits]

davide added a comment.

How exactly are you building (i.e. what triggers this error)?


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D51999



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D52247: Refactor FindVariable() core functionality into StackFrame out of SBFrame

[Davide Italiano via Phabricator via lldb-commits]

davide accepted this revision.
davide added a comment.
This revision is now accepted and ready to land.

LGTM modulo minor.




Comment at: source/Target/StackFrame.cpp:1733-1738
+if (sc.block->AppendVariables(
+can_create, get_parent_variables, 
stop_if_block_is_inlined_function,
+[this](Variable *v) { return v->IsInScope(this); },
+&variable_list)) {
+  var_sp = variable_list.FindVariable(ConstString(name));
+}

This is fairly unreadable IMHO. If I were you, I would hoist the lambda out.


https://reviews.llvm.org/D52247



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D52247: Refactor FindVariable() core functionality into StackFrame out of SBFrame

[Davide Italiano via Phabricator via lldb-commits]

davide added inline comments.



Comment at: include/lldb/Target/StackFrame.h:506-507
 
+  lldb::ValueObjectSP FindVariable(const char *name);
+
   //--

Also, I think you might want to add a doxygen comment here, as this function is 
now part of the API.


https://reviews.llvm.org/D52247



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D52247: Refactor FindVariable() core functionality into StackFrame out of SBFrame

[Davide Italiano via Phabricator via lldb-commits]

davide accepted this revision.
davide added a comment.

Sure.


https://reviews.llvm.org/D52247



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D39689: Add a dependency from check-lldb on lld

[Davide Italiano via Phabricator via lldb-commits]

davide added a subscriber: zturner.
davide added a comment.

Not sure lld is the default? I think I always build with bfd (or gold).
I'll check later today when I'm in the office. I'm not against this change per 
se, but adding another dependency seems annoying. cc: @zturner


https://reviews.llvm.org/D39689



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D40519: Remove some duplicated code in UUID.cpp

[Davide Italiano via Phabricator via lldb-commits]

davide accepted this revision.
davide added a comment.

LGTM


https://reviews.llvm.org/D40519



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D40536: Simplify UUID constructors

[Davide Italiano via Phabricator via lldb-commits]

davide accepted this revision.
davide added a comment.
This revision is now accepted and ready to land.

LGTM


https://reviews.llvm.org/D40536



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D40537: Simplify UUID::IsValid()

[Davide Italiano via Phabricator via lldb-commits]

davide added a comment.

Yes, what Zachary said. Thanks for the cleanup. There's a decent amount of code 
in lldb that can be written in a very compact way but it's manually unrolled. I 
think in this case the code produced should be equivalent (and if not, I'd be 
curious to see the codegen).
More generally, whenever a function is not in a hot loop, we might consider 
preferring readability over performances anyway.


https://reviews.llvm.org/D40537



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D40539: Allow using the object file name for debug symbol resolution

[Davide Italiano via Phabricator via lldb-commits]

davide added subscribers: vsk, aprantl, davide.
davide added a comment.

I thought about this, and it's the only patch from your patchset that I don't 
feel really confident about.
I'm afraid it's a little risky to add more code to the objectfile reader 
without having proper unittesting.
In particular, we might want to either check in an object file (not so ideal)  
or use `llvm-mc` to produce a file with the correct section.
More generally, when we want to test debug info either we want to piggyback on 
mc or yaml2obj.
If none of them suffices, we should think about having a tool that takes a 
textual representation of some sort (YAML seems a good start) and produces a 
core dump to be passed to LLDB.
@sas what do you think?
cc:ing @vsk / @aprantl for thoughts.


https://reviews.llvm.org/D40539



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D40616: ObjectFileELF: Add support for compressed sections

[Davide Italiano via Phabricator via lldb-commits]

davide requested changes to this revision.
davide added a comment.
This revision now requires changes to proceed.

This one is a little weird when written as unittest. Not the worst thing, but I 
agree it should use `llvm-lit`.
Can you give it a shot, Pavel? If that doesn't work, we should at least 
evaluate the amount of work needed to get `llvm-lit` to run with `lldb` before 
dismissing it entirely.
BTW, nice to see lldb getting  more and more tests, regardless :)


https://reviews.llvm.org/D40616



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D40616: ObjectFileELF: Add support for compressed sections

[Davide Italiano via Phabricator via lldb-commits]

davide added a comment.

Another very good reason for writing a `FileCheck` test rather than a unittest 
is that writing unittest is tedious :)
In particular for new contributors, FileCheck tests are much easier to write 
and in this case testing the API surface doesn't seem to add much value.


https://reviews.llvm.org/D40616



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D40475: DWZ 12/12: DWZ test mode

[Davide Italiano via Phabricator via lldb-commits]

davide added a comment.

@jankratochvil I run Fedora for now on my desktop, but I don't have time and 
resources to devote to a buildbot.
IMHO, having a fedora bot would be a great thing for two reasons:

1. I found out Fedora exposes bugs that ubuntu doesn't (e.g. in symbol 
resolution, due to slightly different naming for globals in shared libraries)
2. We can improve coverage for Linux for lldb.

I think the first step to get this to work is to get the build green there, and 
then we can iterate, if you're interested.


https://reviews.llvm.org/D40475



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D40636: Create a trivial lldb-test program

[Davide Italiano via Phabricator via lldb-commits]

davide accepted this revision.
davide added a comment.
This revision is now accepted and ready to land.

This LGTM. I think we can iterate in tree from what we have.


https://reviews.llvm.org/D40636



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits