[lldb][Expression] Remove m_found_function_with_type_info in favour of boolean return

[Michael137]

I've been skimming this code while investigating a bug around module lookup and this looked like something we could clean up. We don't need to be carrying around state inside of NameSearchContext to tell us to look into modules. We can signal this via a boolean return from LookupFunction.

[llvmbot]

@llvm/pr-subscribers-lldb

Author: Michael Buch (Michael137)

Changes

I've been skimming this code while investigating a bug around module lookup and this looked like something we could clean up. We don't need to be carrying around state inside of NameSearchContext to tell us to look into modules. We can signal this via a boolean return from LookupFunction.

---

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

3 Files Affected:

• (modified) lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp (+11-12)
• (modified) lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.h (+4-1)
• (modified) lldb/source/Plugins/ExpressionParser/Clang/NameSearchContext.h (-1)

diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
index db4973b4a4d3e..fec8d29248c20 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
@@ -1047,8 +1047,6 @@ void ClangExpressionDeclMap::LookupInModulesDeclVendor(
     MaybeRegisterFunctionBody(copied_function);
 
     context.AddNamedDecl(copied_function);
-
-    context.m_found_function_with_type_info = true;
   } else if (auto copied_var = dyn_cast<clang::VarDecl>(copied_decl)) {
     context.AddNamedDecl(copied_var);
     context.m_found_variable = true;
@@ -1217,11 +1215,11 @@ SymbolContextList ClangExpressionDeclMap::SearchFunctionsInSymbolContexts(
   return sc_func_list;
 }
 
-void ClangExpressionDeclMap::LookupFunction(
+bool ClangExpressionDeclMap::LookupFunction(
     NameSearchContext &context, lldb::ModuleSP module_sp, ConstString name,
     const CompilerDeclContext &namespace_decl) {
   if (!m_parser_vars)
-    return;
+    return false;
 
   Target *target = m_parser_vars->m_exe_ctx.GetTargetPtr();
 
@@ -1281,6 +1279,8 @@ void ClangExpressionDeclMap::LookupFunction(
     }
   }
 
+  bool found_function_with_type_info = false;
+
   if (sc_list.GetSize()) {
     Symbol *extern_symbol = nullptr;
     Symbol *non_extern_symbol = nullptr;
@@ -1297,7 +1297,7 @@ void ClangExpressionDeclMap::LookupFunction(
           continue;
 
         AddOneFunction(context, sym_ctx.function, nullptr);
-        context.m_found_function_with_type_info = true;
+        found_function_with_type_info = true;
       } else if (sym_ctx.symbol) {
         Symbol *symbol = sym_ctx.symbol;
         if (target && symbol->GetType() == eSymbolTypeReExported) {
@@ -1313,20 +1313,20 @@ void ClangExpressionDeclMap::LookupFunction(
       }
     }
 
-    if (!context.m_found_function_with_type_info) {
+    if (!found_function_with_type_info) {
       for (clang::NamedDecl *decl : decls_from_modules) {
         if (llvm::isa<clang::FunctionDecl>(decl)) {
           clang::NamedDecl *copied_decl =
               llvm::cast_or_null<FunctionDecl>(CopyDecl(decl));
           if (copied_decl) {
             context.AddNamedDecl(copied_decl);
-            context.m_found_function_with_type_info = true;
+            found_function_with_type_info = true;
           }
         }
       }
     }
 
-    if (!context.m_found_function_with_type_info) {
+    if (!found_function_with_type_info) {
       if (extern_symbol) {
         AddOneFunction(context, nullptr, extern_symbol);
       } else if (non_extern_symbol) {
@@ -1334,6 +1334,8 @@ void ClangExpressionDeclMap::LookupFunction(
       }
     }
   }
+
+  return found_function_with_type_info;
 }
 
 void ClangExpressionDeclMap::FindExternalVisibleDecls(
@@ -1432,10 +1434,7 @@ void ClangExpressionDeclMap::FindExternalVisibleDecls(
     }
   }
 
-  LookupFunction(context, module_sp, name, namespace_decl);
-
-  // Try the modules next.
-  if (!context.m_found_function_with_type_info)
+  if (!LookupFunction(context, module_sp, name, namespace_decl))
     LookupInModulesDeclVendor(context, name);
 
   if (target && !context.m_found_variable && !namespace_decl) {
diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.h b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.h
index ea763b94fcc75..dddc5a06c9051 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.h
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.h
@@ -481,7 +481,10 @@ class ClangExpressionDeclMap : public ClangASTSource {
   ///
   /// \param[in] namespace_decl
   ///     If valid and module is non-NULL, the parent namespace.
-  void LookupFunction(NameSearchContext &context, lldb::ModuleSP module_sp,
+  ///
+  /// \returns Returns \c true if we successfully found a function
+  /// and could create a decl with correct type-info for it.
+  bool LookupFunction(NameSearchContext &context, lldb::ModuleSP module_sp,
                       ConstString name,
                       const CompilerDeclContext &namespace_decl);
 
diff --git a/lldb/source/Plugins/ExpressionParser/Clang/NameSearchContext.h b/lldb/source/Plugins/ExpressionParser/Clang/NameSearchContext.h
index 9a3320636081b..a890c935a48fa 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/NameSearchContext.h
+++ b/lldb/source/Plugins/ExpressionParser/Clang/NameSearchContext.h
@@ -40,7 +40,6 @@ struct NameSearchContext {
   llvm::SmallSet<CompilerType, 5> m_function_types;
 
   bool m_found_variable = false;
-  bool m_found_function_with_type_info = false;
   bool m_found_local_vars_nsp = false;
   bool m_found_type = false;
 

[labath]

👍