[Lldb-commits] [PATCH] D135521: [lldb][trace] Add a basic function call dump [1] - Add the command scaffolding

2022-10-14 Thread Sujin Park via Phabricator via lldb-commits
persona0220 accepted this revision.
persona0220 added a comment.
This revision is now accepted and ready to land.

LGTM! I'll also review the algorithm soon :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135521

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


[Lldb-commits] [PATCH] D135827: [lldb] Print newline between found types

2022-10-14 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett added a reviewer: DavidSpickett.
DavidSpickett added a comment.

This LGTM thanks for the fix just one comment on the test.




Comment at: 
lldb/test/API/lang/cpp/type_lookup_duplicate/TestCppTypeLookupDuplicate.py:16
+
+self.expect("image lookup -A -t Foo", DATA_TYPES_DISPLAYED_CORRECTLY, 
substrs=["2 matches found", "\nid =", "\nid ="])

Could you match the whole string here? A bit easier to see at a glance that 
way. Unless the output is not stable?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135827

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


[Lldb-commits] [PATCH] D135826: [lldb] Start from end of previous substr when checking ordered substrs

2022-10-14 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett added a reviewer: DavidSpickett.
DavidSpickett accepted this revision.
DavidSpickett added a comment.
This revision is now accepted and ready to land.

This LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135826

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


[Lldb-commits] [PATCH] D135825: [LLDB] Only run lldb-server Shell tests if it gets built

2022-10-14 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett accepted this revision.
DavidSpickett added a comment.
This revision is now accepted and ready to land.

LGTM, thanks for the fix.


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

https://reviews.llvm.org/D135825

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


[Lldb-commits] [PATCH] D135921: [WIP][lldb][Breakpoint] Fix setting breakpoints on templates by basename

2022-10-14 Thread Michael Buch via Phabricator via lldb-commits
Michael137 added inline comments.



Comment at: 
lldb/test/API/functionalities/breakpoint/cpp/TestCPPBreakpointLocations.py:63
+   'auto 
ns::Foo::func>()']},
+# {'name': 'func>', 'loc_names': ['auto 
ns::Foo::func>()']}, # FIXME
+

Michael137 wrote:
> These didn't work before this patch (or in lldb-14) either. So may xfail them 
> for now
This turns out to be a discrepancy between how we parse basenames for templates 
and the `DW_AT_name` that gets generated for these nested templates. In DWARF, 
the name contains a space between angle brackets (i.e., `func >`). 
So setting a breakpoint without the space fails to find the function name in 
the DWARF index. However, adding the space still doesn't work because it trips 
over something around the parser, haven't checked what exactly yet. Will try 
address this in a separate patch


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135921

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


[Lldb-commits] [PATCH] D135921: [WIP][lldb][Breakpoint] Fix setting breakpoints on templates by basename

2022-10-14 Thread Michael Buch via Phabricator via lldb-commits
Michael137 added inline comments.



Comment at: 
lldb/test/API/functionalities/breakpoint/cpp/TestCPPBreakpointLocations.py:63
+   'auto 
ns::Foo::func>()']},
+# {'name': 'func>', 'loc_names': ['auto 
ns::Foo::func>()']}, # FIXME
+

Michael137 wrote:
> Michael137 wrote:
> > These didn't work before this patch (or in lldb-14) either. So may xfail 
> > them for now
> This turns out to be a discrepancy between how we parse basenames for 
> templates and the `DW_AT_name` that gets generated for these nested 
> templates. In DWARF, the name contains a space between angle brackets (i.e., 
> `func >`). So setting a breakpoint without the space fails to 
> find the function name in the DWARF index. However, adding the space still 
> doesn't work because it trips over something around the parser, haven't 
> checked what exactly yet. Will try address this in a separate patch
Ah, the demangler gives us back a string without the space between angle 
brackets. So we don't match. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135921

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


[Lldb-commits] [PATCH] D135921: [WIP][lldb][Breakpoint] Fix setting breakpoints on templates by basename

2022-10-14 Thread Pavel Labath via Phabricator via lldb-commits
labath added inline comments.



Comment at: lldb/unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp:167
 
+  EXPECT_TRUE(reference_4.ContainsPath("operator"));
   EXPECT_TRUE(reference_4.ContainsPath("operator bool"));

Is this actually expected? Like, I don't think it's completely wrong, but I 
definitely did not expect it to do that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135921

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


[Lldb-commits] [PATCH] D135921: [WIP][lldb][Breakpoint] Fix setting breakpoints on templates by basename

2022-10-14 Thread Pavel Labath via Phabricator via lldb-commits
labath added inline comments.



Comment at: lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp:280
+
+  return {};
+}

I think this should return `basename` here. Otherwise, all non-templated names 
will match the empty string.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135921

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


[Lldb-commits] [PATCH] D135921: [WIP][lldb][Breakpoint] Fix setting breakpoints on templates by basename

2022-10-14 Thread Michael Buch via Phabricator via lldb-commits
Michael137 added inline comments.



Comment at: lldb/unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp:167
 
+  EXPECT_TRUE(reference_4.ContainsPath("operator"));
   EXPECT_TRUE(reference_4.ContainsPath("operator bool"));

labath wrote:
> Is this actually expected? Like, I don't think it's completely wrong, but I 
> definitely did not expect it to do that.
I agree, it's not intuitive. This happens because `operator` is a reserved 
keyword in the eyes of the `CPlusPlusNameParser`. It will just consume the 
entire token and return an empty string.

This won't work for the breakpoint matching logic. I thought we'd just test it 
here in case the someone ever decides to change this behaviour 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135921

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


[Lldb-commits] [PATCH] D135921: [WIP][lldb][Breakpoint] Fix setting breakpoints on templates by basename

2022-10-14 Thread Michael Buch via Phabricator via lldb-commits
Michael137 added inline comments.



Comment at: lldb/unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp:167
 
+  EXPECT_TRUE(reference_4.ContainsPath("operator"));
   EXPECT_TRUE(reference_4.ContainsPath("operator bool"));

Michael137 wrote:
> labath wrote:
> > Is this actually expected? Like, I don't think it's completely wrong, but I 
> > definitely did not expect it to do that.
> I agree, it's not intuitive. This happens because `operator` is a reserved 
> keyword in the eyes of the `CPlusPlusNameParser`. It will just consume the 
> entire token and return an empty string.
> 
> This won't work for the breakpoint matching logic. I thought we'd just test 
> it here in case the someone ever decides to change this behaviour 
> This won't work for the breakpoint matching logic.
I.e., we don't allow matching breakpoints on just `operator`. IIRC something 
else in the breakpoint resolver accounts for this. Seems fragile though


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135921

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


[Lldb-commits] [PATCH] D135921: [WIP][lldb][Breakpoint] Fix setting breakpoints on templates by basename

2022-10-14 Thread Michael Buch via Phabricator via lldb-commits
Michael137 added inline comments.



Comment at: lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp:280
+
+  return {};
+}

labath wrote:
> I think this should return `basename` here. Otherwise, all non-templated 
> names will match the empty string.
Very true


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135921

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


[Lldb-commits] [PATCH] D135648: [lldb] Add matching based on Python callbacks for data formatters.

2022-10-14 Thread Pavel Labath via Phabricator via lldb-commits
labath added inline comments.



Comment at: lldb/source/Commands/CommandObjectType.cpp:2306
   if (type == eRegularSynth) {
-if (category->AnyMatches(type_name, eFormatCategoryItemFilter, false)) {
+// TODO: Get a suitable type object for type_name so we can create a
+// complete FormattersMatchCandidate.

Is that even possible? Like, this command can be run before any binaries/debug 
info is loaded, right? Maybe we should just call that WAI?



Comment at: 
lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp:2159-2167
+  // Default to false in case of error while running the callback.
+  bool result = false;
+  {
+Locker py_lock(this,
+   Locker::AcquireLock | Locker::InitSession | 
Locker::NoSTDIN);
+result = LLDBSwigPythonFormatterCallbackFunction(
+python_function_name, m_dictionary_name.c_str(), type_impl_sp);

I think we already had code like this somewhere, but these two versions are 
completely equivalent.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135648

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


[Lldb-commits] [PATCH] D135921: [WIP][lldb][Breakpoint] Fix setting breakpoints on templates by basename

2022-10-14 Thread Michael Buch via Phabricator via lldb-commits
Michael137 updated this revision to Diff 467741.
Michael137 added a comment.

- Add more test cases
- Remove FIXMEs
- Add docs
- Don't return empty string for non-templates


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135921

Files:
  lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
  lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h
  lldb/test/API/functionalities/breakpoint/cpp/Makefile
  lldb/test/API/functionalities/breakpoint/cpp/TestCPPBreakpointLocations.py
  lldb/test/API/functionalities/breakpoint/cpp/main.cpp
  lldb/unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp

Index: lldb/unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp
===
--- lldb/unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp
+++ lldb/unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp
@@ -143,7 +143,12 @@
   CPlusPlusLanguage::MethodName reference_3(ConstString("int func01()"));
   CPlusPlusLanguage::MethodName 
   reference_4(ConstString("bar::baz::operator bool()"));
-  
+  CPlusPlusLanguage::MethodName reference_5(
+  ConstString("bar::baz::operator bool>()"));
+  CPlusPlusLanguage::MethodName reference_6(ConstString(
+  "bar::baz::operator<<, Type>>()"));
+
+  EXPECT_TRUE(reference_1.ContainsPath(""));
   EXPECT_TRUE(reference_1.ContainsPath("func01"));
   EXPECT_TRUE(reference_1.ContainsPath("bar::func01"));
   EXPECT_TRUE(reference_1.ContainsPath("foo::bar::func01"));
@@ -153,17 +158,35 @@
   EXPECT_FALSE(reference_1.ContainsPath("::foo::baz::func01"));
   EXPECT_FALSE(reference_1.ContainsPath("foo::bar::baz::func01"));
   
+  EXPECT_TRUE(reference_2.ContainsPath(""));
   EXPECT_TRUE(reference_2.ContainsPath("foofoo::bar::func01"));
   EXPECT_FALSE(reference_2.ContainsPath("foo::bar::func01"));
   
+  EXPECT_TRUE(reference_3.ContainsPath(""));
   EXPECT_TRUE(reference_3.ContainsPath("func01"));
   EXPECT_FALSE(reference_3.ContainsPath("func"));
   EXPECT_FALSE(reference_3.ContainsPath("bar::func01"));
 
+  EXPECT_TRUE(reference_4.ContainsPath(""));
+  EXPECT_TRUE(reference_4.ContainsPath("operator"));
   EXPECT_TRUE(reference_4.ContainsPath("operator bool"));
   EXPECT_TRUE(reference_4.ContainsPath("baz::operator bool"));
   EXPECT_TRUE(reference_4.ContainsPath("bar::baz::operator bool"));
   EXPECT_FALSE(reference_4.ContainsPath("az::operator bool"));
+
+  EXPECT_TRUE(reference_5.ContainsPath(""));
+  EXPECT_TRUE(reference_5.ContainsPath("operator"));
+  EXPECT_TRUE(reference_5.ContainsPath("operator bool"));
+  EXPECT_TRUE(reference_5.ContainsPath("operator bool>"));
+  EXPECT_FALSE(reference_5.ContainsPath("operator bool"));
+  EXPECT_FALSE(reference_5.ContainsPath("operator bool>"));
+
+  EXPECT_TRUE(reference_6.ContainsPath(""));
+  EXPECT_TRUE(reference_6.ContainsPath("operator"));
+  EXPECT_TRUE(reference_6.ContainsPath("operator<<"));
+  EXPECT_TRUE(reference_6.ContainsPath(
+  "bar::baz::operator<<, Type>>()"));
+  EXPECT_FALSE(reference_6.ContainsPath("operator<<>"));
 }
 
 TEST(CPlusPlusLanguage, ExtractContextAndIdentifier) {
Index: lldb/test/API/functionalities/breakpoint/cpp/main.cpp
===
--- lldb/test/API/functionalities/breakpoint/cpp/main.cpp
+++ lldb/test/API/functionalities/breakpoint/cpp/main.cpp
@@ -82,6 +82,20 @@
 };
 }
 
+namespace ns {
+template  struct Foo {
+  template  void import() {}
+
+  template  auto func() {}
+
+  operator bool() { return true; }
+
+  template  operator T() { return {}; }
+
+  template  void operator<<(T t) {}
+};
+} // namespace ns
+
 int main (int argc, char const *argv[])
 {
 a::c ac;
@@ -98,5 +112,16 @@
 bc.func3();
 cd.func2();
 cd.func3();
+
+ns::Foo f;
+f.import ();
+f.func();
+f.func>();
+f.operator bool();
+f.operator a::c();
+f.operator ns::Foo();
+f.operator<<(5);
+f.operator<< >({});
+
 return 0;
 }
Index: lldb/test/API/functionalities/breakpoint/cpp/TestCPPBreakpointLocations.py
===
--- lldb/test/API/functionalities/breakpoint/cpp/TestCPPBreakpointLocations.py
+++ lldb/test/API/functionalities/breakpoint/cpp/TestCPPBreakpointLocations.py
@@ -54,6 +54,23 @@
 {'name': 'a::c::func1()', 'loc_names': ['a::c::func1()']},
 {'name': 'b::c::func1()', 'loc_names': ['b::c::func1()']},
 {'name': 'c::d::func2()', 'loc_names': ['c::d::func2()']},
+
+# Template cases
+{'name': 'func', 'loc_names': []},
+{'name': 'func', 'loc_names': ['auto ns::Foo::func()']},
+{'name': 'func', 'loc_names': ['auto ns::Foo::func()',
+   'auto ns::Foo::func>()']},
+
+{'name': 'operator', 'loc_names': []},
+{'name': 'ns::Foo::operator bool', 'loc_names': ['ns::Foo::op

[Lldb-commits] [lldb] 7349bc3 - Speculatively fix the lldb test bots

2022-10-14 Thread Aaron Ballman via lldb-commits

Author: Aaron Ballman
Date: 2022-10-14T08:37:08-04:00
New Revision: 7349bc34836b73518ca3e9366019ef20094f2525

URL: 
https://github.com/llvm/llvm-project/commit/7349bc34836b73518ca3e9366019ef20094f2525
DIFF: 
https://github.com/llvm/llvm-project/commit/7349bc34836b73518ca3e9366019ef20094f2525.diff

LOG: Speculatively fix the lldb test bots

This should fix the issue found by:
https://lab.llvm.org/buildbot/#/builders/68/builds/41100

Added: 


Modified: 

lldb/test/API/commands/expression/expr_inside_lambda/TestExprInsideLambdas.py

Removed: 




diff  --git 
a/lldb/test/API/commands/expression/expr_inside_lambda/TestExprInsideLambdas.py 
b/lldb/test/API/commands/expression/expr_inside_lambda/TestExprInsideLambdas.py
index 9b8937aaa0c23..aa7c01c055fc2 100644
--- 
a/lldb/test/API/commands/expression/expr_inside_lambda/TestExprInsideLambdas.py
+++ 
b/lldb/test/API/commands/expression/expr_inside_lambda/TestExprInsideLambdas.py
@@ -112,7 +112,7 @@ def test_expr_inside_lambda(self):
" 'base_var'"))
 
 self.expectExprError("local_var", ("use of non-static data member 
'local_var'"
-   " of '' from nested type 
'LocalLambdaClass'"))
+   " of '(unnamed class)' from nested 
type 'LocalLambdaClass'"))
 
 # Inside non_capturing_method
 lldbutil.continue_to_breakpoint(process, bkpt)



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


[Lldb-commits] [PATCH] D135917: [lldb][trace] Add a basic function call dump [2] - Implement the reconstruction algorithm

2022-10-14 Thread Jakob Johnson via Phabricator via lldb-commits
jj10306 requested changes to this revision.
jj10306 added a comment.
This revision now requires changes to proceed.

this is awesome - the tests do a great job documenting the code and weird edge 
cases so thanks for that.
just some minor nits and questions, looks good overall




Comment at: lldb/include/lldb/Target/TraceDumper.h:86
+  ///   A traced segment is a maximal list of consecutive traced instructions
+  ///   that belong to the same call. A traced segment will end in three
+  ///   possible ways:

nit



Comment at: lldb/include/lldb/Target/TraceDumper.h:213-214
+  /// The symbol information of the delimiting instructions.
+  SymbolInfo first_symbol_info;
+  SymbolInfo last_symbol_info;
+  /// A nested call starting at the end of this segment.

how would you have two different symbol info for the first and last? is last 
referring to the symbol of the next function?



Comment at: lldb/include/lldb/Target/TraceDumper.h:216
+  /// A nested call starting at the end of this segment.
+  llvm::Optional nested_call;
+  /// Owning call

nit:is the Optional redundant? thoughts on just using the UP and mentioning 
nullptr indicates there is no nested call



Comment at: lldb/source/Target/TraceDumper.cpp:92-94
+  Block *inline_block_a =
+  insn.sc.block ? insn.sc.block->GetContainingInlinedBlock() : nullptr;
+  Block *inline_block_b = prev_insn.sc.block

are these functions transferring ownership of these block pointers (ie are you 
responsible for freeing this mem)?



Comment at: lldb/source/Target/TraceDumper.cpp:584
+
+/// Given an instruction that happens after a return, find the ancestor 
function
+/// call that owns it. If this ancestor doesn't exist, create a new ancestor 
and

"that happens after a return"
from looking at how this function's called from 
`AppendInstructionToFunctionCallForest` this appears to be the return 
instruction, not the instruction after a return. is this correct?



Comment at: lldb/source/Target/TraceDumper.cpp:617-619
+  // Note: If this is not robust enough, we should actually check if we
+  // returning to the instruction that follows the last instruction from
+  // that call, as that's the behavior of CALL instructions.

this would only be the case for well behaved CALL/RET, right? in the case of 
modified stack return or JMP, this wouldn't necessarily be true?



Comment at: lldb/source/Target/TraceDumper.cpp:783-785
+  // TODO: In case of a CPU change, we create a new root because we haven't
+  // investigated yet if a call tree can safely continue or if interrupts
+  // could have polluted the original call tree.

wouldn't this be a very common case where a thread is in the middle of 
executing a function, gets context switched out of CPU X, then after some time 
gets context switched into CPU Y and continues executing the function?



Comment at: lldb/test/API/commands/trace/TestTraceDumpFunctionCalls.py:31-32
+
+[call tree #0]
+m.out`foo() + 65 at multi_thread.cpp:12:21 to 12:21  [4, 19524]
+

this is saying that events 4 - 19524 were all executed by foo() and there 
weren't any other function calls?  am I understanding this output correctly?




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135917

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


[Lldb-commits] [PATCH] D135516: [lldb] [MainLoopPosix] Fix crash upon adding lots of pending callbacks

2022-10-14 Thread Michał Górny via Phabricator via lldb-commits
mgorny added inline comments.



Comment at: lldb/source/Host/posix/MainLoopPosix.cpp:408
   assert(bytes_written == 1);
+  m_trigger_done = true;
 }

labath wrote:
> I /think/ this is not right. The other thread can wake up as soon as the 
> Write call is done, and can proceed to clear the `done` flag before we are 
> able to set it. If we set it afterwards, then we we suppress all subsequent 
> writes, and the callbacks would never run. If we set the flag before we make 
> the Write call, then it should be ok -- though the flag should probably have 
> a different name then.
I think you are correct. Will update that soonish.


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

https://reviews.llvm.org/D135516

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


[Lldb-commits] [PATCH] D135516: [lldb] [MainLoopPosix] Fix crash upon adding lots of pending callbacks

2022-10-14 Thread Michał Górny via Phabricator via lldb-commits
mgorny updated this revision to Diff 467834.
mgorny added a comment.

Move and rename the `bool`.


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

https://reviews.llvm.org/D135516

Files:
  lldb/include/lldb/Host/posix/MainLoopPosix.h
  lldb/source/Host/posix/MainLoopPosix.cpp
  lldb/unittests/Host/MainLoopTest.cpp


Index: lldb/unittests/Host/MainLoopTest.cpp
===
--- lldb/unittests/Host/MainLoopTest.cpp
+++ lldb/unittests/Host/MainLoopTest.cpp
@@ -171,6 +171,17 @@
   ASSERT_TRUE(callback2_called);
 }
 
+// Regression test for assertion failure if a lot of callbacks end up
+// being queued after loop exits.
+TEST_F(MainLoopTest, PendingCallbackAfterLoopExited) {
+  MainLoop loop;
+  Status error;
+  ASSERT_TRUE(loop.Run().Success());
+  // Try to fill the pipe buffer in.
+  for (int i = 0; i < 65536; ++i)
+loop.AddPendingCallback([&](MainLoopBase &loop) {});
+}
+
 #ifdef LLVM_ON_UNIX
 TEST_F(MainLoopTest, DetectsEOF) {
 
Index: lldb/source/Host/posix/MainLoopPosix.cpp
===
--- lldb/source/Host/posix/MainLoopPosix.cpp
+++ lldb/source/Host/posix/MainLoopPosix.cpp
@@ -222,7 +222,7 @@
 }
 #endif
 
-MainLoopPosix::MainLoopPosix() {
+MainLoopPosix::MainLoopPosix() : m_triggering(false) {
   Status error = m_trigger_pipe.CreateNew(/*child_process_inherit=*/false);
   assert(error.Success());
   const int trigger_pipe_fd = m_trigger_pipe.GetReadFileDescriptor();
@@ -371,6 +371,7 @@
 
 impl.ProcessEvents();
 
+m_triggering = false;
 ProcessPendingCallbacks();
   }
   return Status();
@@ -395,6 +396,10 @@
 }
 
 void MainLoopPosix::TriggerPendingCallbacks() {
+  if (m_triggering)
+return;
+  m_triggering = true;
+
   char c = '.';
   size_t bytes_written;
   Status error = m_trigger_pipe.Write(&c, 1, bytes_written);
Index: lldb/include/lldb/Host/posix/MainLoopPosix.h
===
--- lldb/include/lldb/Host/posix/MainLoopPosix.h
+++ lldb/include/lldb/Host/posix/MainLoopPosix.h
@@ -13,6 +13,7 @@
 #include "lldb/Host/MainLoopBase.h"
 #include "lldb/Host/Pipe.h"
 #include "llvm/ADT/DenseMap.h"
+#include 
 #include 
 #include 
 #include 
@@ -87,6 +88,7 @@
   llvm::DenseMap m_read_fds;
   llvm::DenseMap m_signals;
   Pipe m_trigger_pipe;
+  std::atomic m_triggering;
 #if HAVE_SYS_EVENT_H
   int m_kqueue;
 #endif


Index: lldb/unittests/Host/MainLoopTest.cpp
===
--- lldb/unittests/Host/MainLoopTest.cpp
+++ lldb/unittests/Host/MainLoopTest.cpp
@@ -171,6 +171,17 @@
   ASSERT_TRUE(callback2_called);
 }
 
+// Regression test for assertion failure if a lot of callbacks end up
+// being queued after loop exits.
+TEST_F(MainLoopTest, PendingCallbackAfterLoopExited) {
+  MainLoop loop;
+  Status error;
+  ASSERT_TRUE(loop.Run().Success());
+  // Try to fill the pipe buffer in.
+  for (int i = 0; i < 65536; ++i)
+loop.AddPendingCallback([&](MainLoopBase &loop) {});
+}
+
 #ifdef LLVM_ON_UNIX
 TEST_F(MainLoopTest, DetectsEOF) {
 
Index: lldb/source/Host/posix/MainLoopPosix.cpp
===
--- lldb/source/Host/posix/MainLoopPosix.cpp
+++ lldb/source/Host/posix/MainLoopPosix.cpp
@@ -222,7 +222,7 @@
 }
 #endif
 
-MainLoopPosix::MainLoopPosix() {
+MainLoopPosix::MainLoopPosix() : m_triggering(false) {
   Status error = m_trigger_pipe.CreateNew(/*child_process_inherit=*/false);
   assert(error.Success());
   const int trigger_pipe_fd = m_trigger_pipe.GetReadFileDescriptor();
@@ -371,6 +371,7 @@
 
 impl.ProcessEvents();
 
+m_triggering = false;
 ProcessPendingCallbacks();
   }
   return Status();
@@ -395,6 +396,10 @@
 }
 
 void MainLoopPosix::TriggerPendingCallbacks() {
+  if (m_triggering)
+return;
+  m_triggering = true;
+
   char c = '.';
   size_t bytes_written;
   Status error = m_trigger_pipe.Write(&c, 1, bytes_written);
Index: lldb/include/lldb/Host/posix/MainLoopPosix.h
===
--- lldb/include/lldb/Host/posix/MainLoopPosix.h
+++ lldb/include/lldb/Host/posix/MainLoopPosix.h
@@ -13,6 +13,7 @@
 #include "lldb/Host/MainLoopBase.h"
 #include "lldb/Host/Pipe.h"
 #include "llvm/ADT/DenseMap.h"
+#include 
 #include 
 #include 
 #include 
@@ -87,6 +88,7 @@
   llvm::DenseMap m_read_fds;
   llvm::DenseMap m_signals;
   Pipe m_trigger_pipe;
+  std::atomic m_triggering;
 #if HAVE_SYS_EVENT_H
   int m_kqueue;
 #endif
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] 021a3d5 - [lldb] Start from end of previous substr when checking ordered substrs

2022-10-14 Thread Arthur Eubanks via lldb-commits

Author: Arthur Eubanks
Date: 2022-10-14T11:16:51-07:00
New Revision: 021a3d5a3f73c59d7e43857c0230e07fa602d948

URL: 
https://github.com/llvm/llvm-project/commit/021a3d5a3f73c59d7e43857c0230e07fa602d948
DIFF: 
https://github.com/llvm/llvm-project/commit/021a3d5a3f73c59d7e43857c0230e07fa602d948.diff

LOG: [lldb] Start from end of previous substr when checking ordered substrs

I'm trying to add a test which tests that the same substr occurs twice in a 
row, but it matches even if only one of the substr occurs.

This found a bug in concurrent_base.py.

Reviewed By: DavidSpickett

Differential Revision: https://reviews.llvm.org/D135826

Added: 


Modified: 
lldb/packages/Python/lldbsuite/test/concurrent_base.py
lldb/packages/Python/lldbsuite/test/lldbtest.py

lldb/test/API/functionalities/data-formatter/data-formatter-cpp/TestDataFormatterCpp.py

Removed: 




diff  --git a/lldb/packages/Python/lldbsuite/test/concurrent_base.py 
b/lldb/packages/Python/lldbsuite/test/concurrent_base.py
index 6acd71ce9e46d..a235ceb76413f 100644
--- a/lldb/packages/Python/lldbsuite/test/concurrent_base.py
+++ b/lldb/packages/Python/lldbsuite/test/concurrent_base.py
@@ -74,9 +74,7 @@ def add_breakpoint(self, line, descriptions):
 bpno = lldbutil.run_break_set_by_file_and_line(
 self, self.filename, line, num_expected_locations=-1)
 bp = self.inferior_target.FindBreakpointByID(bpno)
-descriptions.append(
-": file = 'main.cpp', line = %d" %
-self.finish_breakpoint_line)
+descriptions.append(": file = 'main.cpp', line = %d" % line)
 return bp
 
 def inferior_done(self):

diff  --git a/lldb/packages/Python/lldbsuite/test/lldbtest.py 
b/lldb/packages/Python/lldbsuite/test/lldbtest.py
index 1ea29b3f5a39d..2d054f971cd02 100644
--- a/lldb/packages/Python/lldbsuite/test/lldbtest.py
+++ b/lldb/packages/Python/lldbsuite/test/lldbtest.py
@@ -2358,7 +2358,7 @@ def found_str(matched):
 start = 0
 for substr in substrs:
 index = output[start:].find(substr)
-start = start + index if ordered and matching else 0
+start = start + index + len(substr) if ordered and matching 
else 0
 matched = index != -1
 log_lines.append("{} sub string: \"{}\" ({})".format(
 expecting_str, substr, found_str(matched)))

diff  --git 
a/lldb/test/API/functionalities/data-formatter/data-formatter-cpp/TestDataFormatterCpp.py
 
b/lldb/test/API/functionalities/data-formatter/data-formatter-cpp/TestDataFormatterCpp.py
index d4d6d249ebce3..c4d4c465fedb6 100644
--- 
a/lldb/test/API/functionalities/data-formatter/data-formatter-cpp/TestDataFormatterCpp.py
+++ 
b/lldb/test/API/functionalities/data-formatter/data-formatter-cpp/TestDataFormatterCpp.py
@@ -116,7 +116,7 @@ def cleanup():
 substrs=[
 '(char *) $',
 ' = ptr = ',
-' 
"1234567890123456789012345678901234567890123456789012345678901234ABC"'])
+
'"1234567890123456789012345678901234567890123456789012345678901234ABC"'])
 
 self.runCmd("type summary add -c TestPoint")
 



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


[Lldb-commits] [PATCH] D135826: [lldb] Start from end of previous substr when checking ordered substrs

2022-10-14 Thread Arthur Eubanks via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG021a3d5a3f73: [lldb] Start from end of previous substr when 
checking ordered substrs (authored by aeubanks).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135826

Files:
  lldb/packages/Python/lldbsuite/test/concurrent_base.py
  lldb/packages/Python/lldbsuite/test/lldbtest.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-cpp/TestDataFormatterCpp.py


Index: 
lldb/test/API/functionalities/data-formatter/data-formatter-cpp/TestDataFormatterCpp.py
===
--- 
lldb/test/API/functionalities/data-formatter/data-formatter-cpp/TestDataFormatterCpp.py
+++ 
lldb/test/API/functionalities/data-formatter/data-formatter-cpp/TestDataFormatterCpp.py
@@ -116,7 +116,7 @@
 substrs=[
 '(char *) $',
 ' = ptr = ',
-' 
"1234567890123456789012345678901234567890123456789012345678901234ABC"'])
+
'"1234567890123456789012345678901234567890123456789012345678901234ABC"'])
 
 self.runCmd("type summary add -c TestPoint")
 
Index: lldb/packages/Python/lldbsuite/test/lldbtest.py
===
--- lldb/packages/Python/lldbsuite/test/lldbtest.py
+++ lldb/packages/Python/lldbsuite/test/lldbtest.py
@@ -2358,7 +2358,7 @@
 start = 0
 for substr in substrs:
 index = output[start:].find(substr)
-start = start + index if ordered and matching else 0
+start = start + index + len(substr) if ordered and matching 
else 0
 matched = index != -1
 log_lines.append("{} sub string: \"{}\" ({})".format(
 expecting_str, substr, found_str(matched)))
Index: lldb/packages/Python/lldbsuite/test/concurrent_base.py
===
--- lldb/packages/Python/lldbsuite/test/concurrent_base.py
+++ lldb/packages/Python/lldbsuite/test/concurrent_base.py
@@ -74,9 +74,7 @@
 bpno = lldbutil.run_break_set_by_file_and_line(
 self, self.filename, line, num_expected_locations=-1)
 bp = self.inferior_target.FindBreakpointByID(bpno)
-descriptions.append(
-": file = 'main.cpp', line = %d" %
-self.finish_breakpoint_line)
+descriptions.append(": file = 'main.cpp', line = %d" % line)
 return bp
 
 def inferior_done(self):


Index: lldb/test/API/functionalities/data-formatter/data-formatter-cpp/TestDataFormatterCpp.py
===
--- lldb/test/API/functionalities/data-formatter/data-formatter-cpp/TestDataFormatterCpp.py
+++ lldb/test/API/functionalities/data-formatter/data-formatter-cpp/TestDataFormatterCpp.py
@@ -116,7 +116,7 @@
 substrs=[
 '(char *) $',
 ' = ptr = ',
-' "1234567890123456789012345678901234567890123456789012345678901234ABC"'])
+'"1234567890123456789012345678901234567890123456789012345678901234ABC"'])
 
 self.runCmd("type summary add -c TestPoint")
 
Index: lldb/packages/Python/lldbsuite/test/lldbtest.py
===
--- lldb/packages/Python/lldbsuite/test/lldbtest.py
+++ lldb/packages/Python/lldbsuite/test/lldbtest.py
@@ -2358,7 +2358,7 @@
 start = 0
 for substr in substrs:
 index = output[start:].find(substr)
-start = start + index if ordered and matching else 0
+start = start + index + len(substr) if ordered and matching else 0
 matched = index != -1
 log_lines.append("{} sub string: \"{}\" ({})".format(
 expecting_str, substr, found_str(matched)))
Index: lldb/packages/Python/lldbsuite/test/concurrent_base.py
===
--- lldb/packages/Python/lldbsuite/test/concurrent_base.py
+++ lldb/packages/Python/lldbsuite/test/concurrent_base.py
@@ -74,9 +74,7 @@
 bpno = lldbutil.run_break_set_by_file_and_line(
 self, self.filename, line, num_expected_locations=-1)
 bp = self.inferior_target.FindBreakpointByID(bpno)
-descriptions.append(
-": file = 'main.cpp', line = %d" %
-self.finish_breakpoint_line)
+descriptions.append(": file = 'main.cpp', line = %d" % line)
 return bp
 
 def inferior_done(self):
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D135827: [lldb] Print newline between found types

2022-10-14 Thread Arthur Eubanks via Phabricator via lldb-commits
aeubanks added inline comments.



Comment at: 
lldb/test/API/lang/cpp/type_lookup_duplicate/TestCppTypeLookupDuplicate.py:16
+
+self.expect("image lookup -A -t Foo", DATA_TYPES_DISPLAYED_CORRECTLY, 
substrs=["2 matches found", "\nid =", "\nid ="])

DavidSpickett wrote:
> Could you match the whole string here? A bit easier to see at a glance that 
> way. Unless the output is not stable?
it's
```
(lldb) im loo -t Foo
2 matches found in 
/home/aeubanks/repos/llvm-project/build/cmake/lldb-test-build.noindex/lang/cpp/type_lookup_duplicate/TestCppTypeLookupDuplicate.test_namespace_only_dwarf/a.out:
id = {0x004f}, name = "Foo", qualified = "a::Foo", byte-size = 1, decl = 
main.cpp:2, compiler_type = "struct Foo {
}"
id = {0x0058}, name = "Foo", qualified = "b::Foo", byte-size = 1, decl = 
main.cpp:6, compiler_type = "struct Foo {
}"
```

I'd rather not depend so much on the exact output since that doesn't 
necessarily look stable


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135827

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


[Lldb-commits] [PATCH] D135979: [lldb][NFCish] Move DWARFDebugInfoEntry::GetQualifiedName() into DWARFASTParserClang

2022-10-14 Thread Arthur Eubanks via Phabricator via lldb-commits
aeubanks created this revision.
Herald added a reviewer: shafik.
Herald added a project: All.
aeubanks requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

In D134378 , we'll need the clang AST to be 
able to construct the qualified in some cases.

This makes logging in one place slightly less informative.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D135979

Files:
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.h
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp

Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -2960,8 +2960,6 @@
 
 if (!try_resolving_type) {
   if (log) {
-std::string qualified_name;
-type_die.GetQualifiedName(qualified_name);
 GetObjectFile()->GetModule()->LogMessage(
 log,
 "SymbolFileDWARF::"
@@ -2969,7 +2967,7 @@
 "qualified-name='%s') ignoring die=0x%8.8x (%s)",
 DW_TAG_value_to_name(dwarf_decl_ctx[0].tag),
 dwarf_decl_ctx.GetQualifiedName(), type_die.GetOffset(),
-qualified_name.c_str());
+type_die.GetName());
   }
   return true;
 }
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
@@ -99,11 +99,6 @@
 
   const char *GetPubname(const DWARFUnit *cu) const;
 
-  const char *GetQualifiedName(DWARFUnit *cu, std::string &storage) const;
-
-  const char *GetQualifiedName(DWARFUnit *cu, const DWARFAttributes &attributes,
-   std::string &storage) const;
-
   bool GetDIENamesAndRanges(
   DWARFUnit *cu, const char *&name, const char *&mangled,
   DWARFRangeList &rangeList, int &decl_file, int &decl_line,
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
@@ -798,66 +798,6 @@
   return DWARFDIE();
 }
 
-const char *DWARFDebugInfoEntry::GetQualifiedName(DWARFUnit *cu,
-  std::string &storage) const {
-  DWARFAttributes attributes;
-  GetAttributes(cu, attributes, Recurse::yes);
-  return GetQualifiedName(cu, attributes, storage);
-}
-
-const char *
-DWARFDebugInfoEntry::GetQualifiedName(DWARFUnit *cu,
-  const DWARFAttributes &attributes,
-  std::string &storage) const {
-
-  const char *name = GetName(cu);
-
-  if (name) {
-DWARFDIE parent_decl_ctx_die = GetParentDeclContextDIE(cu);
-storage.clear();
-// TODO: change this to get the correct decl context parent
-while (parent_decl_ctx_die) {
-  const dw_tag_t parent_tag = parent_decl_ctx_die.Tag();
-  switch (parent_tag) {
-  case DW_TAG_namespace: {
-const char *namespace_name = parent_decl_ctx_die.GetName();
-if (namespace_name) {
-  storage.insert(0, "::");
-  storage.insert(0, namespace_name);
-} else {
-  storage.insert(0, "(anonymous namespace)::");
-}
-parent_decl_ctx_die = parent_decl_ctx_die.GetParentDeclContextDIE();
-  } break;
-
-  case DW_TAG_class_type:
-  case DW_TAG_structure_type:
-  case DW_TAG_union_type: {
-const char *class_union_struct_name = parent_decl_ctx_die.GetName();
-
-if (class_union_struct_name) {
-  storage.insert(0, "::");
-  storage.insert(0, class_union_struct_name);
-}
-parent_decl_ctx_die = parent_decl_ctx_die.GetParentDeclContextDIE();
-  } break;
-
-  default:
-parent_decl_ctx_die.Clear();
-break;
-  }
-}
-
-if (storage.empty())
-  storage.append("::");
-
-storage.append(name);
-  }
-  if (storage.empty())
-return nullptr;
-  return storage.c_str();
-}
-
 lldb::offset_t DWARFDebugInfoEntry::GetFirstAttributeOffset() const {
   return GetOffset() + llvm::getULEB128Size(m_abbr_idx);
 }
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.h
===
--- lldb/source/Plugins/S

[Lldb-commits] [PATCH] D135979: [lldb][NFCish] Move DWARFDebugInfoEntry::GetQualifiedName() into DWARFASTParserClang

2022-10-14 Thread Arthur Eubanks via Phabricator via lldb-commits
aeubanks updated this revision to Diff 467868.
aeubanks added a comment.
Herald added a subscriber: JDevlieghere.

update


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135979

Files:
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.h
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp

Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -2960,8 +2960,6 @@
 
 if (!try_resolving_type) {
   if (log) {
-std::string qualified_name;
-type_die.GetQualifiedName(qualified_name);
 GetObjectFile()->GetModule()->LogMessage(
 log,
 "SymbolFileDWARF::"
@@ -2969,7 +2967,7 @@
 "qualified-name='%s') ignoring die=0x%8.8x (%s)",
 DW_TAG_value_to_name(dwarf_decl_ctx[0].tag),
 dwarf_decl_ctx.GetQualifiedName(), type_die.GetOffset(),
-qualified_name.c_str());
+type_die.GetName());
   }
   return true;
 }
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
@@ -99,11 +99,6 @@
 
   const char *GetPubname(const DWARFUnit *cu) const;
 
-  const char *GetQualifiedName(DWARFUnit *cu, std::string &storage) const;
-
-  const char *GetQualifiedName(DWARFUnit *cu, const DWARFAttributes &attributes,
-   std::string &storage) const;
-
   bool GetDIENamesAndRanges(
   DWARFUnit *cu, const char *&name, const char *&mangled,
   DWARFRangeList &rangeList, int &decl_file, int &decl_line,
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
@@ -798,66 +798,6 @@
   return DWARFDIE();
 }
 
-const char *DWARFDebugInfoEntry::GetQualifiedName(DWARFUnit *cu,
-  std::string &storage) const {
-  DWARFAttributes attributes;
-  GetAttributes(cu, attributes, Recurse::yes);
-  return GetQualifiedName(cu, attributes, storage);
-}
-
-const char *
-DWARFDebugInfoEntry::GetQualifiedName(DWARFUnit *cu,
-  const DWARFAttributes &attributes,
-  std::string &storage) const {
-
-  const char *name = GetName(cu);
-
-  if (name) {
-DWARFDIE parent_decl_ctx_die = GetParentDeclContextDIE(cu);
-storage.clear();
-// TODO: change this to get the correct decl context parent
-while (parent_decl_ctx_die) {
-  const dw_tag_t parent_tag = parent_decl_ctx_die.Tag();
-  switch (parent_tag) {
-  case DW_TAG_namespace: {
-const char *namespace_name = parent_decl_ctx_die.GetName();
-if (namespace_name) {
-  storage.insert(0, "::");
-  storage.insert(0, namespace_name);
-} else {
-  storage.insert(0, "(anonymous namespace)::");
-}
-parent_decl_ctx_die = parent_decl_ctx_die.GetParentDeclContextDIE();
-  } break;
-
-  case DW_TAG_class_type:
-  case DW_TAG_structure_type:
-  case DW_TAG_union_type: {
-const char *class_union_struct_name = parent_decl_ctx_die.GetName();
-
-if (class_union_struct_name) {
-  storage.insert(0, "::");
-  storage.insert(0, class_union_struct_name);
-}
-parent_decl_ctx_die = parent_decl_ctx_die.GetParentDeclContextDIE();
-  } break;
-
-  default:
-parent_decl_ctx_die.Clear();
-break;
-  }
-}
-
-if (storage.empty())
-  storage.append("::");
-
-storage.append(name);
-  }
-  if (storage.empty())
-return nullptr;
-  return storage.c_str();
-}
-
 lldb::offset_t DWARFDebugInfoEntry::GetFirstAttributeOffset() const {
   return GetOffset() + llvm::getULEB128Size(m_abbr_idx);
 }
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.h
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.h
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.h
@@ -30,8 +30,6 @@
 
   const char *GetPubname() const;
 
-  const char *GetQualifiedName(std::string &storage) const;
-
   using

[Lldb-commits] [PATCH] D134378: [lldb] Support simplified template names

2022-10-14 Thread Arthur Eubanks via Phabricator via lldb-commits
aeubanks added inline comments.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:1561
+std::string
+DWARFASTParserClang::GetCPlusPlusQualifiedName(const DWARFDIE &die) {
+  if (!die.IsValid())

dblaikie wrote:
> Sorry, when I gave feedback on some pieces of this that were just moved 
> around rather than new code written in this review - I don't mind the code 
> moving around without changes (and optionally before/after the move making 
> improvements, but not necessary).
> 
> If possible, might simplify the code review to move the code first/separately 
> from this change, if the move isn't too meaningless independent of the rest 
> of the changes?
https://reviews.llvm.org/D135979


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134378

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


[Lldb-commits] [lldb] 0cfa501 - [LLDB] Only run lldb-server Shell tests if it gets built

2022-10-14 Thread Alex Langford via lldb-commits

Author: Alex Langford
Date: 2022-10-14T12:00:57-07:00
New Revision: 0cfa50154a47eb65de5bc687f6565625f556edc8

URL: 
https://github.com/llvm/llvm-project/commit/0cfa50154a47eb65de5bc687f6565625f556edc8
DIFF: 
https://github.com/llvm/llvm-project/commit/0cfa50154a47eb65de5bc687f6565625f556edc8.diff

LOG: [LLDB] Only run lldb-server Shell tests if it gets built

It's easy enough to disable the lldb-server build. The lldb-server unit
tests already have logic to disable them if we don't build, so this just
makes it even.

Reviewed By: DavidSpickett

Differential Revision: https://reviews.llvm.org/D135825

Added: 
lldb/test/Shell/lldb-server/lit.local.cfg

Modified: 
lldb/test/CMakeLists.txt
lldb/test/Shell/lit.cfg.py
lldb/test/Shell/lit.site.cfg.py.in

Removed: 




diff  --git a/lldb/test/CMakeLists.txt b/lldb/test/CMakeLists.txt
index 64a9082df52eb..17572f9a0d8fd 100644
--- a/lldb/test/CMakeLists.txt
+++ b/lldb/test/CMakeLists.txt
@@ -167,6 +167,7 @@ llvm_canonicalize_cmake_booleans(
   LLVM_ENABLE_ZLIB
   LLVM_ENABLE_SHARED_LIBS
   LLDB_HAS_LIBCXX
+  LLDB_TOOL_LLDB_SERVER_BUILD
   LLDB_USE_SYSTEM_DEBUGSERVER
   LLDB_IS_64_BITS)
 

diff  --git a/lldb/test/Shell/lit.cfg.py b/lldb/test/Shell/lit.cfg.py
index 9fd206b4b373b..7717400f0de1b 100644
--- a/lldb/test/Shell/lit.cfg.py
+++ b/lldb/test/Shell/lit.cfg.py
@@ -132,6 +132,9 @@ def calculate_arch_features(arch_string):
 if config.lldb_system_debugserver:
 config.available_features.add('system-debugserver')
 
+if config.have_lldb_server:
+config.available_features.add('lldb-server')
+
 # NetBSD permits setting dbregs either if one is root
 # or if user_set_dbregs is enabled
 can_set_dbregs = True

diff  --git a/lldb/test/Shell/lit.site.cfg.py.in 
b/lldb/test/Shell/lit.site.cfg.py.in
index 29d309ca91836..d58918c873153 100644
--- a/lldb/test/Shell/lit.site.cfg.py.in
+++ b/lldb/test/Shell/lit.site.cfg.py.in
@@ -22,6 +22,7 @@ config.lldb_bitness = 64 if @LLDB_IS_64_BITS@ else 32
 config.lldb_enable_python = @LLDB_ENABLE_PYTHON@
 config.lldb_enable_lua = @LLDB_ENABLE_LUA@
 config.lldb_build_directory = "@LLDB_TEST_BUILD_DIRECTORY@"
+config.have_lldb_server = @LLDB_TOOL_LLDB_SERVER_BUILD@
 config.lldb_system_debugserver = @LLDB_USE_SYSTEM_DEBUGSERVER@
 # The shell tests use their own module caches.
 config.lldb_module_cache = os.path.join("@LLDB_TEST_MODULE_CACHE_LLDB@", 
"lldb-shell")

diff  --git a/lldb/test/Shell/lldb-server/lit.local.cfg 
b/lldb/test/Shell/lldb-server/lit.local.cfg
new file mode 100644
index 0..6ede36a690441
--- /dev/null
+++ b/lldb/test/Shell/lldb-server/lit.local.cfg
@@ -0,0 +1,3 @@
+# These tests rely on lldb-server
+if not config.have_lldb_server:
+config.unsupported = True



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


[Lldb-commits] [PATCH] D135825: [LLDB] Only run lldb-server Shell tests if it gets built

2022-10-14 Thread Alex Langford via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG0cfa50154a47: [LLDB] Only run lldb-server Shell tests if it 
gets built (authored by bulbazord).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135825

Files:
  lldb/test/CMakeLists.txt
  lldb/test/Shell/lit.cfg.py
  lldb/test/Shell/lit.site.cfg.py.in
  lldb/test/Shell/lldb-server/lit.local.cfg


Index: lldb/test/Shell/lldb-server/lit.local.cfg
===
--- /dev/null
+++ lldb/test/Shell/lldb-server/lit.local.cfg
@@ -0,0 +1,3 @@
+# These tests rely on lldb-server
+if not config.have_lldb_server:
+config.unsupported = True
Index: lldb/test/Shell/lit.site.cfg.py.in
===
--- lldb/test/Shell/lit.site.cfg.py.in
+++ lldb/test/Shell/lit.site.cfg.py.in
@@ -22,6 +22,7 @@
 config.lldb_enable_python = @LLDB_ENABLE_PYTHON@
 config.lldb_enable_lua = @LLDB_ENABLE_LUA@
 config.lldb_build_directory = "@LLDB_TEST_BUILD_DIRECTORY@"
+config.have_lldb_server = @LLDB_TOOL_LLDB_SERVER_BUILD@
 config.lldb_system_debugserver = @LLDB_USE_SYSTEM_DEBUGSERVER@
 # The shell tests use their own module caches.
 config.lldb_module_cache = os.path.join("@LLDB_TEST_MODULE_CACHE_LLDB@", 
"lldb-shell")
Index: lldb/test/Shell/lit.cfg.py
===
--- lldb/test/Shell/lit.cfg.py
+++ lldb/test/Shell/lit.cfg.py
@@ -132,6 +132,9 @@
 if config.lldb_system_debugserver:
 config.available_features.add('system-debugserver')
 
+if config.have_lldb_server:
+config.available_features.add('lldb-server')
+
 # NetBSD permits setting dbregs either if one is root
 # or if user_set_dbregs is enabled
 can_set_dbregs = True
Index: lldb/test/CMakeLists.txt
===
--- lldb/test/CMakeLists.txt
+++ lldb/test/CMakeLists.txt
@@ -167,6 +167,7 @@
   LLVM_ENABLE_ZLIB
   LLVM_ENABLE_SHARED_LIBS
   LLDB_HAS_LIBCXX
+  LLDB_TOOL_LLDB_SERVER_BUILD
   LLDB_USE_SYSTEM_DEBUGSERVER
   LLDB_IS_64_BITS)
 


Index: lldb/test/Shell/lldb-server/lit.local.cfg
===
--- /dev/null
+++ lldb/test/Shell/lldb-server/lit.local.cfg
@@ -0,0 +1,3 @@
+# These tests rely on lldb-server
+if not config.have_lldb_server:
+config.unsupported = True
Index: lldb/test/Shell/lit.site.cfg.py.in
===
--- lldb/test/Shell/lit.site.cfg.py.in
+++ lldb/test/Shell/lit.site.cfg.py.in
@@ -22,6 +22,7 @@
 config.lldb_enable_python = @LLDB_ENABLE_PYTHON@
 config.lldb_enable_lua = @LLDB_ENABLE_LUA@
 config.lldb_build_directory = "@LLDB_TEST_BUILD_DIRECTORY@"
+config.have_lldb_server = @LLDB_TOOL_LLDB_SERVER_BUILD@
 config.lldb_system_debugserver = @LLDB_USE_SYSTEM_DEBUGSERVER@
 # The shell tests use their own module caches.
 config.lldb_module_cache = os.path.join("@LLDB_TEST_MODULE_CACHE_LLDB@", "lldb-shell")
Index: lldb/test/Shell/lit.cfg.py
===
--- lldb/test/Shell/lit.cfg.py
+++ lldb/test/Shell/lit.cfg.py
@@ -132,6 +132,9 @@
 if config.lldb_system_debugserver:
 config.available_features.add('system-debugserver')
 
+if config.have_lldb_server:
+config.available_features.add('lldb-server')
+
 # NetBSD permits setting dbregs either if one is root
 # or if user_set_dbregs is enabled
 can_set_dbregs = True
Index: lldb/test/CMakeLists.txt
===
--- lldb/test/CMakeLists.txt
+++ lldb/test/CMakeLists.txt
@@ -167,6 +167,7 @@
   LLVM_ENABLE_ZLIB
   LLVM_ENABLE_SHARED_LIBS
   LLDB_HAS_LIBCXX
+  LLDB_TOOL_LLDB_SERVER_BUILD
   LLDB_USE_SYSTEM_DEBUGSERVER
   LLDB_IS_64_BITS)
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D135979: [lldb][NFCish] Move DWARFDebugInfoEntry::GetQualifiedName() into DWARFASTParserClang

2022-10-14 Thread David Blaikie via Phabricator via lldb-commits
dblaikie accepted this revision.
dblaikie added a comment.
This revision is now accepted and ready to land.

Looked like this was a fairly uncontroversial part of the other patch, so I 
feel confident approving it - but welcome to wait for a second opinion from 
more lldb-affiliated developers.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135979

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


[Lldb-commits] [PATCH] D135983: [lldb] Fix a -Wdeprecated-declarations warning

2022-10-14 Thread Nico Weber via Phabricator via lldb-commits
thakis created this revision.
thakis added a reviewer: JDevlieghere.
Herald added a project: All.
thakis requested review of this revision.

Fixes:

  ../../lldb/source/Symbol/LocateSymbolFileMacOSX.cpp:633:26:
  warning: 'CFPropertyListCreateFromXMLData' is deprecated:
   first deprecated in macOS 10.10 -
   Use CFPropertyListCreateWithData instead.
   [-Wdeprecated-declarations]
  (CFDictionaryRef)::CFPropertyListCreateFromXMLData(
 ^

Hopefully no behavior change.


https://reviews.llvm.org/D135983

Files:
  lldb/source/Symbol/LocateSymbolFileMacOSX.cpp


Index: lldb/source/Symbol/LocateSymbolFileMacOSX.cpp
===
--- lldb/source/Symbol/LocateSymbolFileMacOSX.cpp
+++ lldb/source/Symbol/LocateSymbolFileMacOSX.cpp
@@ -630,8 +630,8 @@
   command_output.size(), kCFAllocatorNull));
 
   CFCReleaser plist(
-  (CFDictionaryRef)::CFPropertyListCreateFromXMLData(
-  NULL, data.get(), kCFPropertyListImmutable, NULL));
+  (CFDictionaryRef)::CFPropertyListCreateWithData(
+  NULL, data.get(), kCFPropertyListImmutable, NULL, NULL));
 
   if (!plist.get()) {
 LLDB_LOGF(log, "'%s' failed: output is not a valid plist",


Index: lldb/source/Symbol/LocateSymbolFileMacOSX.cpp
===
--- lldb/source/Symbol/LocateSymbolFileMacOSX.cpp
+++ lldb/source/Symbol/LocateSymbolFileMacOSX.cpp
@@ -630,8 +630,8 @@
   command_output.size(), kCFAllocatorNull));
 
   CFCReleaser plist(
-  (CFDictionaryRef)::CFPropertyListCreateFromXMLData(
-  NULL, data.get(), kCFPropertyListImmutable, NULL));
+  (CFDictionaryRef)::CFPropertyListCreateWithData(
+  NULL, data.get(), kCFPropertyListImmutable, NULL, NULL));
 
   if (!plist.get()) {
 LLDB_LOGF(log, "'%s' failed: output is not a valid plist",
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D135979: [lldb][NFCish] Move DWARFDebugInfoEntry::GetQualifiedName() into DWARFASTParserClang

2022-10-14 Thread Michael Buch via Phabricator via lldb-commits
Michael137 added inline comments.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:1600
   // file and line that things are declared on.
-  std::string qualified_name;
-  if (die.GetQualifiedName(qualified_name))
-unique_typename = ConstString(qualified_name);
+  unique_typename = ConstString(GetCPlusPlusQualifiedName(die));
   unique_decl.Clear();

Overall LGTM. Correct me if I missed something, but the main functional change 
is that previously when `GetQualifiedName` returned `nullptr` we didn't 
re-assign it to `unique_typename`. However, now we would unconditionally 
assign. Does that matter? Should we only re-assign if the string is non-empty? 
Or should we use some `Optional` type?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135979

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


[Lldb-commits] [PATCH] D135917: [lldb][trace] Add a basic function call dump [2] - Implement the reconstruction algorithm

2022-10-14 Thread walter erquinigo via Phabricator via lldb-commits
wallace marked 8 inline comments as done.
wallace added inline comments.



Comment at: lldb/include/lldb/Target/TraceDumper.h:213-214
+  /// The symbol information of the delimiting instructions.
+  SymbolInfo first_symbol_info;
+  SymbolInfo last_symbol_info;
+  /// A nested call starting at the end of this segment.

jj10306 wrote:
> how would you have two different symbol info for the first and last? is last 
> referring to the symbol of the next function?
the SymbolInfo class contains disassemblies of the instruction and source line 
information, so technically two different instructions will always give you two 
different SymbolInfo objects. 



Comment at: lldb/include/lldb/Target/TraceDumper.h:216
+  /// A nested call starting at the end of this segment.
+  llvm::Optional nested_call;
+  /// Owning call

jj10306 wrote:
> nit:is the Optional redundant? thoughts on just using the UP and 
> mentioning nullptr indicates there is no nested call
I'll turn these structs into classes with stricter safety



Comment at: lldb/source/Target/TraceDumper.cpp:92-94
+  Block *inline_block_a =
+  insn.sc.block ? insn.sc.block->GetContainingInlinedBlock() : nullptr;
+  Block *inline_block_b = prev_insn.sc.block

jj10306 wrote:
> are these functions transferring ownership of these block pointers (ie are 
> you responsible for freeing this mem)?
there's no ownership transfer. They just expose the underlying pointers.



Comment at: lldb/source/Target/TraceDumper.cpp:584
+
+/// Given an instruction that happens after a return, find the ancestor 
function
+/// call that owns it. If this ancestor doesn't exist, create a new ancestor 
and

jj10306 wrote:
> "that happens after a return"
> from looking at how this function's called from 
> `AppendInstructionToFunctionCallForest` this appears to be the return 
> instruction, not the instruction after a return. is this correct?
now really. This function is called only from a switch that operates on the 
previous instruction kind, not the current instruction kind



Comment at: lldb/source/Target/TraceDumper.cpp:617-619
+  // Note: If this is not robust enough, we should actually check if we
+  // returning to the instruction that follows the last instruction from
+  // that call, as that's the behavior of CALL instructions.

jj10306 wrote:
> this would only be the case for well behaved CALL/RET, right? in the case of 
> modified stack return or JMP, this wouldn't necessarily be true?
it's still true if you pop frames out from the stack, but if you return using a 
jmp, then anything could happen.



Comment at: lldb/source/Target/TraceDumper.cpp:783-785
+  // TODO: In case of a CPU change, we create a new root because we haven't
+  // investigated yet if a call tree can safely continue or if interrupts
+  // could have polluted the original call tree.

jj10306 wrote:
> wouldn't this be a very common case where a thread is in the middle of 
> executing a function, gets context switched out of CPU X, then after some 
> time gets context switched into CPU Y and continues executing the function?
yes, and we won't solve this for the time being. We can leave this for next 
year. 



Comment at: lldb/test/API/commands/trace/TestTraceDumpFunctionCalls.py:31-32
+
+[call tree #0]
+m.out`foo() + 65 at multi_thread.cpp:12:21 to 12:21  [4, 19524]
+

jj10306 wrote:
> this is saying that events 4 - 19524 were all executed by foo() and there 
> weren't any other function calls?  am I understanding this output correctly?
> 
> 
s/events/trace items

and yes. It was a hot loop in the same function


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135917

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


[Lldb-commits] [PATCH] D135917: [lldb][trace] Add a basic function call dump [2] - Implement the reconstruction algorithm

2022-10-14 Thread walter erquinigo via Phabricator via lldb-commits
wallace updated this revision to Diff 467911.
wallace marked 7 inline comments as done.
wallace added a comment.

- address all comments
- turn all the structs into classes with safe accessors


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135917

Files:
  lldb/include/lldb/Target/TraceDumper.h
  lldb/source/Commands/CommandObjectThread.cpp
  lldb/source/Target/TraceDumper.cpp
  lldb/test/API/commands/trace/TestTraceDumpFunctionCalls.py
  lldb/test/API/commands/trace/TestTraceLoad.py
  lldb/test/API/commands/trace/inline-function/a.out
  lldb/test/API/commands/trace/inline-function/inline.cpp

Index: lldb/test/API/commands/trace/inline-function/inline.cpp
===
--- /dev/null
+++ lldb/test/API/commands/trace/inline-function/inline.cpp
@@ -0,0 +1,18 @@
+__attribute__((always_inline)) inline int mult(int x, int y) {
+  int f = x * y;
+  f++;
+  f *= f;
+  return f;
+}
+
+int foo(int x) {
+  int z = mult(x, x - 1);
+  z++;
+  return z;
+}
+
+int main() {
+  int x = 12;
+  int z = foo(x);
+  return z + x;
+}
Index: lldb/test/API/commands/trace/TestTraceLoad.py
===
--- lldb/test/API/commands/trace/TestTraceLoad.py
+++ lldb/test/API/commands/trace/TestTraceLoad.py
@@ -68,8 +68,8 @@
   "individualCounts": {
 "software disabled tracing": 1,
 "trace synchronization point": 1,
-"HW clock tick": 8,
-"CPU core changed": 1
+"CPU core changed": 1,
+"HW clock tick": 8
   }
 },
 "continuousExecutions": 1,
Index: lldb/test/API/commands/trace/TestTraceDumpFunctionCalls.py
===
--- lldb/test/API/commands/trace/TestTraceDumpFunctionCalls.py
+++ lldb/test/API/commands/trace/TestTraceDumpFunctionCalls.py
@@ -3,15 +3,107 @@
 from lldbsuite.test.decorators import *
 
 class TestTraceDumpInfo(TraceIntelPTTestCaseBase):
-def testDumpFunctionCalls(self):
+def testDumpSimpleFunctionCalls(self):
   self.expect("trace load -v " +
 os.path.join(self.getSourceDir(), "intelpt-trace", "trace.json"))
 
   self.expect("thread trace dump function-calls 2",
 error=True, substrs=['error: no thread with index: "2"'])
 
-  self.expect("thread trace dump function-calls 1 -j",
-substrs=['json = true, pretty_json = false, file = false, thread = 3842849'])
+  # We don't support yet JSON
+  self.expect("thread trace dump function-calls 1 -j", substrs=['[]'])
 
-  self.expect("thread trace dump function-calls 1 -F /tmp -J",
-substrs=['false, pretty_json = true, file = true, thread = 3842849'])
+  # We test first some code without function call
+  self.expect("thread trace dump function-calls 1",
+substrs=['''thread #1: tid = 3842849
+
+[call tree #0]
+a.out`main + 4 at main.cpp:2 to 4:0  [1, 22]'''])
+
+def testFunctionCallsWithErrors(self):
+  self.expect("trace load -v " +
+os.path.join(self.getSourceDir(), "intelpt-multi-core-trace", "trace.json"))
+
+  # We expect that tracing errors appear as a different tree
+  self.expect("thread trace dump function-calls 2",
+substrs=['''thread #2: tid = 3497496
+
+[call tree #0]
+m.out`foo() + 65 at multi_thread.cpp:12:21 to 12:21  [4, 19524]
+
+[call tree #1]
+  [19532, 19532]'''])
+
+  self.expect("thread trace dump function-calls 3",
+substrs=['''thread #3: tid = 3497497
+
+[call tree #0]
+m.out`bar() + 30 at multi_thread.cpp:19:3 to 20:6  [5, 61831]
+
+[call tree #1]
+  [61834, 61834]'''])
+
+def testInlineFunctionCalls(self):
+  self.expect("file " + os.path.join(self.getSourceDir(), "inline-function", "a.out"))
+  self.expect("b main") # we'll trace from the beginning of main
+  self.expect("b 17")
+  self.expect("r")
+  self.expect("thread trace start")
+  self.expect("c")
+  self.expect("thread trace dump function-calls",
+substrs=['''[call tree #0]
+a.out`main + 8 at inline.cpp:15:7 to 16:14  [1, 5]
+  a.out`foo(int) at inline.cpp:8:16 to 9:15  [6, 13]
+a.out`foo(int) + 22 [inlined] mult(int, int) at inline.cpp:2:7 to 5:10  [14, 21]
+  a.out`foo(int) + 49 at inline.cpp:9:15 to 12:1  [22, 26]
+a.out`main + 25 at inline.cpp:16:14 to 16:14  [27, 27]'''])
+
+def testIncompleteInlineFunctionCalls(self):
+  self.expect("file " + os.path.join(self.getSourceDir(), "inline-function", "a.out"))
+  self.expect("b 4") # we'll trace from the middle of the inline method
+  self.expect("b 17")
+  self.expect("r")
+  self.expect("thread trace start")
+  self.expect("c")
+  self.expect("thread trace dump function-calls",
+substrs=['''[call tree #0]
+a.out`main
+  a.out`foo(int)
+a.out`foo(int) + 36 [inlined] mult(int, int) + 14 at inline.cpp:4:5 to 5:10  [1, 5]
+  a.out`foo(int

[Lldb-commits] [PATCH] D135979: [lldb][NFCish] Move DWARFDebugInfoEntry::GetQualifiedName() into DWARFASTParserClang

2022-10-14 Thread Arthur Eubanks via Phabricator via lldb-commits
aeubanks updated this revision to Diff 467916.
aeubanks added a comment.

check if string is empty before assigning to unique_typename


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135979

Files:
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.h
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp

Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -2960,8 +2960,6 @@
 
 if (!try_resolving_type) {
   if (log) {
-std::string qualified_name;
-type_die.GetQualifiedName(qualified_name);
 GetObjectFile()->GetModule()->LogMessage(
 log,
 "SymbolFileDWARF::"
@@ -2969,7 +2967,7 @@
 "qualified-name='%s') ignoring die=0x%8.8x (%s)",
 DW_TAG_value_to_name(dwarf_decl_ctx[0].tag),
 dwarf_decl_ctx.GetQualifiedName(), type_die.GetOffset(),
-qualified_name.c_str());
+type_die.GetName());
   }
   return true;
 }
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
@@ -99,11 +99,6 @@
 
   const char *GetPubname(const DWARFUnit *cu) const;
 
-  const char *GetQualifiedName(DWARFUnit *cu, std::string &storage) const;
-
-  const char *GetQualifiedName(DWARFUnit *cu, const DWARFAttributes &attributes,
-   std::string &storage) const;
-
   bool GetDIENamesAndRanges(
   DWARFUnit *cu, const char *&name, const char *&mangled,
   DWARFRangeList &rangeList, int &decl_file, int &decl_line,
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
@@ -798,66 +798,6 @@
   return DWARFDIE();
 }
 
-const char *DWARFDebugInfoEntry::GetQualifiedName(DWARFUnit *cu,
-  std::string &storage) const {
-  DWARFAttributes attributes;
-  GetAttributes(cu, attributes, Recurse::yes);
-  return GetQualifiedName(cu, attributes, storage);
-}
-
-const char *
-DWARFDebugInfoEntry::GetQualifiedName(DWARFUnit *cu,
-  const DWARFAttributes &attributes,
-  std::string &storage) const {
-
-  const char *name = GetName(cu);
-
-  if (name) {
-DWARFDIE parent_decl_ctx_die = GetParentDeclContextDIE(cu);
-storage.clear();
-// TODO: change this to get the correct decl context parent
-while (parent_decl_ctx_die) {
-  const dw_tag_t parent_tag = parent_decl_ctx_die.Tag();
-  switch (parent_tag) {
-  case DW_TAG_namespace: {
-const char *namespace_name = parent_decl_ctx_die.GetName();
-if (namespace_name) {
-  storage.insert(0, "::");
-  storage.insert(0, namespace_name);
-} else {
-  storage.insert(0, "(anonymous namespace)::");
-}
-parent_decl_ctx_die = parent_decl_ctx_die.GetParentDeclContextDIE();
-  } break;
-
-  case DW_TAG_class_type:
-  case DW_TAG_structure_type:
-  case DW_TAG_union_type: {
-const char *class_union_struct_name = parent_decl_ctx_die.GetName();
-
-if (class_union_struct_name) {
-  storage.insert(0, "::");
-  storage.insert(0, class_union_struct_name);
-}
-parent_decl_ctx_die = parent_decl_ctx_die.GetParentDeclContextDIE();
-  } break;
-
-  default:
-parent_decl_ctx_die.Clear();
-break;
-  }
-}
-
-if (storage.empty())
-  storage.append("::");
-
-storage.append(name);
-  }
-  if (storage.empty())
-return nullptr;
-  return storage.c_str();
-}
-
 lldb::offset_t DWARFDebugInfoEntry::GetFirstAttributeOffset() const {
   return GetOffset() + llvm::getULEB128Size(m_abbr_idx);
 }
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.h
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.h
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.h
@@ -30,8 +30,6 @@
 
   const char *GetPubname() const;
 
-  const char *GetQualifiedName(std::string &storage) cons

[Lldb-commits] [PATCH] D134604: [clang] Instiantiate early substituted entities with sugared template arguments

2022-10-14 Thread Matheus Izvekov via Phabricator via lldb-commits
mizvekov updated this revision to Diff 467920.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134604

Files:
  clang-tools-extra/clangd/unittests/HoverTests.cpp
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/Sema/SemaTemplate.cpp
  clang/lib/Sema/SemaTemplateDeduction.cpp
  clang/lib/Sema/SemaTemplateInstantiate.cpp
  clang/test/CXX/drs/dr3xx.cpp
  clang/test/CXX/expr/expr.const/p3-0x.cpp
  clang/test/CXX/expr/expr.prim/expr.prim.req/compound-requirement.cpp
  clang/test/CXX/expr/expr.prim/expr.prim.req/nested-requirement.cpp
  clang/test/CXX/expr/expr.prim/expr.prim.req/simple-requirement.cpp
  clang/test/CXX/expr/expr.prim/expr.prim.req/type-requirement.cpp
  clang/test/CXX/temp/temp.deduct.guide/p3.cpp
  clang/test/CXX/temp/temp.param/p10-2a.cpp
  clang/test/Misc/diag-template-diffing.cpp
  clang/test/SemaCXX/sizeless-1.cpp
  clang/test/SemaTemplate/cxx2a-constraint-caching.cpp
  clang/test/SemaTemplate/instantiate-requires-expr.cpp
  clang/test/SemaTemplate/instantiation-default-1.cpp
  clang/test/SemaTemplate/make_integer_seq.cpp
  clang/test/SemaTemplate/pr52970.cpp
  clang/test/SemaTemplate/temp_arg_nontype.cpp
  
lldb/test/API/commands/expression/import-std-module/shared_ptr/TestSharedPtrFromStdModule.py
  
lldb/test/API/commands/expression/import-std-module/weak_ptr-dbg-info-content/TestDbgInfoContentWeakPtrFromStdModule.py
  
lldb/test/API/commands/expression/import-std-module/weak_ptr/TestWeakPtrFromStdModule.py

Index: lldb/test/API/commands/expression/import-std-module/weak_ptr/TestWeakPtrFromStdModule.py
===
--- lldb/test/API/commands/expression/import-std-module/weak_ptr/TestWeakPtrFromStdModule.py
+++ lldb/test/API/commands/expression/import-std-module/weak_ptr/TestWeakPtrFromStdModule.py
@@ -24,9 +24,9 @@
  result_type="std::weak_ptr",
  result_summary="3 strong=1 weak=2",
  result_children=[ValueCheck(name="__ptr_")])
-self.expect_expr("*w.lock()", result_type="int", result_value="3")
-self.expect_expr("*w.lock() = 5", result_type="int", result_value="5")
-self.expect_expr("*w.lock()", result_type="int", result_value="5")
+self.expect_expr("*w.lock()", result_type="element_type", result_value="3")
+self.expect_expr("*w.lock() = 5", result_type="element_type", result_value="5")
+self.expect_expr("*w.lock()", result_type="element_type", result_value="5")
 self.expect_expr("w.use_count()", result_type="long", result_value="1")
 self.expect("expr w.reset()")
 self.expect_expr("w.use_count()", result_type="long", result_value="0")
Index: lldb/test/API/commands/expression/import-std-module/weak_ptr-dbg-info-content/TestDbgInfoContentWeakPtrFromStdModule.py
===
--- lldb/test/API/commands/expression/import-std-module/weak_ptr-dbg-info-content/TestDbgInfoContentWeakPtrFromStdModule.py
+++ lldb/test/API/commands/expression/import-std-module/weak_ptr-dbg-info-content/TestDbgInfoContentWeakPtrFromStdModule.py
@@ -23,7 +23,7 @@
 self.expect_expr("w",
  result_type="std::weak_ptr",
  result_children=[ValueCheck(name="__ptr_")])
-self.expect_expr("*w.lock()", result_type="Foo")
+self.expect_expr("*w.lock()", result_type="element_type")
 self.expect_expr("w.lock()->a", result_type="int", result_value="3")
 self.expect_expr("w.lock()->a = 5",
  result_type="int",
Index: lldb/test/API/commands/expression/import-std-module/shared_ptr/TestSharedPtrFromStdModule.py
===
--- lldb/test/API/commands/expression/import-std-module/shared_ptr/TestSharedPtrFromStdModule.py
+++ lldb/test/API/commands/expression/import-std-module/shared_ptr/TestSharedPtrFromStdModule.py
@@ -24,9 +24,9 @@
  result_type="std::shared_ptr",
  result_summary="3 strong=1 weak=1",
  result_children=[ValueCheck(name="__ptr_")])
-self.expect_expr("*s", result_type="int", result_value="3")
-self.expect_expr("*s = 5", result_type="int", result_value="5")
-self.expect_expr("*s", result_type="int", result_value="5")
+self.expect_expr("*s", result_type="element_type", result_value="3")
+self.expect_expr("*s = 5", result_type="element_type", result_value="5")
+self.expect_expr("*s", result_type="element_type", result_value="5")
 self.expect_expr("(bool)s", result_type="bool", result_value="true")
 self.expect("expr s.reset()")
 self.expect_expr("(bool)s", result_type="bool", result_value

[Lldb-commits] [PATCH] D131858: [clang] Track the templated entity in type substitution.

2022-10-14 Thread Matheus Izvekov via Phabricator via lldb-commits
mizvekov added a comment.

I pushed some late changes:

- Implement this new scheme uniformly across all template parameter kinds. 
Before we were doing only type parameters, now we are doing non-type and 
template template parameters. While the diff is not very small, it's more of 
the same stuff.
- Fixed passing the wrong decl for variable template partial specialization.
- Relaxed assertion on Subst* construction. We previously checked we could 
access the parameter, but lldb testing showed some cases in the ASTImporter 
where the parameter might not have been imported yet. This is otherwise fine as 
it will be available before first use.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131858

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


[Lldb-commits] [PATCH] D135979: [lldb][NFCish] Move DWARFDebugInfoEntry::GetQualifiedName() into DWARFASTParserClang

2022-10-14 Thread Arthur Eubanks via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG8a98287f255b: [lldb][NFCish] Move 
DWARFDebugInfoEntry::GetQualifiedName() into… (authored by aeubanks).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135979

Files:
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.h
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp

Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -2960,8 +2960,6 @@
 
 if (!try_resolving_type) {
   if (log) {
-std::string qualified_name;
-type_die.GetQualifiedName(qualified_name);
 GetObjectFile()->GetModule()->LogMessage(
 log,
 "SymbolFileDWARF::"
@@ -2969,7 +2967,7 @@
 "qualified-name='%s') ignoring die=0x%8.8x (%s)",
 DW_TAG_value_to_name(dwarf_decl_ctx[0].tag),
 dwarf_decl_ctx.GetQualifiedName(), type_die.GetOffset(),
-qualified_name.c_str());
+type_die.GetName());
   }
   return true;
 }
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
@@ -99,11 +99,6 @@
 
   const char *GetPubname(const DWARFUnit *cu) const;
 
-  const char *GetQualifiedName(DWARFUnit *cu, std::string &storage) const;
-
-  const char *GetQualifiedName(DWARFUnit *cu, const DWARFAttributes &attributes,
-   std::string &storage) const;
-
   bool GetDIENamesAndRanges(
   DWARFUnit *cu, const char *&name, const char *&mangled,
   DWARFRangeList &rangeList, int &decl_file, int &decl_line,
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
@@ -798,66 +798,6 @@
   return DWARFDIE();
 }
 
-const char *DWARFDebugInfoEntry::GetQualifiedName(DWARFUnit *cu,
-  std::string &storage) const {
-  DWARFAttributes attributes;
-  GetAttributes(cu, attributes, Recurse::yes);
-  return GetQualifiedName(cu, attributes, storage);
-}
-
-const char *
-DWARFDebugInfoEntry::GetQualifiedName(DWARFUnit *cu,
-  const DWARFAttributes &attributes,
-  std::string &storage) const {
-
-  const char *name = GetName(cu);
-
-  if (name) {
-DWARFDIE parent_decl_ctx_die = GetParentDeclContextDIE(cu);
-storage.clear();
-// TODO: change this to get the correct decl context parent
-while (parent_decl_ctx_die) {
-  const dw_tag_t parent_tag = parent_decl_ctx_die.Tag();
-  switch (parent_tag) {
-  case DW_TAG_namespace: {
-const char *namespace_name = parent_decl_ctx_die.GetName();
-if (namespace_name) {
-  storage.insert(0, "::");
-  storage.insert(0, namespace_name);
-} else {
-  storage.insert(0, "(anonymous namespace)::");
-}
-parent_decl_ctx_die = parent_decl_ctx_die.GetParentDeclContextDIE();
-  } break;
-
-  case DW_TAG_class_type:
-  case DW_TAG_structure_type:
-  case DW_TAG_union_type: {
-const char *class_union_struct_name = parent_decl_ctx_die.GetName();
-
-if (class_union_struct_name) {
-  storage.insert(0, "::");
-  storage.insert(0, class_union_struct_name);
-}
-parent_decl_ctx_die = parent_decl_ctx_die.GetParentDeclContextDIE();
-  } break;
-
-  default:
-parent_decl_ctx_die.Clear();
-break;
-  }
-}
-
-if (storage.empty())
-  storage.append("::");
-
-storage.append(name);
-  }
-  if (storage.empty())
-return nullptr;
-  return storage.c_str();
-}
-
 lldb::offset_t DWARFDebugInfoEntry::GetFirstAttributeOffset() const {
   return GetOffset() + llvm::getULEB128Size(m_abbr_idx);
 }
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.h
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.h
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.h
@@ -30,8 +30,6 @@
 
   const char *GetPubname() const

[Lldb-commits] [lldb] 8a98287 - [lldb][NFCish] Move DWARFDebugInfoEntry::GetQualifiedName() into DWARFASTParserClang

2022-10-14 Thread Arthur Eubanks via lldb-commits

Author: Arthur Eubanks
Date: 2022-10-14T15:36:58-07:00
New Revision: 8a98287f255b974a8636c50a662e13ad61a59446

URL: 
https://github.com/llvm/llvm-project/commit/8a98287f255b974a8636c50a662e13ad61a59446
DIFF: 
https://github.com/llvm/llvm-project/commit/8a98287f255b974a8636c50a662e13ad61a59446.diff

LOG: [lldb][NFCish] Move DWARFDebugInfoEntry::GetQualifiedName() into 
DWARFASTParserClang

In D134378, we'll need the clang AST to be able to construct the qualified in 
some cases.

This makes logging in one place slightly less informative.

Reviewed By: dblaikie, Michael137

Differential Revision: https://reviews.llvm.org/D135979

Added: 


Modified: 
lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp
lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.h
lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp

Removed: 




diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index 988dc0236652..3f2614af64d4 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -1523,6 +1523,55 @@ TypeSP 
DWARFASTParserClang::UpdateSymbolContextScopeForType(
   return type_sp;
 }
 
+std::string
+DWARFASTParserClang::GetCPlusPlusQualifiedName(const DWARFDIE &die) {
+  if (!die.IsValid())
+return "";
+  const char *name = die.GetName();
+  if (!name)
+return "";
+  std::string qualified_name;
+  DWARFDIE parent_decl_ctx_die = die.GetParentDeclContextDIE();
+  // TODO: change this to get the correct decl context parent
+  while (parent_decl_ctx_die) {
+const dw_tag_t parent_tag = parent_decl_ctx_die.Tag();
+switch (parent_tag) {
+case DW_TAG_namespace: {
+  if (const char *namespace_name = parent_decl_ctx_die.GetName()) {
+qualified_name.insert(0, "::");
+qualified_name.insert(0, namespace_name);
+  } else {
+qualified_name.insert(0, "(anonymous namespace)::");
+  }
+  parent_decl_ctx_die = parent_decl_ctx_die.GetParentDeclContextDIE();
+  break;
+}
+
+case DW_TAG_class_type:
+case DW_TAG_structure_type:
+case DW_TAG_union_type: {
+  if (const char *class_union_struct_name = parent_decl_ctx_die.GetName()) 
{
+qualified_name.insert(0, "::");
+qualified_name.insert(0, class_union_struct_name);
+  }
+  parent_decl_ctx_die = parent_decl_ctx_die.GetParentDeclContextDIE();
+  break;
+}
+
+default:
+  parent_decl_ctx_die.Clear();
+  break;
+}
+  }
+
+  if (qualified_name.empty())
+qualified_name.append("::");
+
+  qualified_name.append(name);
+
+  return qualified_name;
+}
+
 TypeSP
 DWARFASTParserClang::ParseStructureLikeDIE(const SymbolContext &sc,
const DWARFDIE &die,
@@ -1548,8 +1597,8 @@ DWARFASTParserClang::ParseStructureLikeDIE(const 
SymbolContext &sc,
   // For C++, we rely solely upon the one definition rule that says
   // only one thing can exist at a given decl context. We ignore the
   // file and line that things are declared on.
-  std::string qualified_name;
-  if (die.GetQualifiedName(qualified_name))
+  std::string qualified_name = GetCPlusPlusQualifiedName(die);
+  if (!qualified_name.empty())
 unique_typename = ConstString(qualified_name);
   unique_decl.Clear();
 }

diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h 
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
index a8640b4a5693..860e5f4f7d5b 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
@@ -121,11 +121,14 @@ class DWARFASTParserClang : public DWARFASTParser {
   bool ParseTemplateDIE(const DWARFDIE &die,
 lldb_private::TypeSystemClang::TemplateParameterInfos
 &template_param_infos);
+
   bool ParseTemplateParameterInfos(
   const DWARFDIE &parent_die,
   lldb_private::TypeSystemClang::TemplateParameterInfos
   &template_param_infos);
 
+  std::string GetCPlusPlusQualifiedName(const DWARFDIE &die);
+
   bool ParseChildMembers(
   const DWARFDIE &die, lldb_private::CompilerType &class_compiler_type,
   std::vector> &base_classes,

diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp
index e8492079af88..7b9da6fa166f 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp
@@ -210,13 +210,6 @@ const c

[Lldb-commits] [PATCH] D134378: [lldb] Support simplified template names

2022-10-14 Thread Arthur Eubanks via Phabricator via lldb-commits
aeubanks updated this revision to Diff 467946.
aeubanks added a comment.

rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134378

Files:
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
  lldb/test/API/lang/cpp/complete-type-check/TestCppIsTypeComplete.py
  lldb/test/API/lang/cpp/unique-types2/Makefile
  lldb/test/API/lang/cpp/unique-types2/TestUniqueTypes.py
  lldb/test/API/lang/cpp/unique-types2/main.cpp

Index: lldb/test/API/lang/cpp/unique-types2/main.cpp
===
--- /dev/null
+++ lldb/test/API/lang/cpp/unique-types2/main.cpp
@@ -0,0 +1,22 @@
+template  class Foo {
+  T t;
+};
+
+template  class FooPack {
+  T t;
+};
+
+int main() {
+  Foo t1;
+  Foo t2;
+  Foo> t3;
+
+  FooPack p1;
+  FooPack p2;
+  FooPack> p3;
+  FooPack t4;
+  FooPack t5;
+  FooPack t6;
+  FooPack t7;
+  // Set breakpoint here
+}
Index: lldb/test/API/lang/cpp/unique-types2/TestUniqueTypes.py
===
--- /dev/null
+++ lldb/test/API/lang/cpp/unique-types2/TestUniqueTypes.py
@@ -0,0 +1,40 @@
+"""
+Test that we return only the requested template instantiation.
+"""
+
+import lldb
+import lldbsuite.test.lldbutil as lldbutil
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+
+class UniqueTypesTestCase(TestBase):
+def do_test(self, debug_flags):
+"""Test that we only display the requested Foo instantiation, not all Foo instantiations."""
+self.build(dictionary=debug_flags)
+lldbutil.run_to_source_breakpoint(self, "// Set breakpoint here", lldb.SBFileSpec("main.cpp"))
+
+self.expect("image lookup -A -t 'Foo'", DATA_TYPES_DISPLAYED_CORRECTLY, substrs=["1 match found"])
+self.expect("image lookup -A -t 'Foo'", DATA_TYPES_DISPLAYED_CORRECTLY, substrs=["1 match found"])
+self.expect("image lookup -A -t 'Foo >'", DATA_TYPES_DISPLAYED_CORRECTLY, substrs=["1 match found"])
+self.expect("image lookup -A -t 'Foo'", DATA_TYPES_DISPLAYED_CORRECTLY, error=True)
+
+self.expect("image lookup -A -t 'FooPack'", DATA_TYPES_DISPLAYED_CORRECTLY, substrs=["1 match found"])
+self.expect("image lookup -A -t 'FooPack'", DATA_TYPES_DISPLAYED_CORRECTLY, substrs=["1 match found"])
+self.expect("image lookup -A -t 'FooPack >'", DATA_TYPES_DISPLAYED_CORRECTLY, substrs=["1 match found"])
+self.expect("image lookup -A -t 'FooPack'", DATA_TYPES_DISPLAYED_CORRECTLY, substrs=["1 match found"])
+self.expect("image lookup -A -t 'FooPack'", DATA_TYPES_DISPLAYED_CORRECTLY, substrs=["1 match found"])
+self.expect("image lookup -A -t 'FooPack'", DATA_TYPES_DISPLAYED_CORRECTLY, substrs=["1 match found"])
+self.expect("image lookup -A -t 'FooPack'", DATA_TYPES_DISPLAYED_CORRECTLY, substrs=["1 match found"])
+self.expect("image lookup -A -t 'FooPack'", DATA_TYPES_DISPLAYED_CORRECTLY, error=True)
+self.expect("image lookup -A -t 'FooPack'", DATA_TYPES_DISPLAYED_CORRECTLY, error=True)
+
+@skipIf(compiler=no_match("clang"))
+@skipIf(compiler_version=["<", "15.0"])
+def test_simple_template_name(self):
+self.do_test(dict(CFLAGS_EXTRAS="-gsimple-template-names"))
+
+@skipIf(compiler=no_match("clang"))
+@skipIf(compiler_version=["<", "15.0"])
+def test_no_simple_template_name(self):
+self.do_test(dict(CFLAGS_EXTRAS="-gno-simple-template-names"))
+
Index: lldb/test/API/lang/cpp/unique-types2/Makefile
===
--- /dev/null
+++ lldb/test/API/lang/cpp/unique-types2/Makefile
@@ -0,0 +1,3 @@
+CXX_SOURCES := main.cpp
+
+include Makefile.rules
Index: lldb/test/API/lang/cpp/complete-type-check/TestCppIsTypeComplete.py
===
--- lldb/test/API/lang/cpp/complete-type-check/TestCppIsTypeComplete.py
+++ lldb/test/API/lang/cpp/complete-type-check/TestCppIsTypeComplete.py
@@ -51,7 +51,7 @@
 # Record types without a defining declaration are not complete.
 self.assertPointeeIncomplete("FwdClass *", "fwd_class")
 self.assertPointeeIncomplete("FwdClassTypedef *", "fwd_class_typedef")
-self.assertPointeeIncomplete("FwdTemplateClass<> *", "fwd_template_class")
+self.assertPointeeIncomplete("FwdTemplateClass *", "fwd_template_class")
 
 # A pointer type is complete even when it points to an incomplete type.
 fwd_class_ptr = self.expect_expr("fwd_class", result_type="FwdClass *")
Index: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
===
--- lldb/source/Plugins/TypeS

[Lldb-commits] [lldb] d4a55ad - [lldb][Breakpoint] Fix setting breakpoints on templates by basename

2022-10-14 Thread Michael Buch via lldb-commits

Author: Michael Buch
Date: 2022-10-14T23:51:00+01:00
New Revision: d4a55ad346514b2478762cbc198942c72347e81e

URL: 
https://github.com/llvm/llvm-project/commit/d4a55ad346514b2478762cbc198942c72347e81e
DIFF: 
https://github.com/llvm/llvm-project/commit/d4a55ad346514b2478762cbc198942c72347e81e.diff

LOG: [lldb][Breakpoint] Fix setting breakpoints on templates by basename

This patch fixes a regression with setting breakpoints on template
functions by name. E.g.,:
```
$ cat main.cpp
template
struct Foo {
  template
  void func() {}
};

int main() {
  Foo f;
  f.func();
}

(lldb) br se -n func
```

This has regressed since `3339000e0bda696c2e29173d15958c0a4978a143`
where we started using the `CPlusPlusNameParser` for getting the
basename of the function symbol and match it exactly against
the name in the breakpoint command. The parser will include template
parameters in the basename, so the exact match will always fail

**Testing**

* Added API tests
* Added unit-tests

Differential Revision: https://reviews.llvm.org/D135921

Added: 


Modified: 
lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h
lldb/test/API/functionalities/breakpoint/cpp/Makefile
lldb/test/API/functionalities/breakpoint/cpp/TestCPPBreakpointLocations.py
lldb/test/API/functionalities/breakpoint/cpp/main.cpp
lldb/unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp

Removed: 




diff  --git a/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp 
b/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
index 0339d38904759..d4dfc4c0e1d2e 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
@@ -269,9 +269,21 @@ std::string 
CPlusPlusLanguage::MethodName::GetScopeQualifiedName() {
   return res;
 }
 
+llvm::StringRef
+CPlusPlusLanguage::MethodName::GetBasenameNoTemplateParameters() {
+  llvm::StringRef basename = GetBasename();
+  size_t arg_start, arg_end;
+  llvm::StringRef parens("<>", 2);
+  if (ReverseFindMatchingChars(basename, parens, arg_start, arg_end))
+return basename.substr(0, arg_start);
+
+  return basename;
+}
+
 bool CPlusPlusLanguage::MethodName::ContainsPath(llvm::StringRef path) {
   if (!m_parsed)
 Parse();
+
   // If we can't parse the incoming name, then just check that it contains 
path.
   if (m_parse_error)
 return m_full.GetStringRef().contains(path);
@@ -286,8 +298,23 @@ bool 
CPlusPlusLanguage::MethodName::ContainsPath(llvm::StringRef path) {
   if (!success)
 return m_full.GetStringRef().contains(path);
 
-  if (identifier != GetBasename())
+  // Basename may include template arguments.
+  // E.g.,
+  // GetBaseName(): func
+  // identifier   : func
+  //
+  // ...but we still want to account for identifiers with template parameter
+  // lists, e.g., when users set breakpoints on template specializations.
+  //
+  // E.g.,
+  // GetBaseName(): func
+  // identifier   : func
+  //
+  // Try to match the basename with or without template parameters.
+  if (GetBasename() != identifier &&
+  GetBasenameNoTemplateParameters() != identifier)
 return false;
+
   // Incoming path only had an identifier, so we match.
   if (context.empty())
 return true;

diff  --git a/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h 
b/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h
index 53a01cfc4799d..4d4226accd02b 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h
+++ b/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h
@@ -58,6 +58,21 @@ class CPlusPlusLanguage : public Language {
 
 bool ContainsPath(llvm::StringRef path);
 
+  private:
+/// Returns the Basename of this method without a template parameter
+/// list, if any.
+///
+// Examples:
+//
+//   ++-+
+//   | MethodName | Returns |
+//   ++-+
+//   | void func()| func|
+//   | void func()   | func|
+//   | void func>()  | func|
+//   ++-+
+llvm::StringRef GetBasenameNoTemplateParameters();
+
   protected:
 void Parse();
 bool TrySimplifiedParse();

diff  --git a/lldb/test/API/functionalities/breakpoint/cpp/Makefile 
b/lldb/test/API/functionalities/breakpoint/cpp/Makefile
index 2c00681fa2280..66108b79e7fe0 100644
--- a/lldb/test/API/functionalities/breakpoint/cpp/Makefile
+++ b/lldb/test/API/functionalities/breakpoint/cpp/Makefile
@@ -1,4 +1,5 @@
 CXX_SOURCES := main.cpp
+CXXFLAGS_EXTRAS := -std=c++14
 
 ifneq (,$(findstring icc,$(CC)))
 CXXFLAGS_EXTRAS := -debug inline-debug-info

diff  --git 
a/lldb/test/API/functionalities/breakpoint/cpp/TestCPPBreakpointLocations.py 
b/l

[Lldb-commits] [PATCH] D135921: [lldb][Breakpoint] Fix setting breakpoints on templates by basename

2022-10-14 Thread Michael Buch via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGd4a55ad34651: [lldb][Breakpoint] Fix setting breakpoints on 
templates by basename (authored by Michael137).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135921

Files:
  lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
  lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h
  lldb/test/API/functionalities/breakpoint/cpp/Makefile
  lldb/test/API/functionalities/breakpoint/cpp/TestCPPBreakpointLocations.py
  lldb/test/API/functionalities/breakpoint/cpp/main.cpp
  lldb/unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp

Index: lldb/unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp
===
--- lldb/unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp
+++ lldb/unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp
@@ -143,7 +143,12 @@
   CPlusPlusLanguage::MethodName reference_3(ConstString("int func01()"));
   CPlusPlusLanguage::MethodName 
   reference_4(ConstString("bar::baz::operator bool()"));
-  
+  CPlusPlusLanguage::MethodName reference_5(
+  ConstString("bar::baz::operator bool>()"));
+  CPlusPlusLanguage::MethodName reference_6(ConstString(
+  "bar::baz::operator<<, Type>>()"));
+
+  EXPECT_TRUE(reference_1.ContainsPath(""));
   EXPECT_TRUE(reference_1.ContainsPath("func01"));
   EXPECT_TRUE(reference_1.ContainsPath("bar::func01"));
   EXPECT_TRUE(reference_1.ContainsPath("foo::bar::func01"));
@@ -153,17 +158,35 @@
   EXPECT_FALSE(reference_1.ContainsPath("::foo::baz::func01"));
   EXPECT_FALSE(reference_1.ContainsPath("foo::bar::baz::func01"));
   
+  EXPECT_TRUE(reference_2.ContainsPath(""));
   EXPECT_TRUE(reference_2.ContainsPath("foofoo::bar::func01"));
   EXPECT_FALSE(reference_2.ContainsPath("foo::bar::func01"));
   
+  EXPECT_TRUE(reference_3.ContainsPath(""));
   EXPECT_TRUE(reference_3.ContainsPath("func01"));
   EXPECT_FALSE(reference_3.ContainsPath("func"));
   EXPECT_FALSE(reference_3.ContainsPath("bar::func01"));
 
+  EXPECT_TRUE(reference_4.ContainsPath(""));
+  EXPECT_TRUE(reference_4.ContainsPath("operator"));
   EXPECT_TRUE(reference_4.ContainsPath("operator bool"));
   EXPECT_TRUE(reference_4.ContainsPath("baz::operator bool"));
   EXPECT_TRUE(reference_4.ContainsPath("bar::baz::operator bool"));
   EXPECT_FALSE(reference_4.ContainsPath("az::operator bool"));
+
+  EXPECT_TRUE(reference_5.ContainsPath(""));
+  EXPECT_TRUE(reference_5.ContainsPath("operator"));
+  EXPECT_TRUE(reference_5.ContainsPath("operator bool"));
+  EXPECT_TRUE(reference_5.ContainsPath("operator bool>"));
+  EXPECT_FALSE(reference_5.ContainsPath("operator bool"));
+  EXPECT_FALSE(reference_5.ContainsPath("operator bool>"));
+
+  EXPECT_TRUE(reference_6.ContainsPath(""));
+  EXPECT_TRUE(reference_6.ContainsPath("operator"));
+  EXPECT_TRUE(reference_6.ContainsPath("operator<<"));
+  EXPECT_TRUE(reference_6.ContainsPath(
+  "bar::baz::operator<<, Type>>()"));
+  EXPECT_FALSE(reference_6.ContainsPath("operator<<>"));
 }
 
 TEST(CPlusPlusLanguage, ExtractContextAndIdentifier) {
Index: lldb/test/API/functionalities/breakpoint/cpp/main.cpp
===
--- lldb/test/API/functionalities/breakpoint/cpp/main.cpp
+++ lldb/test/API/functionalities/breakpoint/cpp/main.cpp
@@ -82,6 +82,20 @@
 };
 }
 
+namespace ns {
+template  struct Foo {
+  template  void import() {}
+
+  template  auto func() {}
+
+  operator bool() { return true; }
+
+  template  operator T() { return {}; }
+
+  template  void operator<<(T t) {}
+};
+} // namespace ns
+
 int main (int argc, char const *argv[])
 {
 a::c ac;
@@ -98,5 +112,16 @@
 bc.func3();
 cd.func2();
 cd.func3();
+
+ns::Foo f;
+f.import ();
+f.func();
+f.func>();
+f.operator bool();
+f.operator a::c();
+f.operator ns::Foo();
+f.operator<<(5);
+f.operator<< >({});
+
 return 0;
 }
Index: lldb/test/API/functionalities/breakpoint/cpp/TestCPPBreakpointLocations.py
===
--- lldb/test/API/functionalities/breakpoint/cpp/TestCPPBreakpointLocations.py
+++ lldb/test/API/functionalities/breakpoint/cpp/TestCPPBreakpointLocations.py
@@ -54,6 +54,23 @@
 {'name': 'a::c::func1()', 'loc_names': ['a::c::func1()']},
 {'name': 'b::c::func1()', 'loc_names': ['b::c::func1()']},
 {'name': 'c::d::func2()', 'loc_names': ['c::d::func2()']},
+
+# Template cases
+{'name': 'func', 'loc_names': []},
+{'name': 'func', 'loc_names': ['auto ns::Foo::func()']},
+{'name': 'func', 'loc_names': ['auto ns::Foo::func()',
+   'auto ns::Foo::func>()']},
+
+{'name': 'operator', 'loc_names': []},
+{'name': 'ns::Foo::operator bool'

[Lldb-commits] [PATCH] D135979: [lldb][NFCish] Move DWARFDebugInfoEntry::GetQualifiedName() into DWARFASTParserClang

2022-10-14 Thread Shafik Yaghmour via Phabricator via lldb-commits
shafik added inline comments.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:1534
+  std::string qualified_name;
+  DWARFDIE parent_decl_ctx_die = die.GetParentDeclContextDIE();
+  // TODO: change this to get the correct decl context parent

So it looks like you can do `SymbolFileDWARF::GetDWARFDeclContext(...)`  and 
`DWARFDeclContext` has `GetQualifiedName()` which I think is doing what you are 
doing below and maybe handles more cases. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135979

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


[Lldb-commits] [PATCH] D135998: Make sure Target::EvaluateExpression() passes up an error instead of silently dropping it.

2022-10-14 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl created this revision.
aprantl added reviewers: jingham, kastiglione, labath.
Herald added a project: All.
aprantl requested review of this revision.

When `UserExpression::Evaluate()` fails and doesn't return a ValueObject there 
is no vehicle for returning the error in the return value.

This behavior can be observed by applying the following patch:

  diff --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp
  index f1a311b7252c..58c03ccdb068 100644
  --- a/lldb/source/Target/Target.cpp
  +++ b/lldb/source/Target/Target.cpp
  @@ -2370,6 +2370,7 @@ UserExpression *Target::GetUserExpressionForLanguage(
   Expression::ResultType desired_type,
   const EvaluateExpressionOptions &options, ValueObject *ctx_obj,
   Status &error) {
  +  error.SetErrorStringWithFormat("Ha ha!");  return nullptr;
 auto type_system_or_err = GetScratchTypeSystemForLanguage(language);
 if (auto err = type_system_or_err.takeError()) {
   error.SetErrorStringWithFormat(

and then running

  $ lldb -o "p 1"
  (lldb) p 1
  (lldb)

This patch fixes this by creating an empty result ValueObject that wraps the 
error.


https://reviews.llvm.org/D135998

Files:
  lldb/source/Commands/CommandObjectExpression.cpp
  lldb/source/Target/Target.cpp


Index: lldb/source/Target/Target.cpp
===
--- lldb/source/Target/Target.cpp
+++ lldb/source/Target/Target.cpp
@@ -26,6 +26,7 @@
 #include "lldb/Core/StreamFile.h"
 #include "lldb/Core/StructuredDataImpl.h"
 #include "lldb/Core/ValueObject.h"
+#include "lldb/Core/ValueObjectConstResult.h"
 #include "lldb/Expression/DiagnosticManager.h"
 #include "lldb/Expression/ExpressionVariable.h"
 #include "lldb/Expression/REPL.h"
@@ -2528,6 +2529,10 @@
 execution_results = UserExpression::Evaluate(exe_ctx, options, expr, 
prefix,
  result_valobj_sp, error,
  fixed_expression, ctx_obj);
+// Pass up the error by wrapping it inside an error result.
+if (error.Fail() && !result_valobj_sp)
+  result_valobj_sp = ValueObjectConstResult::Create(
+  exe_ctx.GetBestExecutionContextScope(), error);
   }
 
   if (execution_results == eExpressionCompleted)
Index: lldb/source/Commands/CommandObjectExpression.cpp
===
--- lldb/source/Commands/CommandObjectExpression.cpp
+++ lldb/source/Commands/CommandObjectExpression.cpp
@@ -461,6 +461,8 @@
 result.SetStatus(eReturnStatusFailed);
   }
 }
+  } else {
+error_stream.Printf("error: unknown error\n");
   }
 
   return (success != eExpressionSetupError &&


Index: lldb/source/Target/Target.cpp
===
--- lldb/source/Target/Target.cpp
+++ lldb/source/Target/Target.cpp
@@ -26,6 +26,7 @@
 #include "lldb/Core/StreamFile.h"
 #include "lldb/Core/StructuredDataImpl.h"
 #include "lldb/Core/ValueObject.h"
+#include "lldb/Core/ValueObjectConstResult.h"
 #include "lldb/Expression/DiagnosticManager.h"
 #include "lldb/Expression/ExpressionVariable.h"
 #include "lldb/Expression/REPL.h"
@@ -2528,6 +2529,10 @@
 execution_results = UserExpression::Evaluate(exe_ctx, options, expr, prefix,
  result_valobj_sp, error,
  fixed_expression, ctx_obj);
+// Pass up the error by wrapping it inside an error result.
+if (error.Fail() && !result_valobj_sp)
+  result_valobj_sp = ValueObjectConstResult::Create(
+  exe_ctx.GetBestExecutionContextScope(), error);
   }
 
   if (execution_results == eExpressionCompleted)
Index: lldb/source/Commands/CommandObjectExpression.cpp
===
--- lldb/source/Commands/CommandObjectExpression.cpp
+++ lldb/source/Commands/CommandObjectExpression.cpp
@@ -461,6 +461,8 @@
 result.SetStatus(eReturnStatusFailed);
   }
 }
+  } else {
+error_stream.Printf("error: unknown error\n");
   }
 
   return (success != eExpressionSetupError &&
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D135998: Make sure Target::EvaluateExpression() passes up an error instead of silently dropping it.

2022-10-14 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl added a comment.

In case anyone is curious about the motivation — In swift-lldb 
`GetUserExpressionForLanguage()` can actually fail and then you get into this 
very situation. Even though the expression still fails, a command line user has 
no feedback about this.


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

https://reviews.llvm.org/D135998

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


[Lldb-commits] [PATCH] D135979: [lldb][NFCish] Move DWARFDebugInfoEntry::GetQualifiedName() into DWARFASTParserClang

2022-10-14 Thread Arthur Eubanks via Phabricator via lldb-commits
aeubanks added inline comments.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:1534
+  std::string qualified_name;
+  DWARFDIE parent_decl_ctx_die = die.GetParentDeclContextDIE();
+  // TODO: change this to get the correct decl context parent

shafik wrote:
> So it looks like you can do `SymbolFileDWARF::GetDWARFDeclContext(...)`  and 
> `DWARFDeclContext` has `GetQualifiedName()` which I think is doing what you 
> are doing below and maybe handles more cases. 
`DWARFDeclContext` doesn't have access to the clang AST, which in D134378 we 
will use to reconstruct template parameters (I hope I'm understanding your 
suggestion correctly, still new to lldb)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135979

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


[Lldb-commits] [PATCH] D136006: [LLDB][NativePDB] Fix ParseAllNamespacesPlusChildrenOf and avoid duplicate iteration on symbol stream

2022-10-14 Thread Zequan Wu via Phabricator via lldb-commits
zequanwu created this revision.
Herald added a project: All.
zequanwu requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D136006

Files:
  lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp
  lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.h

Index: lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.h
===
--- lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.h
+++ lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.h
@@ -11,6 +11,7 @@
 
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Threading.h"
 
 #include "Plugins/ExpressionParser/Clang/ClangASTImporter.h"
 
@@ -122,7 +123,9 @@
  TypeIndex func_ti, CompilerType func_ct,
  uint32_t param_count, clang::StorageClass func_storage,
  bool is_inline, clang::DeclContext *parent);
-  void ParseAllNamespacesPlusChildrenOf(llvm::Optional parent);
+  void ParseAllNamespacesPlusChildrenOf(llvm::StringRef parent);
+  void ParseAllTypes();
+  void ParseAllFunctionsAndNonLocalVars();
   void ParseDeclsForSimpleContext(clang::DeclContext &context);
   void ParseBlockChildren(PdbCompilandSymId block_id);
 
@@ -135,7 +138,8 @@
   TypeSystemClang &m_clang;
 
   ClangASTImporter m_importer;
-
+  llvm::once_flag m_parse_functions_and_non_local_vars;
+  llvm::once_flag m_parse_all_types;
   llvm::DenseMap m_decl_to_status;
   llvm::DenseMap m_uid_to_decl;
   llvm::DenseMap m_uid_to_type;
Index: lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp
===
--- lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp
+++ lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp
@@ -14,6 +14,7 @@
 #include "llvm/DebugInfo/PDB/Native/TpiStream.h"
 #include "llvm/Demangle/MicrosoftDemangle.h"
 
+
 #include "Plugins/ExpressionParser/Clang/ClangASTMetadata.h"
 #include "Plugins/ExpressionParser/Clang/ClangUtil.h"
 #include "Plugins/Language/CPlusPlus/MSVCUndecoratedNameParser.h"
@@ -21,7 +22,7 @@
 #include "lldb/Core/Module.h"
 #include "lldb/Symbol/ObjectFile.h"
 #include "lldb/Utility/LLDBAssert.h"
-
+#include "lldb/Utility/Timer.h"
 #include "PdbUtil.h"
 #include "UdtRecordCompleter.h"
 #include "SymbolFileNativePDB.h"
@@ -1233,7 +1234,7 @@
 }
 
 void PdbAstBuilder::ParseAllNamespacesPlusChildrenOf(
-llvm::Optional parent) {
+llvm::StringRef parent) {
   SymbolFileNativePDB *pdb = static_cast(
   m_clang.GetSymbolFile()->GetBackingSymbolFile());
   PdbIndex &index = pdb->GetIndex();
@@ -1247,12 +1248,6 @@
 
 CVTagRecord tag = CVTagRecord::create(cvt);
 
-if (!parent) {
-  clang::QualType qt = GetOrCreateType(tid);
-  CompleteType(qt);
-  continue;
-}
-
 // Call CreateDeclInfoForType unconditionally so that the namespace info
 // gets created.  But only call CreateRecordType if the namespace name
 // matches.
@@ -1263,41 +1258,69 @@
   continue;
 
 clang::NamespaceDecl *ns = llvm::cast(context);
-std::string actual_ns = ns->getQualifiedNameAsString();
-if (llvm::StringRef(actual_ns).startswith(*parent)) {
-  clang::QualType qt = GetOrCreateType(tid);
-  CompleteType(qt);
-  continue;
+llvm::StringRef ns_name = ns->getName();
+if (ns_name.startswith(parent)) {
+  ns_name = ns_name.drop_front(parent.size());
+  if (ns_name.startswith("::")) {
+clang::QualType qt = GetOrCreateType(tid);
+CompleteType(qt);
+  }
 }
   }
+}
 
-  uint32_t module_count = index.dbi().modules().getModuleCount();
-  for (uint16_t modi = 0; modi < module_count; ++modi) {
-CompilandIndexItem &cii = index.compilands().GetOrCreateCompiland(modi);
-const CVSymbolArray &symbols = cii.m_debug_stream.getSymbolArray();
-auto iter = symbols.begin();
-while (iter != symbols.end()) {
-  PdbCompilandSymId sym_id{modi, iter.offset()};
-
-  switch (iter->kind()) {
-  case S_GPROC32:
-  case S_LPROC32:
-GetOrCreateFunctionDecl(sym_id);
-iter = symbols.at(getScopeEndOffset(*iter));
-break;
-  case S_GDATA32:
-  case S_GTHREAD32:
-  case S_LDATA32:
-  case S_LTHREAD32:
-GetOrCreateVariableDecl(PdbCompilandSymId(modi, 0), sym_id);
-++iter;
-break;
-  default:
-++iter;
+void PdbAstBuilder::ParseAllTypes() {
+  llvm::call_once(m_parse_all_types, [this]() {
+SymbolFileNativePDB *pdb = static_cast(
+m_clang.GetSymbolFile()->GetBackingSymbolFile());
+PdbIndex &index = pdb->GetIndex();
+TypeIndex ti{index.tpi().TypeIndexBegin()};
+for (const CVType &cvt : index.tpi().typeArray()) {
+  PdbTypeSymId tid{ti};
+  ++ti;
+
+  if (!IsTagRecord(cvt))
 continue;
+
+  clang::QualType qt = GetOrCreateTy

[Lldb-commits] [PATCH] D135648: [lldb] Add matching based on Python callbacks for data formatters.

2022-10-14 Thread Jorge Gorbe Moya via Phabricator via lldb-commits
jgorbe updated this revision to Diff 467987.
jgorbe added a comment.

Address review comments.


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

https://reviews.llvm.org/D135648

Files:
  lldb/bindings/python/python-swigsafecast.swig
  lldb/bindings/python/python-wrapper.swig
  lldb/include/lldb/API/SBType.h
  lldb/include/lldb/DataFormatters/DataVisualization.h
  lldb/include/lldb/DataFormatters/FormatClasses.h
  lldb/include/lldb/DataFormatters/FormatManager.h
  lldb/include/lldb/DataFormatters/FormattersContainer.h
  lldb/include/lldb/DataFormatters/TypeCategory.h
  lldb/include/lldb/DataFormatters/TypeCategoryMap.h
  lldb/include/lldb/Interpreter/ScriptInterpreter.h
  lldb/include/lldb/Target/Language.h
  lldb/include/lldb/lldb-enumerations.h
  lldb/source/API/SBTypeNameSpecifier.cpp
  lldb/source/Commands/CommandObjectType.cpp
  lldb/source/DataFormatters/DataVisualization.cpp
  lldb/source/DataFormatters/FormatManager.cpp
  lldb/source/DataFormatters/TypeCategory.cpp
  lldb/source/DataFormatters/TypeCategoryMap.cpp
  lldb/source/Plugins/Language/ObjC/ObjCLanguage.cpp
  lldb/source/Plugins/Language/ObjC/ObjCLanguage.h
  lldb/source/Plugins/ScriptInterpreter/Python/SWIGPythonBridge.h
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
  lldb/source/Target/Language.cpp
  lldb/test/API/functionalities/data-formatter/callback-matching/Makefile
  
lldb/test/API/functionalities/data-formatter/callback-matching/TestDataFormatterCallbackMatching.py
  
lldb/test/API/functionalities/data-formatter/callback-matching/formatters_with_callback.py
  lldb/test/API/functionalities/data-formatter/callback-matching/main.cpp
  lldb/unittests/DataFormatter/FormattersContainerTest.cpp
  lldb/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp

Index: lldb/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp
===
--- lldb/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp
+++ lldb/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp
@@ -67,6 +67,12 @@
   return false;
 }
 
+bool lldb_private::LLDBSwigPythonFormatterCallbackFunction(
+const char *python_function_name, const char *session_dictionary_name,
+lldb::TypeImplSP type_impl_sp) {
+  return false;
+}
+
 bool lldb_private::LLDBSwigPythonCallTypeScript(
 const char *python_function_name, const void *session_dictionary,
 const lldb::ValueObjectSP &valobj_sp, void **pyfunct_wrapper,
Index: lldb/unittests/DataFormatter/FormattersContainerTest.cpp
===
--- lldb/unittests/DataFormatter/FormattersContainerTest.cpp
+++ lldb/unittests/DataFormatter/FormattersContainerTest.cpp
@@ -7,12 +7,20 @@
 //===--===//
 
 #include "lldb/DataFormatters/FormattersContainer.h"
+#include "lldb/DataFormatters/FormatClasses.h"
 
 #include "gtest/gtest.h"
 
 using namespace lldb;
 using namespace lldb_private;
 
+// Creates a dummy candidate with just a type name in order to test the string
+// matching (exact name match and regex match) paths.
+FormattersMatchCandidate CandidateFromTypeName(const char *type_name) {
+  return FormattersMatchCandidate(ConstString(type_name), nullptr, TypeImpl(),
+  FormattersMatchCandidate::Flags());
+}
+
 // All the prefixes that the exact name matching will strip from the type.
 static const std::vector exact_name_prefixes = {
 "", // no prefix.
@@ -25,63 +33,63 @@
 SCOPED_TRACE("Prefix: " + prefix);
 
 TypeMatcher matcher(ConstString(prefix + "Name"));
-EXPECT_TRUE(matcher.Matches(ConstString("class Name")));
-EXPECT_TRUE(matcher.Matches(ConstString("struct Name")));
-EXPECT_TRUE(matcher.Matches(ConstString("union Name")));
-EXPECT_TRUE(matcher.Matches(ConstString("enum Name")));
-EXPECT_TRUE(matcher.Matches(ConstString("Name")));
-
-EXPECT_FALSE(matcher.Matches(ConstString("Name ")));
-EXPECT_FALSE(matcher.Matches(ConstString("ame")));
-EXPECT_FALSE(matcher.Matches(ConstString("Nam")));
-EXPECT_FALSE(matcher.Matches(ConstString("am")));
-EXPECT_FALSE(matcher.Matches(ConstString("a")));
-EXPECT_FALSE(matcher.Matches(ConstString(" ")));
-EXPECT_FALSE(matcher.Matches(ConstString("class N")));
-EXPECT_FALSE(matcher.Matches(ConstString("class ")));
-EXPECT_FALSE(matcher.Matches(ConstString("class")));
+EXPECT_TRUE(matcher.Matches(CandidateFromTypeName("class Name")));
+EXPECT_TRUE(matcher.Matches(CandidateFromTypeName("struct Name")));
+EXPECT_TRUE(matcher.Matches(CandidateFromTypeName("union Name")));
+EXPECT_TRUE(matcher.Matches(CandidateFromTypeName("enum Name")));
+EXPECT_TRUE(matcher.Matches(CandidateFromTypeName("Name")));
+
+EXPECT_FALSE(matcher.Matches(CandidateFromTypeName("Name ")));
+EXPECT_FALS

[Lldb-commits] [PATCH] D135648: [lldb] Add matching based on Python callbacks for data formatters.

2022-10-14 Thread Jorge Gorbe Moya via Phabricator via lldb-commits
jgorbe marked 2 inline comments as done.
jgorbe added inline comments.



Comment at: lldb/source/Commands/CommandObjectType.cpp:2306
   if (type == eRegularSynth) {
-if (category->AnyMatches(type_name, eFormatCategoryItemFilter, false)) {
+// TODO: Get a suitable type object for type_name so we can create a
+// complete FormattersMatchCandidate.

labath wrote:
> Is that even possible? Like, this command can be run before any 
> binaries/debug info is loaded, right? Maybe we should just call that WAI?
You're right. I hadn't though of the case where we have no target yet. Changed 
the comments in this file and a similar one in 
lldb/include/lldb/DataFormatters/FormattersContainer.h to remove the TODO and 
indicate this is WAI. Thanks!



Comment at: 
lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp:2159-2167
+  // Default to false in case of error while running the callback.
+  bool result = false;
+  {
+Locker py_lock(this,
+   Locker::AcquireLock | Locker::InitSession | 
Locker::NoSTDIN);
+result = LLDBSwigPythonFormatterCallbackFunction(
+python_function_name, m_dictionary_name.c_str(), type_impl_sp);

labath wrote:
> I think we already had code like this somewhere, but these two versions are 
> completely equivalent.
Done!


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

https://reviews.llvm.org/D135648

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


[Lldb-commits] [PATCH] D127695: [clang] Implement Template Specialization Resugaring

2022-10-14 Thread John McCall via Phabricator via lldb-commits
rjmccall added a comment.

What is this work about?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127695

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


[Lldb-commits] [PATCH] D136011: [lldb] Don't check environment default char signedness when creating clang type for "char"

2022-10-14 Thread Arthur Eubanks via Phabricator via lldb-commits
aeubanks created this revision.
Herald added a project: All.
aeubanks requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

With -f(un)signed-char, the die corresponding to "char" may be the wrong 
DW_ATE_(un)signed_char. Ultimately we can determine whether a type is the 
unspecified signedness char by looking if its name is "char" (as opposed to 
"signed char"/"unsigned char").

Fixes #23443


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D136011

Files:
  lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
  lldb/test/API/commands/expression/char/TestExprsChar.py
  lldb/test/API/commands/expression/char/main.cpp


Index: lldb/test/API/commands/expression/char/main.cpp
===
--- lldb/test/API/commands/expression/char/main.cpp
+++ lldb/test/API/commands/expression/char/main.cpp
@@ -1,5 +1,7 @@
 #include 
 
+char g = 0;
+
 int foo(char c) { return 1; }
 int foo(signed char c) { return 2; }
 int foo(unsigned char c) { return 3; }
Index: lldb/test/API/commands/expression/char/TestExprsChar.py
===
--- lldb/test/API/commands/expression/char/TestExprsChar.py
+++ lldb/test/API/commands/expression/char/TestExprsChar.py
@@ -14,30 +14,13 @@
 self.expect_expr("foo(c)", result_value="1")
 self.expect_expr("foo(sc)", result_value="2")
 self.expect_expr("foo(uc)", result_value="3")
+self.expect_expr("g", result_type="char")
 
 def test_default_char(self):
 self.do_test()
 
-@skipIf(oslist=["linux"], archs=["aarch64", "arm"], 
bugnumber="llvm.org/pr23069")
-@expectedFailureAll(
-archs=[
-"powerpc64le",
-"s390x"],
-bugnumber="llvm.org/pr23069")
 def test_signed_char(self):
 self.do_test(dictionary={'CFLAGS_EXTRAS': '-fsigned-char'})
 
-@expectedFailureAll(
-archs=[
-"i[3-6]86",
-"x86_64",
-"arm64",
-'arm64e',
-'armv7',
-'armv7k',
-'arm64_32'],
-bugnumber="llvm.org/pr23069, ")
-@expectedFailureAll(triple='mips*', bugnumber="llvm.org/pr23069")
-@expectedFailureAll(oslist=['windows'], archs=['aarch64'], 
bugnumber="llvm.org/pr23069")
 def test_unsigned_char(self):
 self.do_test(dictionary={'CFLAGS_EXTRAS': '-funsigned-char'})
Index: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
===
--- lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -1063,7 +1063,7 @@
 break;
 
   case DW_ATE_signed_char:
-if (ast.getLangOpts().CharIsSigned && type_name == "char") {
+if (type_name == "char") {
   if (QualTypeMatchesBitSize(bit_size, ast, ast.CharTy))
 return GetType(ast.CharTy);
 }
@@ -1115,7 +1115,7 @@
 break;
 
   case DW_ATE_unsigned_char:
-if (!ast.getLangOpts().CharIsSigned && type_name == "char") {
+if (type_name == "char") {
   if (QualTypeMatchesBitSize(bit_size, ast, ast.CharTy))
 return GetType(ast.CharTy);
 }


Index: lldb/test/API/commands/expression/char/main.cpp
===
--- lldb/test/API/commands/expression/char/main.cpp
+++ lldb/test/API/commands/expression/char/main.cpp
@@ -1,5 +1,7 @@
 #include 
 
+char g = 0;
+
 int foo(char c) { return 1; }
 int foo(signed char c) { return 2; }
 int foo(unsigned char c) { return 3; }
Index: lldb/test/API/commands/expression/char/TestExprsChar.py
===
--- lldb/test/API/commands/expression/char/TestExprsChar.py
+++ lldb/test/API/commands/expression/char/TestExprsChar.py
@@ -14,30 +14,13 @@
 self.expect_expr("foo(c)", result_value="1")
 self.expect_expr("foo(sc)", result_value="2")
 self.expect_expr("foo(uc)", result_value="3")
+self.expect_expr("g", result_type="char")
 
 def test_default_char(self):
 self.do_test()
 
-@skipIf(oslist=["linux"], archs=["aarch64", "arm"], bugnumber="llvm.org/pr23069")
-@expectedFailureAll(
-archs=[
-"powerpc64le",
-"s390x"],
-bugnumber="llvm.org/pr23069")
 def test_signed_char(self):
 self.do_test(dictionary={'CFLAGS_EXTRAS': '-fsigned-char'})
 
-@expectedFailureAll(
-archs=[
-"i[3-6]86",
-"x86_64",
-"arm64",
-'arm64e',
-'armv7',
-'armv7k',
-'arm64_32'],
-bugnumber="llvm.org/pr23069, ")
-@expectedFailureAll(triple='mips*', bugnumber="llvm.org/pr23069")
-@expectedFailureAll(oslist=['windows'], archs=['aarch64'], bugnumber="llvm.org/pr23069")
 def test_unsigned_char(self):
 self.do_test(dictionar

[Lldb-commits] [PATCH] D136011: [lldb] Don't check environment default char signedness when creating clang type for "char"

2022-10-14 Thread Arthur Eubanks via Phabricator via lldb-commits
aeubanks added a comment.

not 100% sure this is the right fix


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136011

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


[Lldb-commits] [PATCH] D127695: [clang] Implement Template Specialization Resugaring

2022-10-14 Thread Matheus Izvekov via Phabricator via lldb-commits
mizvekov added a comment.

In D127695#3860237 , @rjmccall wrote:

> What is this work about?

I have written more about it in this discourse thread: 
https://discourse.llvm.org/t/rfc-improving-diagnostics-with-template-specialization-resugaring/64294


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127695

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


[Lldb-commits] [PATCH] D127695: [clang] Implement Template Specialization Resugaring

2022-10-14 Thread John McCall via Phabricator via lldb-commits
rjmccall added a comment.

I see, thank you. I know you've marked this a WIP, but it's always good to 
describe these things in the patch description; that's what it's for.

This is quite exciting!  Thank you for looking into this.

I think you could usefully be much more incremental here.  Obviously the 
resugaring transform is one big piece, but it's likely that you could usefully 
roll out the applications of it one at a time and see some incremental 
improvements, and it would be a lot easier for us to then review the changes 
you're needing to make to see why you're making them.  That shouldn't make it 
any more difficult to measure holistic things like the overall compile-time 
impact of resugaring — it's all filtering through a single place, so you ought 
to be able to easily disable it there.  You could also disable it by default by 
leaving it behind a `-cc1` flag that you only enable in your tests.  That would 
let you actually land the work incrementally, which I would have no objection 
to.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127695

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