Fixed ValueObject::GetExpressionPath() for paths including anonymous struct/union

[mamai]

Summary

When the parent of an expression is anonymous, skip adding '.' or '->' before the expression name.

[mamai]

Fixed test header, removed useless include.

[jingham]

The source change looks fine, but Greg's the owner of this code so I'll let him say yes for sure.

But don't call the test case "ExpressionPath". The expression parser is different from frame variable. I understand that your intent was "the expression you feed to frame variable" but since we have an "expression" command, and the expressions you feed to it are different from those you feed to "frame variable" overloading the word is only going to cause confusion.

[mamai]

Sure. Would you have a suggestion of how to call it? Maybe something like variable_flat since it is the command used? Also, is it the right place to add it in functionalities or should it be elsewhere?

[jingham]

There's actually already a test for handling C anonymous unions in lang/c/anonymous. That tests the expression parser and some direct SBValue queries at present. It would be appropriate to add your test as another test case in here.

[jingham]

The part of frame variable you are testing here - namely the parsing of these "frame variable" paths, can also be directly accessed with SBValue::GetValueForExpressionPath (aargh, shouldn't have called that "expression" should we? Too late to change now.) Anyway, you can also use that to test more directly if you want.

[ldrumm]

Hi all

This differential may also be somewhat relevant / of interest, although it deals only with typedefs of untagged struct types.

Best

Luke

[clayborg]

See inlined comments.

packages/Python/lldbsuite/test/functionalities/expression_path/TestExpressionPath.py

44–46 ↗

(On Diff #50172)

Shouldn't we also see "s.r[0] = 1" and "s.r[1] = 2" in response to "frame variable s --flat"?

47 ↗

(On Diff #50172)

Add newline at end of file

packages/Python/lldbsuite/test/functionalities/expression_path/main.cpp

27 ↗

(On Diff #50172)

Add newline at end of file

[mamai]

Replaced the test by a new case in lang/c/anonymous test, as suggested by Jim. Is this correct?

[clayborg]

Looks fine to me.

[mamai]

Thanks! Could someone commit it for me? I don't have commit access.

lldb/trunk/packages/Python/lldbsuite/test/lang/c/anonymous/TestAnonymous.py

def test_child_by_name(self):
         if not elem_a.IsValid():
             self.fail ("Failed to get the element a in n")
 
         error = lldb.SBError()
         value = elem_a.GetValueAsSigned(error, 1000)
         if not error.Success() or value != 0:
             self.fail ("failed to get the correct value for element a in n")
 
+    def test_nest_flat(self):
+        self.build()
+        self.common_setup(self.line2)
+
+        # These should display correctly.
+        self.expect('frame variable n --flat',
+                    substrs = ['n.a = 0',
+                               'n.b = 2',
+                               'n.foo.c = 0',
+                               'n.foo.d = 4'])
+
     def setUp(self):
         # Call super's setUp().
         TestBase.setUp(self)
         # Find the line numbers to break in main.c.
         self.source = 'main.c'
         self.line0 = line_number(self.source, '// Set breakpoint 0 here.')
         self.line1 = line_number(self.source, '// Set breakpoint 1 here.')
         self.line2 = line_number(self.source, '// Set breakpoint 2 here.')

lldb/trunk/source/Core/ValueObject.cpp

ValueObject::GetExpressionPath (Stream &s, bool qualify_cxx_base_classes, GetExpressionPathFormat epformat)
     if (m_is_array_item_for_pointer && epformat == eGetExpressionPathFormatHonorPointers)
         s.PutCString(m_name.AsCString());
             
     if (!IsBaseClass())
     {
         if (!is_deref_of_parent)
         {
             ValueObject *non_base_class_parent = GetNonBaseClassParent();
-            if (non_base_class_parent)
+            if (non_base_class_parent && !non_base_class_parent->GetName().IsEmpty())
             {
                 CompilerType non_base_class_parent_compiler_type = non_base_class_parent->GetCompilerType();
                 if (non_base_class_parent_compiler_type)
                 {
                     if (parent && parent->IsDereferenceOfParent() && epformat == eGetExpressionPathFormatHonorPointers)
                     {
                         s.PutCString("->");
                     }