[LLDB] Display artificial promise and coro_frame variables. #71928

hokein · 2023-11-10T11:46:06Z

See the discussion in #69309.

hokein · 2023-11-10T11:48:53Z

Is there a way to run the tests under lldb/test/API/ directory? ninja check-lldb doesn't seem to run these tests on my linux machine.

 ./bin/llvm-lit -sv  <llvm-project-path>/lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/coroutine_handle/    

Testing Time: 0.02s

Total Discovered Tests: 1
  Unsupported: 1 (100.00%)

llvmbot · 2023-11-10T11:56:43Z

@llvm/pr-subscribers-lldb

Author: Haojian Wu (hokein)

Changes

See the discussion in #69309.

Full diff: https://github.com/llvm/llvm-project/pull/71928.diff

3 Files Affected:

(modified) lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp (+5-1)
(modified) lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/coroutine_handle/TestCoroutineHandle.py (+12-1)
(modified) lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/coroutine_handle/main.cpp (+1-1)

diff --git a/lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp b/lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp
index c2488eaa9f5b50d..b5dfd07bdff2453 100644
--- a/lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp
+++ b/lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp
@@ -41,7 +41,11 @@ CPPLanguageRuntime::CPPLanguageRuntime(Process *process)
     : LanguageRuntime(process) {}
 
 bool CPPLanguageRuntime::IsAllowedRuntimeValue(ConstString name) {
-  return name == g_this;
+  // FIXME: use a list when the list grows more.
+  return name == g_this ||
+  // Artificial coroutine-related variables emitted by clang.
+         name == ConstString("__promise") ||
+         name == ConstString("__coro_frame");
 }
 
 bool CPPLanguageRuntime::GetObjectDescription(Stream &str,
diff --git a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/coroutine_handle/TestCoroutineHandle.py b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/coroutine_handle/TestCoroutineHandle.py
index 42ee32f9ccca58d..bcb1da6dc3838c8 100644
--- a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/coroutine_handle/TestCoroutineHandle.py
+++ b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/coroutine_handle/TestCoroutineHandle.py
@@ -78,8 +78,19 @@ def do_test(self, stdlib_type):
                 ],
             )
 
-        # Run until after the `co_yield`
         process = self.process()
+
+        # Break at a coroutine body
+        lldbutil.continue_to_source_breakpoint(
+          self, process, "// Break at co_yield", lldb.SBFileSpec("main.cpp", False)
+        )
+        # Expect artificial variables to be displayed
+        self.expect(
+          "frame variable",
+          substrs=['__promise', '__coro_frame']
+        )
+
+        # Run until after the `co_yield`
         lldbutil.continue_to_source_breakpoint(
             self, process, "// Break after co_yield", lldb.SBFileSpec("main.cpp", False)
         )
diff --git a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/coroutine_handle/main.cpp b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/coroutine_handle/main.cpp
index 8cb81c3bc9f4c4e..4523b7c7baf80aa 100644
--- a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/coroutine_handle/main.cpp
+++ b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/coroutine_handle/main.cpp
@@ -33,7 +33,7 @@ struct int_generator {
   ~int_generator() { hdl.destroy(); }
 };
 
-int_generator my_generator_func() { co_yield 42; }
+int_generator my_generator_func() { co_yield 42; } // Break at co_yield
 
 // This is an empty function which we call just so the debugger has
 // a place to reliably set a breakpoint on.

github-actions · 2023-11-10T11:58:14Z

✅ With the latest revision this PR passed the Python code formatter.

github-actions · 2023-11-10T11:58:17Z

✅ With the latest revision this PR passed the C/C++ code formatter.

Michael137 · 2023-11-10T12:16:01Z

Is there a way to run the tests under lldb/test/API/ directory? ninja check-lldb doesn't seem to run these tests on my linux machine.
 ./bin/llvm-lit -sv  <llvm-project-path>/lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/coroutine_handle/    

Testing Time: 0.02s

Total Discovered Tests: 1
  Unsupported: 1 (100.00%)

Did you configure your build with LLDB_INCLUDE_TESTS=ON and LLDB_INCLUDE_PYTHON=ON? Those CMake variables are necessary to get the API tests to run

Michael137 · 2023-11-10T12:16:55Z

lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp

@@ -41,7 +41,11 @@ CPPLanguageRuntime::CPPLanguageRuntime(Process *process)
    : LanguageRuntime(process) {}

 bool CPPLanguageRuntime::IsAllowedRuntimeValue(ConstString name) {
-  return name == g_this;
+  // FIXME: use a list when the list grows more.


Don't think we need this FIXME

Sure, removed.

hokein · 2023-11-10T14:16:54Z

Did you configure your build with LLDB_INCLUDE_TESTS=ON and LLDB_INCLUDE_PYTHON=ON? Those CMake variables are necessary to get the API tests to run

Thanks! I was able to run the test locally now with -DLLDB_INCLUDE_TESTS=On, -DLLDB_ENABLE_PYTHON=On, and all deps installed (sudo apt-get install build-essential swig python3-dev libedit-dev).

lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp

...ionalities/data-formatter/data-formatter-stl/generic/coroutine_handle/TestCoroutineHandle.py

See the discussion in llvm#69309.

hokein

Thanks for the comments.

...ionalities/data-formatter/data-formatter-stl/generic/coroutine_handle/TestCoroutineHandle.py

lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp

Michael137

LGTM (once bots are happy). I think you may need to run git clang-format or python's black formatter

hokein · 2023-11-14T07:57:45Z

Thanks for the review.

…#71928) See the discussion in llvm#69309.

hokein requested a review from Michael137 November 10, 2023 11:46

hokein marked this pull request as ready for review November 10, 2023 11:56

hokein requested a review from JDevlieghere as a code owner November 10, 2023 11:56

llvmbot added the lldb label Nov 10, 2023

Michael137 reviewed Nov 10, 2023

View reviewed changes

JDevlieghere reviewed Nov 10, 2023

View reviewed changes

lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp Outdated Show resolved Hide resolved

Michael137 reviewed Nov 11, 2023

View reviewed changes

...ionalities/data-formatter/data-formatter-stl/generic/coroutine_handle/TestCoroutineHandle.py Outdated Show resolved Hide resolved

hokein added 3 commits November 13, 2023 09:22

[LLDB] Display artificial __promise and __coro_frame variables.

08c3b1a

See the discussion in llvm#69309.

Remove an unneeded FIXME.

f684e19

Use static constants for builtin variables.

43ab602

hokein force-pushed the lldb-display-coroutines branch from d67d907 to 43ab602 Compare November 13, 2023 08:31

hokein commented Nov 13, 2023

View reviewed changes

...ionalities/data-formatter/data-formatter-stl/generic/coroutine_handle/TestCoroutineHandle.py Outdated Show resolved Hide resolved

lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp Outdated Show resolved Hide resolved

Break on the coroutine function, per a review comment.

d77afee

Michael137 approved these changes Nov 13, 2023

View reviewed changes

Fix format

3bdefad

hokein merged commit b837361 into llvm:main Nov 14, 2023

hokein deleted the lldb-display-coroutines branch November 14, 2023 08:06

zahiraam pushed a commit to zahiraam/llvm-project that referenced this pull request Nov 20, 2023

[LLDB] Display artificial __promise and __coro_frame variables. (llvm…

8a01290

…#71928) See the discussion in llvm#69309.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[LLDB] Display artificial promise and coro_frame variables. #71928

[LLDB] Display artificial promise and coro_frame variables. #71928

Uh oh!

hokein commented Nov 10, 2023

Uh oh!

hokein commented Nov 10, 2023

Uh oh!

llvmbot commented Nov 10, 2023

Uh oh!

github-actions bot commented Nov 10, 2023 •

edited

Loading

Uh oh!

github-actions bot commented Nov 10, 2023 •

edited

Loading

Uh oh!

Michael137 commented Nov 10, 2023

Uh oh!

Michael137 Nov 10, 2023

Uh oh!

hokein Nov 10, 2023

Uh oh!

hokein commented Nov 10, 2023

Uh oh!

Uh oh!

Uh oh!

hokein left a comment

Uh oh!

Uh oh!

Uh oh!

Michael137 left a comment •

edited

Loading

Uh oh!

hokein commented Nov 14, 2023

Uh oh!

Uh oh!

[LLDB] Display artificial __promise and __coro_frame variables. #71928

[LLDB] Display artificial __promise and __coro_frame variables. #71928

Uh oh!

Conversation

hokein commented Nov 10, 2023

Uh oh!

hokein commented Nov 10, 2023

Uh oh!

llvmbot commented Nov 10, 2023

Uh oh!

github-actions bot commented Nov 10, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Nov 10, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Michael137 commented Nov 10, 2023

Uh oh!

Michael137 Nov 10, 2023

Choose a reason for hiding this comment

Uh oh!

hokein Nov 10, 2023

Choose a reason for hiding this comment

Uh oh!

hokein commented Nov 10, 2023

Uh oh!

Uh oh!

Uh oh!

hokein left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Michael137 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hokein commented Nov 14, 2023

Uh oh!

Uh oh!

[LLDB] Display artificial promise and coro_frame variables. #71928

[LLDB] Display artificial promise and coro_frame variables. #71928

github-actions bot commented Nov 10, 2023 •

edited

Loading

github-actions bot commented Nov 10, 2023 •

edited

Loading

Michael137 left a comment •

edited

Loading