Skip to content

[lldb] Support "dereferencing" std::optional in frame var #107077

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Sep 3, 2024
Merged

Conversation

labath
Copy link
Collaborator

@labath labath commented Sep 3, 2024

No description provided.

@labath labath requested a review from Michael137 September 3, 2024 10:21
@labath labath requested a review from JDevlieghere as a code owner September 3, 2024 10:21
@llvmbot llvmbot added the lldb label Sep 3, 2024
@llvmbot
Copy link
Member

llvmbot commented Sep 3, 2024

@llvm/pr-subscribers-lldb

Author: Pavel Labath (labath)

Changes

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

3 Files Affected:

  • (modified) lldb/source/Plugins/Language/CPlusPlus/GenericOptional.cpp (+2)
  • (modified) lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/optional/TestDataFormatterGenericOptional.py (+12)
  • (modified) lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/optional/main.cpp (+5)
diff --git a/lldb/source/Plugins/Language/CPlusPlus/GenericOptional.cpp b/lldb/source/Plugins/Language/CPlusPlus/GenericOptional.cpp
index 23756de7f1e66e..a8a7c16de5e867 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/GenericOptional.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/GenericOptional.cpp
@@ -37,6 +37,8 @@ class GenericOptionalFrontend : public SyntheticChildrenFrontEnd {
   GenericOptionalFrontend(ValueObject &valobj, StdLib stdlib);
 
   size_t GetIndexOfChildWithName(ConstString name) override {
+    if (name == "$$dereference$$")
+      return 0;
     return formatters::ExtractIndexFromString(name.GetCString());
   }
 
diff --git a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/optional/TestDataFormatterGenericOptional.py b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/optional/TestDataFormatterGenericOptional.py
index f6507294bfb2ac..7dc656a7ae225c 100644
--- a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/optional/TestDataFormatterGenericOptional.py
+++ b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/optional/TestDataFormatterGenericOptional.py
@@ -79,6 +79,18 @@ def cleanup():
             ],
         )
 
+        self.expect_var_path("*number_engaged", value="42")
+        self.expect_var_path("*x", children=[ValueCheck(name="x", value="42")])
+        self.expect_var_path("x->x", value="42")
+
+        # The error message could use some improvement, but at least we can
+        # check we don't crash.
+        self.expect(
+            "frame variable *number_not_engaged",
+            error=True,
+            substrs=["not a pointer or reference type"],
+        )
+
     @add_test_categories(["libc++"])
     ## Clang 7.0 is the oldest Clang that can reliably parse newer libc++ versions
     ## with -std=c++17.
diff --git a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/optional/main.cpp b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/optional/main.cpp
index 8f30df4626636c..18a95b9cc246c6 100644
--- a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/optional/main.cpp
+++ b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/optional/main.cpp
@@ -12,6 +12,10 @@
 #define HAVE_OPTIONAL 0
 #endif
 
+struct X {
+  int x;
+};
+
 int main() {
   bool has_optional = HAVE_OPTIONAL;
 
@@ -25,6 +29,7 @@ int main() {
 
   optional_int number_not_engaged;
   optional_int number_engaged = 42;
+  std::optional<X> x = X{42};
 
   printf("%d\n", *number_engaged);
 

Copy link
Member

@Michael137 Michael137 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, was wishing for this just yesterday

self.expect_var_path("x->x", value="42")

# The error message could use some improvement, but at least we can
# check we don't crash.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yea would be nice to bubble a nicer error up somehow. Looks like ValueObject currently just drops any error set by the dereferenced ValueObject (we don't set one in the formatter currently anyway). But not urgent

@labath labath merged commit a5f03b4 into llvm:main Sep 3, 2024
9 checks passed
@labath labath deleted the opt branch September 3, 2024 11:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants