Handle lookup of names identifying both a variable and a type

[uweigand]

Summary

In C++ code, a variable can have the same name as a type, e.g. like

int C;
struct C {

static int a;

};

When evaluating an expression like "C::a" by the LLDB parser, clang
will call back into the external source asking for decls for the
identifier "C". Currently, LLDB will only return the decl for the
variable C. This in turn will cause clang to fail with an error.

(This happens for me with the lang/cpp/scope/TestCppScope.py test
case, due to a static variable C in one of the libm.so objects.)

Instead, it seems clang expects the external data source to return
*both* the variable and the type decl. It will then choose the
appropriate one to use based on its current parsing context.

This patch changes ClangExpressionDeclMap::FindExternalVisibleDecls
to always call ClangASTSource::FindExternalVisibleDecls to possibly
identify types, even if we already found a variable of that name.

[labath]

I think having a self-contained test for this issue (one which does not rely on internals of some system library) would be extremely valuable. Could you add one?

[uweigand]

In D18976#399633, @labath wrote:

I think having a self-contained test for this issue (one which does not rely on internals of some system library) would be extremely valuable. Could you add one?

Well, a simple way would be to just add something like

int A, B, C;

to the lang/cpp/scope/main.cpp test case. Would that be OK or do you prefer a completely separate test?

[labath]

I think adding it to the extending the existing test is fine, given that it's a fairly small one. (In case of large tests, it's better to make a new one, so it can be debugged/xfailed independently). Maybe just add int C so that we test both the situation when we have a variable, and when we don't?

[uweigand]

Add test case.

[uweigand]

Ping? This fixes the last remaining test suite failure on s390x. I'd like to get a clean test suite ...

Patch updated to current sources.

Added jingham since you touched that code recently.

packages/Python/lldbsuite/test/lang/cpp/scope/TestCppScope.py

def test_with_run_command(self):
         # Test result for scopes of variables
 
         global_variables = frame.GetVariables(True, True, True, False)
         global_variables_assert = {
             'A::a': 1111,
             'B::a': 2222,
             'C::a': 3333,
             '::a': 4444,
-            'a': 4444
+            'a': 4444,
+            '::C': 5555,
+            'C': 5555
         }
 
-        self.assertTrue(global_variables.GetSize() == 4, "target variable returns all variables")
+        self.assertTrue(global_variables.GetSize() == 5, "target variable returns all variables")
         for variable in global_variables:
             name = variable.GetName()
             self.assertTrue(name in global_variables_assert, "target variable returns wrong variable " + name)
 
         for name in global_variables_assert:
             value = frame.EvaluateExpression(name)
             assert_value = global_variables_assert[name]
             self.assertTrue(value.IsValid() and value.GetValueAsSigned() == assert_value, name + " = " + str(assert_value))

packages/Python/lldbsuite/test/lang/cpp/scope/main.cpp

struct C {
     static int a;
 };
 
 int A::a = 1111;
 int B::a = 2222;
 int C::a = 3333;
 int a = 4444;
 
+// Verify that LLDB is able to parse "C::a" even when "C" exists both as a
+// type name and a variable name at the same time.
+int C = 5555;
+
 int main() // break here
 {
     return 0;
 }

source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp

else if (isa<TranslationUnitDecl>(context.m_decl_context))
             log->Printf("  CEDM::FEVD[%u] Searching the root namespace", current_id);
 
         FindExternalVisibleDecls(context,
                                  lldb::ModuleSP(),
                                  namespace_decl,
                                  current_id);
     }
 
-    if (!context.m_found.variable && !context.m_found.local_vars_nsp)
-        ClangASTSource::FindExternalVisibleDecls(context);
+    // Always call into ClangASTSource::FindExternalVisibleDecls, even if we already found
+    // some decls above.  It might be that Clang is looking for a type, but we have found
+    // a variable of the same name instead.  Let ClangASTSource add the type to the result
+    // list as well; Clang will filter out the decl it is actually interested in.
 }
 
 void
 ClangExpressionDeclMap::FindExternalVisibleDecls (NameSearchContext &context,
                                                   lldb::ModuleSP module_sp,
                                                   CompilerDeclContext &namespace_decl,
                                                   unsigned int current_id)
 {