Merge pull request #10836 from felipepiovezan/felipe/cherry-pick-diagnostics-notcaptured

🍒[lldb] Implement diagnostic for non-captured vars

Author: felipepiovezan

lldb/include/lldb/Target/Language.h

@@ -505,6 +505,28 @@ class Language : public PluginInterface {
 
   virtual FormatEntity::Entry GetFunctionNameFormat() const { return {}; }
 
+  // BEGIN SWIFT
+  // Implement LanguageCPlusPlus::GetParentNameIfClosure and upstream this.
+  // rdar://152321823
+
+  /// If `mangled_name` refers to a function that is a "closure-like" function,
+  /// returns the name of the parent function where the input closure was
+  /// defined. Returns an empty string if there is no such parent, or if the
+  /// query does not make sense for this language.
+  virtual std::string
+  GetParentNameIfClosure(llvm::StringRef mangled_name) const {
+    return "";
+  }
+
+  /// If `sc` corresponds to a "closure-like" function (as defined in
+  /// `GetParentNameIfClosure`), returns the parent Function*
+  /// where a variable named `variable_name` exists.
+  /// Returns nullptr if `sc` is not a closure, or if the query does
+  /// not make sense for this language.
+  Function *FindParentOfClosureWithVariable(llvm::StringRef variable_name,
+                                            const SymbolContext &sc) const;
+  // END SWIFT
+
 protected:
   // Classes that inherit from Language can see and modify these
 

lldb/source/Plugins/ExpressionParser/Swift/SwiftUserExpression.cpp

@@ -35,6 +35,7 @@
 #include "lldb/Symbol/Type.h"
 #include "lldb/Symbol/Variable.h"
 #include "lldb/Symbol/VariableList.h"
+#include "lldb/Target/Language.h"
 #include "lldb/Utility/LLDBAssert.h"
 #include "lldb/Utility/LLDBLog.h"
 #include "lldb/Utility/Log.h"
@@ -693,6 +694,68 @@ SwiftUserExpression::GetTextAndSetExpressionParser(
   return parse_result;
 }
 
+/// If `sc` represents a "closure-like" function according to `lang`, and
+/// `var_name` can be found in a parent context, create a diagnostic
+/// explaining that this variable is available but not captured by the closure.
+static std::string
+CreateVarInParentScopeDiagnostic(StringRef var_name,
+                                 StringRef parent_func_name) {
+  return llvm::formatv("A variable named '{0}' existed in function '{1}', but "
+                       "it was not captured in the closure.\nHint: the "
+                       "variable may be available in a parent frame.",
+                       var_name, parent_func_name);
+}
+
+/// If `diagnostic_manager` contains a "cannot find <var_name> in scope"
+/// diagnostic, attempt to enhance it by showing if `var_name` is used inside a
+/// closure, not captured, but defined in a parent scope.
+static void EnhanceNotInScopeDiagnostics(DiagnosticManager &diagnostic_manager,
+                                         ExecutionContextScope *exe_scope) {
+  if (!exe_scope)
+    return;
+  lldb::StackFrameSP stack_frame = exe_scope->CalculateStackFrame();
+  if (!stack_frame)
+    return;
+  SymbolContext sc =
+      stack_frame->GetSymbolContext(lldb::eSymbolContextEverything);
+  Language *swift_lang =
+      Language::FindPlugin(lldb::LanguageType::eLanguageTypeSwift);
+  if (!swift_lang)
+    return;
+
+  static const RegularExpression not_in_scope_regex =
+      RegularExpression("(.*): cannot find '([^']+)' in scope\n(.*)");
+  for (auto &diag : diagnostic_manager.Diagnostics()) {
+    if (!diag)
+      continue;
+
+    llvm::SmallVector<StringRef, 4> match_groups;
+
+    if (StringRef old_rendered_msg = diag->GetDetail().rendered;
+        !not_in_scope_regex.Execute(old_rendered_msg, &match_groups))
+      continue;
+
+    StringRef prefix = match_groups[1];
+    StringRef var_name = match_groups[2];
+    StringRef suffix = match_groups[3];
+
+    Function *parent_func =
+        swift_lang->FindParentOfClosureWithVariable(var_name, sc);
+    if (!parent_func)
+      continue;
+    std::string new_message = CreateVarInParentScopeDiagnostic(
+        var_name, parent_func->GetDisplayName());
+
+    std::string new_rendered =
+        llvm::formatv("{0}: {1}\n{2}", prefix, new_message, suffix);
+    const DiagnosticDetail &old_detail = diag->GetDetail();
+    diag = std::make_unique<Diagnostic>(
+        diag->getKind(), diag->GetCompilerID(),
+        DiagnosticDetail{old_detail.source_location, old_detail.severity,
+                         std::move(new_message), std::move(new_rendered)});
+  }
+}
+
 bool SwiftUserExpression::Parse(DiagnosticManager &diagnostic_manager,
                                 ExecutionContext &exe_ctx,
                                 lldb_private::ExecutionPolicy execution_policy,
@@ -888,6 +951,7 @@ bool SwiftUserExpression::Parse(DiagnosticManager &diagnostic_manager,
                 fixed_expression.substr(fixed_start, fixed_end - fixed_start);
         }
       }
+      EnhanceNotInScopeDiagnostics(diagnostic_manager, exe_scope);
       return false;
     case ParseResult::success:
       llvm_unreachable("Success case is checked separately before switch!");

lldb/source/Plugins/Language/Swift/SwiftLanguage.cpp

@@ -2029,6 +2029,10 @@ SwiftLanguage::GetFunctionNameInfo(ConstString name) const {
   return {func_name_type, ConstString(basename)};
 }
 
+std::string
+SwiftLanguage::GetParentNameIfClosure(llvm::StringRef mangled_name) const {
+  return SwiftLanguageRuntime::GetParentNameIfClosure(mangled_name);
+}
 //------------------------------------------------------------------
 // Static Functions
 //------------------------------------------------------------------

lldb/source/Plugins/Language/Swift/SwiftLanguage.h

@@ -84,6 +84,9 @@ class SwiftLanguage : public Language {
 
   std::pair<lldb::FunctionNameType, std::optional<ConstString>>
   GetFunctionNameInfo(ConstString name) const override;
+
+  std::string
+  GetParentNameIfClosure(llvm::StringRef mangled_name) const override;
   //------------------------------------------------------------------
   // Static Functions
   //------------------------------------------------------------------

lldb/source/Plugins/LanguageRuntime/Swift/SwiftLanguageRuntime.h

@@ -204,6 +204,8 @@ class SwiftLanguageRuntime : public LanguageRuntime {
                          const SymbolContext *sc = nullptr,
                          const ExecutionContext *exe_ctx = nullptr);
 
+  static std::string GetParentNameIfClosure(llvm::StringRef mangled_name);
+
   /// Demangle a symbol to a swift::Demangle node tree.
   ///
   /// This is a central point of access, for purposes such as logging.

lldb/source/Plugins/LanguageRuntime/Swift/SwiftLanguageRuntimeNames.cpp

@@ -10,6 +10,8 @@
 //
 //===----------------------------------------------------------------------===//
 
+#include "Plugins/TypeSystem/Swift/SwiftASTContext.h"
+#include "Plugins/TypeSystem/Swift/SwiftDemangle.h"
 #include "SwiftLanguageRuntime.h"
 
 #include "lldb/Breakpoint/StoppointCallbackContext.h"
@@ -1497,4 +1499,30 @@ SwiftLanguageRuntime::GetGenericSignature(llvm::StringRef function_name,
   return signature;
 }
 
+std::string SwiftLanguageRuntime::GetParentNameIfClosure(StringRef name) {
+  using Kind = Node::Kind;
+  swift::Demangle::Context ctx;
+  auto *node = SwiftLanguageRuntime::DemangleSymbolAsNode(name, ctx);
+  if (!node || node->getKind() != Node::Kind::Global)
+    return "";
+
+  // Replace the top level closure node with the child function-like node, and
+  // attempt to remangle. If successful, this produces the parent function-like
+  // entity.
+  static const auto closure_kinds = {Kind::ImplicitClosure,
+                                     Kind::ExplicitClosure};
+  static const auto function_kinds = {Kind::ImplicitClosure,
+                                      Kind::ExplicitClosure, Kind::Function};
+  auto *closure_node = swift_demangle::GetFirstChildOfKind(node, closure_kinds);
+  auto *parent_func_node =
+      swift_demangle::GetFirstChildOfKind(closure_node, function_kinds);
+  if (!parent_func_node)
+    return "";
+  swift_demangle::ReplaceChildWith(*node, *closure_node, *parent_func_node);
+
+  if (ManglingErrorOr<std::string> mangled = swift::Demangle::mangleNode(node);
+      mangled.isSuccess())
+    return mangled.result();
+  return "";
+}
 } // namespace lldb_private

lldb/source/Plugins/TypeSystem/Swift/SwiftDemangle.h

@@ -22,6 +22,32 @@
 namespace lldb_private {
 namespace swift_demangle {
 
+using NodePointer = swift::Demangle::NodePointer;
+using Node = swift::Demangle::Node;
+
+/// Returns the first child of `node` whose kind is in `kinds`.
+inline NodePointer GetFirstChildOfKind(NodePointer node,
+                                       llvm::ArrayRef<Node::Kind> kinds) {
+  if (!node)
+    return nullptr;
+  for (auto *child : *node)
+    if (llvm::is_contained(kinds, child->getKind()))
+      return child;
+  return nullptr;
+}
+
+/// Assumes that `to_replace` is a child of `parent`, and replaces it with
+/// `new_child`. The Nodes must all be owned by the same context.
+inline void ReplaceChildWith(Node &parent, Node &to_replace, Node &new_child) {
+  for (unsigned idx = 0; idx < parent.getNumChildren(); idx++) {
+    auto *child = parent.getChild(idx);
+    if (child == &to_replace)
+      parent.replaceChild(idx, &new_child);
+    return;
+  }
+  llvm_unreachable("invalid child passed to replaceChildWith");
+}
+
 /// Access an inner node by following the given Node::Kind path.
 ///
 /// Note: The Node::Kind path is relative to the given root node. The root

lldb/source/Target/Language.cpp

@@ -14,8 +14,10 @@
 
 #include "lldb/Core/PluginManager.h"
 #include "lldb/Interpreter/OptionValueProperties.h"
+#include "lldb/Symbol/CompileUnit.h"
 #include "lldb/Symbol/SymbolFile.h"
 #include "lldb/Symbol/TypeList.h"
+#include "lldb/Symbol/VariableList.h"
 #include "lldb/Target/Target.h"
 #include "lldb/Utility/Stream.h"
 
@@ -574,3 +576,165 @@ bool SourceLanguage::IsObjC() const {
 bool SourceLanguage::IsCPlusPlus() const {
   return name == llvm::dwarf::DW_LNAME_C_plus_plus;
 }
+
+// BEGIN SWIFT
+// Implement LanguageCPlusPlus::GetParentNameIfClosure and upstream this.
+// rdar://152321823
+
+/// If `sc` represents a "closure"-like function (according to
+/// Language::GetParentNameIfClosure), returns sc.function and all parent
+/// functions up to and including the first non-closure-like function. If `sc`
+/// is not a closure, or if the query does not make sense for `language`,
+/// returns a list containing only sc.function.
+static llvm::SmallVector<Function *>
+GetParentFunctionsWhileClosure(const SymbolContext &sc,
+                               const Language &language) {
+  // The algorithm below terminates on the assumption that
+  // `GetParentNameIfClosure` produces an empty string when composing that
+  // function with itself enough times. For safety, define an upper limit.
+  constexpr auto upper_limit = 8;
+
+  llvm::SmallVector<Function *> parents;
+  Function *root = sc.function;
+  if (root == nullptr)
+    return parents;
+
+  parents.push_back(root);
+  for (int idx = 0; idx < upper_limit; idx++) {
+    ConstString mangled = root->GetMangled().GetMangledName();
+    std::string parent = language.GetParentNameIfClosure(mangled);
+    if (parent.empty())
+      break;
+
+    // Find the enclosing function, if it exists.
+    SymbolContextList sc_list;
+    Module::LookupInfo lookup_info(
+        ConstString(parent), lldb::FunctionNameType::eFunctionNameTypeFull,
+        lldb::eLanguageTypeSwift);
+    sc.module_sp->FindFunctions(lookup_info, CompilerDeclContext(),
+                                ModuleFunctionSearchOptions(), sc_list);
+    if (sc_list.GetSize() != 1 || sc_list[0].function == nullptr)
+      break;
+    parents.push_back(sc_list[0].function);
+    root = sc_list[0].function;
+  }
+  return parents;
+}
+
+/// Scans the line table of `function` looking for the first entry whose line
+/// number is `line_number`. If no such entry is found, returns the entry
+/// closest to but after `line_number`.
+static std::optional<Address> FindAddressForLineNumber(Function &function,
+                                                       uint32_t line_number) {
+  CompileUnit *cu = function.GetCompileUnit();
+  LineTable *line_table = cu ? cu->GetLineTable() : nullptr;
+  if (line_table == nullptr)
+    return std::nullopt;
+
+  // Get the first line entry for this function.
+  AddressRange func_range = function.GetAddressRange();
+  uint32_t first_entry_idx;
+  {
+    LineEntry first_line_entry;
+    line_table->FindLineEntryByAddress(func_range.GetBaseAddress(),
+                                       first_line_entry, &first_entry_idx);
+  }
+
+  LineEntry best_match;
+  for (uint32_t entry_idx = first_entry_idx; entry_idx < line_table->GetSize();
+       entry_idx++) {
+    LineEntry next_line;
+    line_table->GetLineEntryAtIndex(entry_idx, next_line);
+
+    // Stop if this entry is outside the range of `function`.
+    Address base_addr = next_line.range.GetBaseAddress();
+    if (!func_range.ContainsFileAddress(base_addr))
+      break;
+
+    // Stop on an exact match.
+    if (next_line.line == line_number) {
+      best_match = next_line;
+      break;
+    }
+
+    // Otherwise, keep track of the best match so far.
+    if (next_line.line > line_number && next_line.line < best_match.line)
+      best_match = next_line;
+  }
+
+  return best_match.range.GetBaseAddress();
+}
+
+/// Given a list of functions, returns a map: Function -> VariableList
+/// containing local variables of each function.
+static std::map<Function *, VariableList>
+GetFuncToLocalVariablesMap(llvm::ArrayRef<Function *> funcs) {
+  std::map<Function *, VariableList> map;
+  for (Function *function : funcs) {
+    VariableList &variable_list = map[function];
+    Block &block = function->GetBlock(true /*can_create=*/);
+    block.AppendBlockVariables(
+        true /*can_create=*/, true /*get_child_block_variables=*/,
+        true /*stop_if_child_block_is_inlined_function=*/,
+        [](Variable *v) { return true; }, &variable_list);
+  }
+  return map;
+}
+
+/// Returns the first line associated with `function`.
+static uint32_t GetLineNumberForFunction(Function &function) {
+  FileSpec filespec;
+  uint32_t line_num = 0;
+  function.GetStartLineSourceInfo(filespec, line_num);
+  return line_num;
+}
+
+/// Checks if `var` is in scope inside `function` at line `line_number`.
+/// If this check can't be done, a best-effort comparison of:
+///    line_number >= var.line_number
+/// is performed.
+static bool IsVariableInScopeAtLine(uint32_t line_number,
+                                    std::optional<Address> line_addr,
+                                    Variable &var) {
+  auto fallback_line_comp = [&] {
+    return line_number >= var.GetDeclaration().GetLine();
+  };
+
+  if (!line_addr)
+    return fallback_line_comp();
+
+  Block *defining_block = line_addr->CalculateSymbolContextBlock();
+  if (defining_block)
+    return var.IsInScope(*defining_block, *line_addr);
+  return fallback_line_comp();
+}
+
+Function *Language::FindParentOfClosureWithVariable(
+    llvm::StringRef variable_name, const SymbolContext &closure_sc) const {
+  llvm::SmallVector<Function *> function_chain =
+      GetParentFunctionsWhileClosure(closure_sc, *this);
+  if (function_chain.empty())
+    return nullptr;
+
+  std::map<Function *, VariableList> func_to_locals =
+      GetFuncToLocalVariablesMap(function_chain);
+
+  llvm::ArrayRef<Function *> children =
+      llvm::ArrayRef(function_chain).drop_back();
+  llvm::ArrayRef<Function *> parents =
+      llvm::ArrayRef(function_chain).drop_front();
+
+  for (auto [parent, child] : llvm::zip_equal(parents, children)) {
+    VariableList &parent_variables = func_to_locals[parent];
+    uint32_t child_line_number = GetLineNumberForFunction(*child);
+    std::optional<Address> parent_line_addr =
+        FindAddressForLineNumber(*parent, child_line_number);
+
+    for (const VariableSP &var : parent_variables)
+      if (var->GetName() == variable_name &&
+          IsVariableInScopeAtLine(child_line_number, parent_line_addr, *var))
+        return parent;
+  }
+  return nullptr;
+}
+// END SWIFT