[Lldb-commits] [PATCH] D49415: Add unit tests for VMRange
labath added a comment. In https://reviews.llvm.org/D49415#1165179, @teemperor wrote: > - Added a PrintTo function as otherwise the gtest comparison macros won't > compile. Sorry, I did not anticipate that. It looks like the `iterator` typedef inside the VMRange is confusing gtest's universal printing logic (normally it does not require a type to be printable and it will just print a hex dump instead). In any case, having the PrintTo function is a good thing, so thanks for adding it. https://reviews.llvm.org/D49415 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D49435: Added unit tests for Flags
labath accepted this revision. labath added a comment. This revision is now accepted and ready to land. lgtm. Thanks. Comment at: unittests/Utility/CMakeLists.txt:10-11 EnvironmentTest.cpp + FlagsTest.cpp FileSpecTest.cpp JSONTest.cpp Please keep this file sorted (`Fl` after `Fi`) https://reviews.llvm.org/D49435 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D49475: Fix variables.test after D49018
aleksandr.urakov created this revision.
aleksandr.urakov added reviewers: stella.stamenova, lldb-commits.
This one fixes variables.test after https://reviews.llvm.org/D49018. The test
was broken because https://reviews.llvm.org/D49018 adds a location information
to variables, but I hadn't noticed that, because I used 32-bit build to run
tests, so the test looked to me already broken before that commit (the test
relies on mangled names, but the mangling schemes are different for 32-bit and
64-bit).
https://reviews.llvm.org/D49475
Files:
lit/SymbolFile/PDB/variables.test
Index: lit/SymbolFile/PDB/variables.test
===
--- lit/SymbolFile/PDB/variables.test
+++ lit/SymbolFile/PDB/variables.test
@@ -7,19 +7,19 @@
CHECK: SymbolVendor ([[MOD]])
CHECK: CompileUnit{{.*}}, language = "c++", file =
'{{.*}}\VariablesTest.cpp'
CHECK-DAG: Variable{{.*}}, name = "g_IntVar"
-CHECK-SAME: scope = global, external
+CHECK-SAME: scope = global, location = {{.*}}, external
CHECK-DAG: Variable{{.*}}, name = "m_StaticClassMember"
-CHECK-SAME: scope = global, external
+CHECK-SAME: scope = global, location = {{.*}}, external
CHECK-DAG: Variable{{.*}}, name = "g_pConst"
-CHECK-SAME: scope = global, external
+CHECK-SAME: scope = global, location = {{.*}}, external
CHECK-DAG: Variable{{.*}}, name = "same_name_var"
-CHECK-SAME: scope = global, external
+CHECK-SAME: scope = global, location = {{.*}}, external
CHECK-DAG: Variable{{.*}}, name = "g_EnumVar"
-CHECK-SAME: scope = global, external
+CHECK-SAME: scope = global, location = {{.*}}, external
CHECK-DAG: Variable{{.*}}, name = "g_tls"
-CHECK-SAME: scope = thread local, external
+CHECK-SAME: scope = thread local, location = {{.*}}, external
CHECK-DAG: Variable{{.*}}, name = "ClassVar"
-CHECK-SAME: scope = global, external
+CHECK-SAME: scope = global, location = {{.*}}, external
CHECK-DAG: Variable{{.*}}, name = "g_Const"
CHECK-SAME: scope = ??? (2)
Index: lit/SymbolFile/PDB/variables.test
===
--- lit/SymbolFile/PDB/variables.test
+++ lit/SymbolFile/PDB/variables.test
@@ -7,19 +7,19 @@
CHECK: SymbolVendor ([[MOD]])
CHECK: CompileUnit{{.*}}, language = "c++", file = '{{.*}}\VariablesTest.cpp'
CHECK-DAG: Variable{{.*}}, name = "g_IntVar"
-CHECK-SAME: scope = global, external
+CHECK-SAME: scope = global, location = {{.*}}, external
CHECK-DAG: Variable{{.*}}, name = "m_StaticClassMember"
-CHECK-SAME: scope = global, external
+CHECK-SAME: scope = global, location = {{.*}}, external
CHECK-DAG: Variable{{.*}}, name = "g_pConst"
-CHECK-SAME: scope = global, external
+CHECK-SAME: scope = global, location = {{.*}}, external
CHECK-DAG: Variable{{.*}}, name = "same_name_var"
-CHECK-SAME: scope = global, external
+CHECK-SAME: scope = global, location = {{.*}}, external
CHECK-DAG: Variable{{.*}}, name = "g_EnumVar"
-CHECK-SAME: scope = global, external
+CHECK-SAME: scope = global, location = {{.*}}, external
CHECK-DAG: Variable{{.*}}, name = "g_tls"
-CHECK-SAME: scope = thread local, external
+CHECK-SAME: scope = thread local, location = {{.*}}, external
CHECK-DAG: Variable{{.*}}, name = "ClassVar"
-CHECK-SAME: scope = global, external
+CHECK-SAME: scope = global, location = {{.*}}, external
CHECK-DAG: Variable{{.*}}, name = "g_Const"
CHECK-SAME: scope = ??? (2)
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D48782: LLDB Test Suite: Provide an Option to run all tests with Dwarf Package Format (DWP).
jankratochvil requested changes to this revision. jankratochvil added inline comments. This revision now requires changes to proceed. Comment at: packages/Python/lldbsuite/test/make/Makefile.rules:238 +ifneq (,$(wildcard $(DWP))) + MAKE_DWP=YES I was thinking DWP should be specifiable by user but `make check-lldb DWP=/usr/bin/dwp` has no effect. And `DWP` is not set by any `build*(` functions in `packages/Python/lldbsuite/test/plugins/builder_base.py`, sorry but I do not find that obvious. Comment at: packages/Python/lldbsuite/test/make/Makefile.rules:528 +ifeq "$(MAKE_DWP)" "YES" + $(DWP) -e "$(EXE)" -o "$(EXE).dwp" + rm -f $(OBJECTS:.o=.dwo) The patch as exported by `Download Raw Diff` says: ```../../../make/Makefile.rules:529: *** missing separator. Stop. ``` (after forcing it by `DWP=/usr/bin/dwp` in that `Makefile.rules` file) https://reviews.llvm.org/D48782 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D49475: Fix variables.test after D49018
aleksandr.urakov added a comment. Can you commit this for me, please? I have no commit access. https://reviews.llvm.org/D49475 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r337393 - [lit, lldbsuite] Remove tests that are duplicated between lit and lldb-suite
Author: stella.stamenova
Date: Wed Jul 18 08:16:54 2018
New Revision: 337393
URL: http://llvm.org/viewvc/llvm-project?rev=337393&view=rev
Log:
[lit, lldbsuite] Remove tests that are duplicated between lit and lldb-suite
Summary: Several tests exist in both lit and lldbsuite. This removes the lit
version of the duplicated tests.
Reviewers: asmith, zturner
Subscribers: llvm-commits
Differential Revision: https://reviews.llvm.org/D49450
Removed:
lldb/trunk/lit/Expr/Inputs/anonymous-struct.cpp
lldb/trunk/lit/Expr/TestCallStdStringFunction.test
lldb/trunk/lit/Expr/TestCallStopAndContinue.test
lldb/trunk/lit/Expr/TestCallUserAnonTypedef.test
lldb/trunk/lit/Expr/TestCallUserDefinedFunction.test
Removed: lldb/trunk/lit/Expr/Inputs/anonymous-struct.cpp
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/Expr/Inputs/anonymous-struct.cpp?rev=337392&view=auto
==
--- lldb/trunk/lit/Expr/Inputs/anonymous-struct.cpp (original)
+++ lldb/trunk/lit/Expr/Inputs/anonymous-struct.cpp (removed)
@@ -1,26 +0,0 @@
-#include
-
-typedef struct {
-float f;
-int i;
-} my_untagged_struct;
-
-double multiply(my_untagged_struct *s)
-{
-return s->f * s->i;
-}
-
-double multiply(my_untagged_struct *s, int x)
-{
-return multiply(s) * x;
-}
-
-int main(int argc, char **argv)
-{
-my_untagged_struct s = {
-.f = (float)argc,
-.i = argc,
-};
-// lldb testsuite break
-return !(multiply(&s, argc) == pow(argc, 3));
-}
Removed: lldb/trunk/lit/Expr/TestCallStdStringFunction.test
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/Expr/TestCallStdStringFunction.test?rev=337392&view=auto
==
--- lldb/trunk/lit/Expr/TestCallStdStringFunction.test (original)
+++ lldb/trunk/lit/Expr/TestCallStdStringFunction.test (removed)
@@ -1,11 +0,0 @@
-# XFAIL: windows
-# -> llvm.org/pr21765
-
-# RUN: %cxx %p/Inputs/call-function.cpp -g -o %t && %lldb -b -s %s -- %t |
FileCheck %s
-
-breakpoint set --file call-function.cpp --line 52
-run
-print str
-# CHECK: Hello world
-print str.c_str()
-# CHECK: Hello world
Removed: lldb/trunk/lit/Expr/TestCallStopAndContinue.test
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/Expr/TestCallStopAndContinue.test?rev=337392&view=auto
==
--- lldb/trunk/lit/Expr/TestCallStopAndContinue.test (original)
+++ lldb/trunk/lit/Expr/TestCallStopAndContinue.test (removed)
@@ -1,12 +0,0 @@
-# XFAIL: windows
-# -> llvm.org/pr24489
-
-# RUN: %cxx %p/Inputs/call-function.cpp -g -o %t && %lldb -b -s %s -o continue
-o "thread list" -- %t 2>&1 | FileCheck %s
-
-breakpoint set --file call-function.cpp --line 52
-run
-breakpoint set --file call-function.cpp --line 14
-expression -i false -- returnsFive()
-# CHECK: Execution was interrupted, reason: breakpoint
-# CHECK: stop reason = User Expression thread plan
-# CHECK: Completed expression: (Five) $0 = (number = 5{{.*}}, name = "five")
Removed: lldb/trunk/lit/Expr/TestCallUserAnonTypedef.test
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/Expr/TestCallUserAnonTypedef.test?rev=337392&view=auto
==
--- lldb/trunk/lit/Expr/TestCallUserAnonTypedef.test (original)
+++ lldb/trunk/lit/Expr/TestCallUserAnonTypedef.test (removed)
@@ -1,11 +0,0 @@
-# XFAIL: windows
-
-# XFAIL: armhf-linux
-# -> llvm.org/pr27868
-
-# RUN: %cxx %p/Inputs/anonymous-struct.cpp -g -o %t && %lldb -b -s %s -- %t |
FileCheck %s
-
-breakpoint set --file anonymous-struct.cpp --line 24
-run
-expression multiply(&s)
-# CHECK: $0 = 1
Removed: lldb/trunk/lit/Expr/TestCallUserDefinedFunction.test
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/Expr/TestCallUserDefinedFunction.test?rev=337392&view=auto
==
--- lldb/trunk/lit/Expr/TestCallUserDefinedFunction.test (original)
+++ lldb/trunk/lit/Expr/TestCallUserDefinedFunction.test (removed)
@@ -1,20 +0,0 @@
-# XFAIL: windows
-# -> llvm.org/pr24489
-
-# RUN: %cxx %p/Inputs/call-function.cpp -g -o %t && %lldb -b -s %s -- %t |
FileCheck %s
-
-breakpoint set --file call-function.cpp --line 52
-run
-expression fib(5)
-# CHECK: $0 = 5
-expression add(4,8)
-# CHECK: $1 = 12
-
-expression add(add(5,2),add(3,4))
-# CHECK: $2 = 14
-expression add(add(5,2),fib(5))
-# CHECK: $3 = 12
-expression stringCompare((const char*) "Hello world")
-# CHECK: $4 = true
-expression stringCompare((const char*) "Hellworld")
-# CHECK: $5 = false
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D49410: [PDB] Parse UDT symbols and pointers to members
aleksandr.urakov added inline comments.
Comment at: source/Plugins/SymbolFile/PDB/PDBASTParser.cpp:291-295
+if (udt->isConstType())
+ clang_type = clang_type.AddConstModifier();
+
+if (udt->isVolatileType())
+ clang_type = clang_type.AddVolatileModifier();
zturner wrote:
> Can you remind me what this means? How would an entire type be marked const
> or volatile? There's no instance variable in play here, these are just types
> as they are declared in the source code, if I'm not mistaken. Something like
> `const class Foo {int x; };` doesn't make any sense. So I'm curious under
> what circumstances these 2 if statements would evaluate to true.
If I understand right, we may find ourselves here during processing of argument
types of some method.
https://reviews.llvm.org/D49410
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r337395 - [windows] Use a well-known path for ComSpec if we fail to retrieve it
Author: stella.stamenova
Date: Wed Jul 18 08:21:54 2018
New Revision: 337395
URL: http://llvm.org/viewvc/llvm-project?rev=337395&view=rev
Log:
[windows] Use a well-known path for ComSpec if we fail to retrieve it
Summary: Right now we always try to retrieve ComSpec and if we fail, we give
up. This rarely fails, but we can update the logic so that we fail even less
frequently. Since there is a well-known path (albeit not always correct), try
the path when we failed to retrieve it. Note that on other platforms, we
generally just return a well-known path without any checking.
Reviewers: asmith, zturner, labath
Reviewed By: zturner, labath
Subscribers: llvm-commits
Differential Revision: https://reviews.llvm.org/D49451
Modified:
lldb/trunk/source/Host/windows/HostInfoWindows.cpp
Modified: lldb/trunk/source/Host/windows/HostInfoWindows.cpp
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Host/windows/HostInfoWindows.cpp?rev=337395&r1=337394&r2=337395&view=diff
==
--- lldb/trunk/source/Host/windows/HostInfoWindows.cpp (original)
+++ lldb/trunk/source/Host/windows/HostInfoWindows.cpp Wed Jul 18 08:21:54 2018
@@ -98,9 +98,14 @@ FileSpec HostInfoWindows::GetProgramFile
}
FileSpec HostInfoWindows::GetDefaultShell() {
+ // Try to retrieve ComSpec from the environment. On the rare occasion
+ // that it fails, try a well-known path for ComSpec instead.
+
std::string shell;
- GetEnvironmentVar("ComSpec", shell);
- return FileSpec(shell, false);
+ if (GetEnvironmentVar("ComSpec", shell))
+return FileSpec(shell, false);
+
+ return FileSpec("C:\\Windows\\system32\\cmd.exe", false);
}
bool HostInfoWindows::GetEnvironmentVar(const std::string &var_name,
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D49410: [PDB] Parse UDT symbols and pointers to members
aleksandr.urakov added a comment. Don't you mind if I try to combine this with https://reviews.llvm.org/D49368? To avoid doing the work twice. https://reviews.llvm.org/D49410 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r337397 - Fix variables.test after D49018
Author: stella.stamenova
Date: Wed Jul 18 08:50:24 2018
New Revision: 337397
URL: http://llvm.org/viewvc/llvm-project?rev=337397&view=rev
Log:
Fix variables.test after D49018
Summary: This one fixes variables.test after D49018. The test was broken
because D49018 adds a location information to variables, but I hadn't noticed
that, because I used 32-bit build to run tests, so the test looked to me
already broken before that commit (the test relies on mangled names, but the
mangling schemes are different for 32-bit and 64-bit).
Reviewers: stella.stamenova, lldb-commits
Reviewed By: stella.stamenova
Patch By: Aleksandr Urakov
Differential Revision: https://reviews.llvm.org/D49475
Modified:
lldb/trunk/lit/SymbolFile/PDB/variables.test
Modified: lldb/trunk/lit/SymbolFile/PDB/variables.test
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/SymbolFile/PDB/variables.test?rev=337397&r1=337396&r2=337397&view=diff
==
--- lldb/trunk/lit/SymbolFile/PDB/variables.test (original)
+++ lldb/trunk/lit/SymbolFile/PDB/variables.test Wed Jul 18 08:50:24 2018
@@ -7,19 +7,19 @@ CHECK: Module [[MOD:.*]]
CHECK: SymbolVendor ([[MOD]])
CHECK: CompileUnit{{.*}}, language = "c++", file =
'{{.*}}\VariablesTest.cpp'
CHECK-DAG: Variable{{.*}}, name = "g_IntVar"
-CHECK-SAME: scope = global, external
+CHECK-SAME: scope = global, location = {{.*}}, external
CHECK-DAG: Variable{{.*}}, name = "m_StaticClassMember"
-CHECK-SAME: scope = global, external
+CHECK-SAME: scope = global, location = {{.*}}, external
CHECK-DAG: Variable{{.*}}, name = "g_pConst"
-CHECK-SAME: scope = global, external
+CHECK-SAME: scope = global, location = {{.*}}, external
CHECK-DAG: Variable{{.*}}, name = "same_name_var"
-CHECK-SAME: scope = global, external
+CHECK-SAME: scope = global, location = {{.*}}, external
CHECK-DAG: Variable{{.*}}, name = "g_EnumVar"
-CHECK-SAME: scope = global, external
+CHECK-SAME: scope = global, location = {{.*}}, external
CHECK-DAG: Variable{{.*}}, name = "g_tls"
-CHECK-SAME: scope = thread local, external
+CHECK-SAME: scope = thread local, location = {{.*}}, external
CHECK-DAG: Variable{{.*}}, name = "ClassVar"
-CHECK-SAME: scope = global, external
+CHECK-SAME: scope = global, location = {{.*}}, external
CHECK-DAG: Variable{{.*}}, name = "g_Const"
CHECK-SAME: scope = ??? (2)
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D49475: Fix variables.test after D49018
This revision was automatically updated to reflect the committed changes.
Closed by commit rL337397: Fix variables.test after D49018 (authored by
stella.stamenova, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D49475?vs=156049&id=156090#toc
Repository:
rL LLVM
https://reviews.llvm.org/D49475
Files:
lldb/trunk/lit/SymbolFile/PDB/variables.test
Index: lldb/trunk/lit/SymbolFile/PDB/variables.test
===
--- lldb/trunk/lit/SymbolFile/PDB/variables.test
+++ lldb/trunk/lit/SymbolFile/PDB/variables.test
@@ -7,19 +7,19 @@
CHECK: SymbolVendor ([[MOD]])
CHECK: CompileUnit{{.*}}, language = "c++", file =
'{{.*}}\VariablesTest.cpp'
CHECK-DAG: Variable{{.*}}, name = "g_IntVar"
-CHECK-SAME: scope = global, external
+CHECK-SAME: scope = global, location = {{.*}}, external
CHECK-DAG: Variable{{.*}}, name = "m_StaticClassMember"
-CHECK-SAME: scope = global, external
+CHECK-SAME: scope = global, location = {{.*}}, external
CHECK-DAG: Variable{{.*}}, name = "g_pConst"
-CHECK-SAME: scope = global, external
+CHECK-SAME: scope = global, location = {{.*}}, external
CHECK-DAG: Variable{{.*}}, name = "same_name_var"
-CHECK-SAME: scope = global, external
+CHECK-SAME: scope = global, location = {{.*}}, external
CHECK-DAG: Variable{{.*}}, name = "g_EnumVar"
-CHECK-SAME: scope = global, external
+CHECK-SAME: scope = global, location = {{.*}}, external
CHECK-DAG: Variable{{.*}}, name = "g_tls"
-CHECK-SAME: scope = thread local, external
+CHECK-SAME: scope = thread local, location = {{.*}}, external
CHECK-DAG: Variable{{.*}}, name = "ClassVar"
-CHECK-SAME: scope = global, external
+CHECK-SAME: scope = global, location = {{.*}}, external
CHECK-DAG: Variable{{.*}}, name = "g_Const"
CHECK-SAME: scope = ??? (2)
Index: lldb/trunk/lit/SymbolFile/PDB/variables.test
===
--- lldb/trunk/lit/SymbolFile/PDB/variables.test
+++ lldb/trunk/lit/SymbolFile/PDB/variables.test
@@ -7,19 +7,19 @@
CHECK: SymbolVendor ([[MOD]])
CHECK: CompileUnit{{.*}}, language = "c++", file = '{{.*}}\VariablesTest.cpp'
CHECK-DAG: Variable{{.*}}, name = "g_IntVar"
-CHECK-SAME: scope = global, external
+CHECK-SAME: scope = global, location = {{.*}}, external
CHECK-DAG: Variable{{.*}}, name = "m_StaticClassMember"
-CHECK-SAME: scope = global, external
+CHECK-SAME: scope = global, location = {{.*}}, external
CHECK-DAG: Variable{{.*}}, name = "g_pConst"
-CHECK-SAME: scope = global, external
+CHECK-SAME: scope = global, location = {{.*}}, external
CHECK-DAG: Variable{{.*}}, name = "same_name_var"
-CHECK-SAME: scope = global, external
+CHECK-SAME: scope = global, location = {{.*}}, external
CHECK-DAG: Variable{{.*}}, name = "g_EnumVar"
-CHECK-SAME: scope = global, external
+CHECK-SAME: scope = global, location = {{.*}}, external
CHECK-DAG: Variable{{.*}}, name = "g_tls"
-CHECK-SAME: scope = thread local, external
+CHECK-SAME: scope = thread local, location = {{.*}}, external
CHECK-DAG: Variable{{.*}}, name = "ClassVar"
-CHECK-SAME: scope = global, external
+CHECK-SAME: scope = global, location = {{.*}}, external
CHECK-DAG: Variable{{.*}}, name = "g_Const"
CHECK-SAME: scope = ??? (2)
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D49410: [PDB] Parse UDT symbols and pointers to members
Not at all. > On Jul 18, 2018, at 8:27 AM, Aleksandr Urakov via Phabricator > wrote: > > aleksandr.urakov added a comment. > > Don't you mind if I try to combine this with https://reviews.llvm.org/D49368? > To avoid doing the work twice. > > > https://reviews.llvm.org/D49410 > > > ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D49475: Fix variables.test after D49018
aleksandr.urakov added a comment. Thanks! Repository: rL LLVM https://reviews.llvm.org/D49475 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D49018: Convert a location information from PDB to a DWARF expression
stella.stamenova added a comment. Can you test this change and send it for review? Thanks! Repository: rL LLVM https://reviews.llvm.org/D49018 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D49410: [PDB] Parse UDT symbols and pointers to members
Hui added inline comments.
Comment at: source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp:587
+// only do this once.
+if (result->GetID() == type_uid) {
+ pdb->CompleteRecordTypeForPDBSymbol(*pdb_type, result);
aleksandr.urakov wrote:
> I don't fully understand, please, explain me, when does this can be `false`?
All types are parsed by symbol->getSymIndexId() except PDBSymbolBaseClass and
PDBSymbolFunc symbols they are parsed by symbol->getTypeId(). After completion,
Clang type of PDBSymbolBaseClass is reset with symbol->getSymIndexId().
https://reviews.llvm.org/D49410
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D49410: [PDB] Parse UDT symbols and pointers to members
Hui added inline comments. Comment at: source/Plugins/SymbolFile/PDB/PDBASTParser.cpp:696 + base_ast_type.GetOpaqueQualType(), access, + pdb_base_class->isVirtualBaseClass(), /*base_of_class*/ true); + base_classes.push_back(base_spec); aleksandr.urakov wrote: > If I understand correctly, `base_of_class` must be `false` for structs. May > be check the kind of `pdb_udt` here? Struct/Union can't have base class. Only class udt is handled. I think it is safe to set it ture. https://reviews.llvm.org/D49410 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D49410: [PDB] Parse UDT symbols and pointers to members
Hui added inline comments.
Comment at: source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp:588
+if (result->GetID() == type_uid) {
+ pdb->CompleteRecordTypeForPDBSymbol(*pdb_type, result);
+}
aleksandr.urakov wrote:
> What are the advantages and disadvantages of immediate (here) and deferred
> (at the `CompleteType` function) completions? I thought that deferred
> completion is a kind of lazy optimization, so we lost its advantages?
I think the initial purpose was that the 'incomplete' can be detected
immediately after 'first' parsing and then solved immediately.
The deferred type completion can encounter a drawback (maybe be worked around
by using lock)
a. the deferred procedure could happen during the 'm_types' (TypeList)" dumping
b. the deferred output could add new types to the 'm_types'.
c. see below. The dump implementation of TypeList.
It will result in undefined behavior if a list is expanded while accessing it
with iterator.
void TypeList::Dump(Stream *s, bool show_context) {
for (iterator pos = m_types.begin(), end = m_types.end(); pos != end; ++pos) {
pos->get()->Dump(s, show_context);
}
}
In this sense, https://reviews.llvm.org/D49368 has a better solution.
https://reviews.llvm.org/D49410
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D49410: [PDB] Parse UDT symbols and pointers to members
Hui added inline comments.
Comment at: source/Plugins/SymbolFile/PDB/PDBASTParser.cpp:246
+return ty ? ty->shared_from_this() : nullptr;
+ } break;
case PDB_SymType::UDT: {
zturner wrote:
> This looks unusual. Did you clang-format it?
Just tried clang-format this file with llvm style. They seemed good in shape.
https://reviews.llvm.org/D49410
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D49410: [PDB] Parse UDT symbols and pointers to members
Hui added inline comments.
Comment at: source/Plugins/SymbolFile/PDB/PDBASTParser.cpp:271-272
+AccessType access = TranslateMemberAccess(udt->getAccess());
+if (access == lldb::eAccessNone && udt->isNested() &&
+udt->getClassParent()) {
+ access = GetDefaultAccessibilityForUdtKind(udt_kind);
zturner wrote:
> So is this `access == None` a thing that can actually happen? AFAICT it's
> impossible for `getAccess()` to return anything other than public, protected,
> or private, in which case this code path will never get executed, so we can
> just delete it.
Chance is that udt->getAccess() could return PDB_MemberAccess() (ctor) if the
DIA call fails. Need to check if CodeView generates default access for a nested
udt or not. I think.
https://reviews.llvm.org/D49410
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D49410: [PDB] Parse UDT symbols and pointers to members
Hui added inline comments. Comment at: source/Plugins/SymbolFile/PDB/PDBASTParser.cpp:647-648 +auto member_ast_type = type_sp->GetLayoutCompilerType(); +if (!member_ast_type.IsCompleteType()) + member_ast_type.GetCompleteType(); +// CXX class type member must be parsed and complete ahead. aleksandr.urakov wrote: > It's not so important, but I think that these lines can be deleted if > arguments of subsequent `if` will be flipped. member udt or base class shall have all been completed before their parent 's completion. The 'if' is a double insurance? seeing a hard assert at line #659. https://reviews.llvm.org/D49410 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
