Skip to content

Conversation

Michael137
Copy link
Member

@Michael137 Michael137 commented Jul 7, 2025

A default-constructed variant has a valid index (being the first element of the variant). The only case where the index is variant_npos is when the variant is "valueless", which according to cppreference only happens when an exception is thrown during assignment to the variant.

I adjusted the test to test that scenario, and the formatter seems to work.

Unblocks #147253

Instead of using the byte-size to make a guess at what the
`std::variant_npos` value is, just look it up in debug-info.

Unblocks llvm#147253
@Michael137 Michael137 requested a review from labath July 7, 2025 11:46
@Michael137 Michael137 requested a review from JDevlieghere as a code owner July 7, 2025 11:46
@llvmbot llvmbot added the lldb label Jul 7, 2025
@llvmbot
Copy link
Member

llvmbot commented Jul 7, 2025

@llvm/pr-subscribers-lldb

Author: Michael Buch (Michael137)

Changes

Instead of using the byte-size to make a guess at what the std::variant_npos value is, just look it up in debug-info.

Unblocks #147253


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

3 Files Affected:

  • (modified) lldb/examples/synthetic/gnu_libstdcpp.py (+4-9)
  • (modified) lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/variant/TestDataFormatterLibStdcxxVariant.py (-5)
  • (modified) lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/variant/main.cpp (+1)
diff --git a/lldb/examples/synthetic/gnu_libstdcpp.py b/lldb/examples/synthetic/gnu_libstdcpp.py
index 20b9488af5597..96b11322db775 100644
--- a/lldb/examples/synthetic/gnu_libstdcpp.py
+++ b/lldb/examples/synthetic/gnu_libstdcpp.py
@@ -889,17 +889,12 @@ def VariantSummaryProvider(valobj, dict):
     if not (index_obj and index_obj.IsValid() and data_obj and data_obj.IsValid()):
         return "<Can't find _M_index or _M_u>"
 
-    def get_variant_npos_value(index_byte_size):
-        if index_byte_size == 1:
-            return 0xFF
-        elif index_byte_size == 2:
-            return 0xFFFF
-        else:
-            return 0xFFFFFFFF
+    npos = valobj.GetTarget().FindFirstGlobalVariable("std::variant_npos")
+    if not npos:
+        return "<Can't find std::variant_npos sentinel>"
 
-    npos_value = get_variant_npos_value(index_obj.GetByteSize())
     index = index_obj.GetValueAsUnsigned(0)
-    if index == npos_value:
+    if index == npos.GetValueAsUnsigned(0):
         return " No Value"
 
     # Strip references and typedefs.
diff --git a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/variant/TestDataFormatterLibStdcxxVariant.py b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/variant/TestDataFormatterLibStdcxxVariant.py
index 394e221809f7c..c3325c9e73cb9 100644
--- a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/variant/TestDataFormatterLibStdcxxVariant.py
+++ b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/variant/TestDataFormatterLibStdcxxVariant.py
@@ -2,7 +2,6 @@
 Test lldb data formatter for LibStdC++ std::variant.
 """
 
-
 import lldb
 from lldbsuite.test.decorators import *
 from lldbsuite.test.lldbtest import *
@@ -62,9 +61,6 @@ def test_with_run_command(self):
             "frame variable v3",
             substrs=["v3 =  Active Type = char  {", "Value = 'A'", "}"],
         )
-        """
-        TODO: temporarily disable No Value tests as they seem to fail on ubuntu/debian
-        bots. Pending reproduce and investigation.
 
         self.expect("frame variable v_no_value", substrs=["v_no_value =  No Value"])
 
@@ -72,7 +68,6 @@ def test_with_run_command(self):
             "frame variable v_many_types_no_value",
             substrs=["v_many_types_no_value =  No Value"],
         )
-        """
 
     @add_test_categories(["libstdcxx"])
     def test_invalid_variant_index(self):
diff --git a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/variant/main.cpp b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/variant/main.cpp
index 36e0f74f831f8..acb07d5700b8a 100644
--- a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/variant/main.cpp
+++ b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/variant/main.cpp
@@ -49,6 +49,7 @@ int main() {
   v1 = 12; // v contains int
   v1_typedef = v1;
   v_v1 = v1;
+  v1_typedef = v1;
   int i = std::get<int>(v1);
   printf("%d\n", i); // break here
 

Copy link
Collaborator

@labath labath left a comment

Choose a reason for hiding this comment

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

Looking up global variables is not completely ideal (can be slow and look up unrelated things). Would index_obj.GetValueAsSigned() == -1 work by any chance?

@Michael137
Copy link
Member Author

Looking up global variables is not completely ideal (can be slow and look up unrelated things). Would index_obj.GetValueAsSigned() == -1 work by any chance?

Actually after some more investigating it turns out the test was just flawed. A default-constructed variant has a valid index (being the first element of the variant). The only case where the index is variant_npos is when the variant is "valueless", which according to cppreference only happens when an exception is thrown during assignment to the variant.

I adjusted the test to test that scenario, and the formatter seems to work.

@Michael137 Michael137 changed the title [lldb][test] Fix libstdc++ std::variant formatter for empty variant [lldb][test] Fix libstdc++ std::variant formatter tests for valueless variants Jul 7, 2025
Copy link
Collaborator

@labath labath left a comment

Choose a reason for hiding this comment

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

Cool. Thanks for looking into this.

@Michael137 Michael137 merged commit 9ebe6f9 into llvm:main Jul 7, 2025
9 checks passed
@Michael137 Michael137 deleted the lldb/libstdcpp-empty-variant-format branch July 7, 2025 13:48
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