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

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

47

Add newline at end of file

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

27

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.

packages/Python/lldbsuite/test/functionalities/expression_path/Makefile

+LEVEL = ../../make
+
+CXX_SOURCES := main.cpp
+
+include $(LEVEL)/Makefile.rules

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

+"""
+Test the Expression Path.
+"""
+
+from __future__ import print_function
+
+
+
+import os, time
+import re
+import lldb
+from lldbsuite.test.lldbtest import *
+import lldbsuite.test.lldbutil as lldbutil
+
+class ExpressionPathTestCase(TestBase):
+
+    mydir = TestBase.compute_mydir(__file__)
+
+    def setUp(self):
+        # Call super's setUp().
+        TestBase.setUp(self)
+        # Find the line number to break inside main().
+        self.line = line_number('main.cpp', '// Set break point at this line.')
+
+    def test_expression_path(self):
+        """Test the GetExpressionPath() function of ValueObject using 'frame variable --flat' command."""
+        self.build()
+        exe = os.path.join(os.getcwd(), "a.out")
+        self.runCmd("file " + exe, CURRENT_EXECUTABLE_SET)
+
+        # Break in main() after the variables are assigned values.
+        lldbutil.run_break_set_by_file_and_line (self, "main.cpp", self.line, num_expected_locations=1, loc_exact=True)
+
+        self.runCmd("run", RUN_SUCCEEDED)
+
+        # The stop reason of the thread should be breakpoint.
+        self.expect("thread list", STOPPED_DUE_TO_BREAKPOINT,
+            substrs = ['stopped', 'stop reason = breakpoint'])
+
+        # The breakpoint should have a hit count of 1.
+        self.expect("breakpoint list -f", BREAKPOINT_HIT_ONCE,
+            substrs = [' resolved, hit count = 1'])
+      
+        self.expect('frame variable s --flat',
+                    substrs = ['s.x = 1',
+                               's.y = 2'])
 No newline at end of file

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

+//===-- main.cpp ------------------------------------------------*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+struct S
+{
+  union 
+  {
+    unsigned r[2];
+    struct  {
+      int x;
+      int y;
+    };
+  };
+};
+
+int main()
+{
+  struct S s = { 1, 2 };
+  return 0; // Set break point at this line.
+}
 No newline at end of file

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("->");
                     }