[LLDB] Optimize identifier lookup in DIL

lldb/include/lldb/ValueObject/DILEval.h

@@ -25,8 +25,7 @@ namespace lldb_private::dil {
 /// evaluating).
 lldb::ValueObjectSP LookupIdentifier(llvm::StringRef name_ref,
                                      std::shared_ptr<StackFrame> frame_sp,
-                                     lldb::DynamicValueType use_dynamic,
-                                     CompilerType *scope_ptr = nullptr);
+                                     lldb::DynamicValueType use_dynamic);
 
 /// Given the name of an identifier, check to see if it matches the name of a
 /// global variable. If so, find the ValueObject for that global variable, and
@@ -35,8 +34,7 @@ lldb::ValueObjectSP LookupIdentifier(llvm::StringRef name_ref,
 lldb::ValueObjectSP LookupGlobalIdentifier(llvm::StringRef name_ref,
                                            std::shared_ptr<StackFrame> frame_sp,
                                            lldb::TargetSP target_sp,
-                                           lldb::DynamicValueType use_dynamic,
-                                           CompilerType *scope_ptr = nullptr);
+                                           lldb::DynamicValueType use_dynamic);
 
 class Interpreter : Visitor {
 public:

lldb/source/ValueObject/DILEval.cpp

@@ -7,6 +7,7 @@
 //===----------------------------------------------------------------------===//
 
 #include "lldb/ValueObject/DILEval.h"
+#include "lldb/Symbol/CompileUnit.h"
 #include "lldb/Symbol/VariableList.h"
 #include "lldb/Target/RegisterContext.h"
 #include "lldb/ValueObject/DILAST.h"
@@ -18,109 +19,50 @@
 
 namespace lldb_private::dil {
 
-static lldb::ValueObjectSP LookupStaticIdentifier(
-    VariableList &variable_list, std::shared_ptr<StackFrame> exe_scope,
-    llvm::StringRef name_ref, llvm::StringRef unqualified_name) {
-  // First look for an exact match to the (possibly) qualified name.
-  for (const lldb::VariableSP &var_sp : variable_list) {
-    lldb::ValueObjectSP valobj_sp(
-        ValueObjectVariable::Create(exe_scope.get(), var_sp));
-    if (valobj_sp && valobj_sp->GetVariable() &&
-        (valobj_sp->GetVariable()->NameMatches(ConstString(name_ref))))
-      return valobj_sp;
-  }
-
-  // If the qualified name is the same as the unqualfied name, there's nothing
-  // more to be done.
-  if (name_ref == unqualified_name)
-    return nullptr;
-
-  // We didn't match the qualified name; try to match the unqualified name.
-  for (const lldb::VariableSP &var_sp : variable_list) {
-    lldb::ValueObjectSP valobj_sp(
-        ValueObjectVariable::Create(exe_scope.get(), var_sp));
-    if (valobj_sp && valobj_sp->GetVariable() &&
-        (valobj_sp->GetVariable()->NameMatches(ConstString(unqualified_name))))
-      return valobj_sp;
-  }
-
-  return nullptr;
-}
-
 static lldb::VariableSP DILFindVariable(ConstString name,
-                                        lldb::VariableListSP variable_list) {
+                                        VariableList &variable_list) {
   lldb::VariableSP exact_match;
   std::vector<lldb::VariableSP> possible_matches;
 
-  for (lldb::VariableSP var_sp : *variable_list) {
+  for (lldb::VariableSP var_sp : variable_list) {
     llvm::StringRef str_ref_name = var_sp->GetName().GetStringRef();
-    // Check for global vars, which might start with '::'.
-    str_ref_name.consume_front("::");
 
-    if (str_ref_name == name.GetStringRef())
-      possible_matches.push_back(var_sp);
-    else if (var_sp->NameMatches(name))
-      possible_matches.push_back(var_sp);
-  }
-
-  // Look for exact matches (favors local vars over global vars)
-  auto exact_match_it =
-      llvm::find_if(possible_matches, [&](lldb::VariableSP var_sp) {
-        return var_sp->GetName() == name;
-      });
-
-  if (exact_match_it != possible_matches.end())
-    return *exact_match_it;
-
-  // Look for a global var exact match.
-  for (auto var_sp : possible_matches) {
-    llvm::StringRef str_ref_name = var_sp->GetName().GetStringRef();
     str_ref_name.consume_front("::");
+    // Check for the exact same match
     if (str_ref_name == name.GetStringRef())
       return var_sp;
+
+    // Check for possible matches by base name
+    if (var_sp->NameMatches(name))
+      possible_matches.push_back(var_sp);
   }
 
-  // If there's a single non-exact match, take it.
-  if (possible_matches.size() == 1)
+  // If there's a non-exact match, take it.
+  if (possible_matches.size() > 0)
     return possible_matches[0];
 
   return nullptr;
 }
 
 lldb::ValueObjectSP LookupGlobalIdentifier(
     llvm::StringRef name_ref, std::shared_ptr<StackFrame> stack_frame,
-    lldb::TargetSP target_sp, lldb::DynamicValueType use_dynamic,
-    CompilerType *scope_ptr) {
-  // First look for match in "local" global variables.
-  lldb::VariableListSP variable_list(stack_frame->GetInScopeVariableList(true));
-  name_ref.consume_front("::");
+    lldb::TargetSP target_sp, lldb::DynamicValueType use_dynamic) {
+  // Get a global variables list without the locals from the current frame
+  SymbolContext symbol_context =
+      stack_frame->GetSymbolContext(lldb::eSymbolContextCompUnit);
+  lldb::VariableListSP variable_list =
+      symbol_context.comp_unit->GetVariableList(true);
 
+  name_ref.consume_front("::");
   lldb::ValueObjectSP value_sp;
   if (variable_list) {
     lldb::VariableSP var_sp =
-        DILFindVariable(ConstString(name_ref), variable_list);
+        DILFindVariable(ConstString(name_ref), *variable_list);
     if (var_sp)
       value_sp =
           stack_frame->GetValueObjectForFrameVariable(var_sp, use_dynamic);
   }
 
-  if (value_sp)
-    return value_sp;
-
-  // Also check for static global vars.
-  if (variable_list) {
-    const char *type_name = "";
-    if (scope_ptr)
-      type_name = scope_ptr->GetCanonicalType().GetTypeName().AsCString();
-    std::string name_with_type_prefix =
-        llvm::formatv("{0}::{1}", type_name, name_ref).str();
-    value_sp = LookupStaticIdentifier(*variable_list, stack_frame,
-                                      name_with_type_prefix, name_ref);
-    if (!value_sp)
-      value_sp = LookupStaticIdentifier(*variable_list, stack_frame, name_ref,
-                                        name_ref);
-  }
-
   if (value_sp)
     return value_sp;
 
@@ -129,28 +71,22 @@ lldb::ValueObjectSP LookupGlobalIdentifier(
   target_sp->GetImages().FindGlobalVariables(
       ConstString(name_ref), std::numeric_limits<uint32_t>::max(),
       modules_var_list);
-  if (modules_var_list.Empty())
-    return nullptr;
 
-  for (const lldb::VariableSP &var_sp : modules_var_list) {
-    std::string qualified_name = llvm::formatv("::{0}", name_ref).str();
-    if (var_sp->NameMatches(ConstString(name_ref)) ||
-        var_sp->NameMatches(ConstString(qualified_name))) {
+  if (!modules_var_list.Empty()) {
+    lldb::VariableSP var_sp =
+        DILFindVariable(ConstString(name_ref), modules_var_list);
+    if (var_sp)
       value_sp = ValueObjectVariable::Create(stack_frame.get(), var_sp);
-      break;
-    }
-  }
-
-  if (value_sp)
-    return value_sp;
 
+    if (value_sp)
+      return value_sp;
+  }
   return nullptr;
 }
 
 lldb::ValueObjectSP LookupIdentifier(llvm::StringRef name_ref,
                                      std::shared_ptr<StackFrame> stack_frame,
-                                     lldb::DynamicValueType use_dynamic,
-                                     CompilerType *scope_ptr) {
+                                     lldb::DynamicValueType use_dynamic) {
   // Support $rax as a special syntax for accessing registers.
   // Will return an invalid value in case the requested register doesn't exist.
   if (name_ref.consume_front("$")) {
@@ -164,38 +100,34 @@ lldb::ValueObjectSP LookupIdentifier(llvm::StringRef name_ref,
     return nullptr;
   }
 
-  lldb::VariableListSP variable_list(
-      stack_frame->GetInScopeVariableList(false));
-
   if (!name_ref.contains("::")) {
-    if (!scope_ptr || !scope_ptr->IsValid()) {
-      // Lookup in the current frame.
-      // Try looking for a local variable in current scope.
-      lldb::ValueObjectSP value_sp;
-      if (variable_list) {
-        lldb::VariableSP var_sp =
-            DILFindVariable(ConstString(name_ref), variable_list);
-        if (var_sp)
-          value_sp =
-              stack_frame->GetValueObjectForFrameVariable(var_sp, use_dynamic);
-      }
-      if (!value_sp)
-        value_sp = stack_frame->FindVariable(ConstString(name_ref));
-
-      if (value_sp)
-        return value_sp;
-
-      // Try looking for an instance variable (class member).
-      SymbolContext sc = stack_frame->GetSymbolContext(
-          lldb::eSymbolContextFunction | lldb::eSymbolContextBlock);
-      llvm::StringRef ivar_name = sc.GetInstanceVariableName();
-      value_sp = stack_frame->FindVariable(ConstString(ivar_name));
-      if (value_sp)
-        value_sp = value_sp->GetChildMemberWithName(name_ref);
-
-      if (value_sp)
-        return value_sp;
+    // Lookup in the current frame.
+    // Try looking for a local variable in current scope.
+    lldb::VariableListSP variable_list(
+        stack_frame->GetInScopeVariableList(false));
+
+    lldb::ValueObjectSP value_sp;
+    if (variable_list) {
+      lldb::VariableSP var_sp =
+          variable_list->FindVariable(ConstString(name_ref));
+      if (var_sp)
+        value_sp =
+            stack_frame->GetValueObjectForFrameVariable(var_sp, use_dynamic);
     }
+
+    if (value_sp)
+      return value_sp;
+
+    // Try looking for an instance variable (class member).
+    SymbolContext sc = stack_frame->GetSymbolContext(
+        lldb::eSymbolContextFunction | lldb::eSymbolContextBlock);
+    llvm::StringRef ivar_name = sc.GetInstanceVariableName();
+    value_sp = stack_frame->FindVariable(ConstString(ivar_name));
+    if (value_sp)
+      value_sp = value_sp->GetChildMemberWithName(name_ref);
+
+    if (value_sp)
+      return value_sp;
   }
   return nullptr;
 }

lldb/test/API/commands/frame/var-dil/basics/GlobalVariableLookup/Makefile

@@ -1,3 +1,3 @@
-CXX_SOURCES := main.cpp
+CXX_SOURCES := main.cpp extern.cpp
 
 include Makefile.rules

lldb/test/API/commands/frame/var-dil/basics/GlobalVariableLookup/TestFrameVarDILGlobalVariableLookup.py

@@ -32,20 +32,20 @@ def test_frame_var(self):
         self.expect_var_path("::globalPtr", type="int *")
         self.expect_var_path("::globalRef", type="int &")
 
-        self.expect(
-            "frame variable 'externGlobalVar'",
-            error=True,
-            substrs=["use of undeclared identifier"],
-        )  # 0x00C0FFEE
-        self.expect(
-            "frame variable '::externGlobalVar'",
-            error=True,
-            substrs=["use of undeclared identifier"],
-        )  # ["12648430"])
-
         self.expect_var_path("ns::globalVar", value="13")
         self.expect_var_path("ns::globalPtr", type="int *")
         self.expect_var_path("ns::globalRef", type="int &")
         self.expect_var_path("::ns::globalVar", value="13")
         self.expect_var_path("::ns::globalPtr", type="int *")
         self.expect_var_path("::ns::globalRef", type="int &")
+
+        self.expect_var_path("externGlobalVar", value="2")
+        self.expect_var_path("::externGlobalVar", value="2")
+        self.expect_var_path("ext::externGlobalVar", value="4")
+        self.expect_var_path("::ext::externGlobalVar", value="4")
+
+        self.expect_var_path("ExtStruct::static_inline", value="16")
+
+        # Test local variable priority over global
+        self.expect_var_path("foo", value="1")
+        self.expect_var_path("::foo", value="2")

lldb/test/API/commands/frame/var-dil/basics/GlobalVariableLookup/extern.cpp

@@ -0,0 +1,10 @@
+int externGlobalVar = 2;
+
+namespace ext {
+int externGlobalVar = 4;
+} // namespace ext
+
+struct ExtStruct {
+private:
+  static constexpr inline int static_inline = 16;
+} es;

lldb/test/API/commands/frame/var-dil/basics/GlobalVariableLookup/main.cpp

@@ -10,6 +10,9 @@ int *globalPtr = &globalVar;
 int &globalRef = globalVar;
 } // namespace ns
 
+int foo = 2;
+
 int main(int argc, char **argv) {
+  int foo = 1;
   return 0; // Set a breakpoint here
 }