[lldb] Implement diagnostics detecting when a variable is not captured

lldb/include/lldb/Expression/DiagnosticManager.h

@@ -107,7 +107,8 @@ class DiagnosticManager {
     m_fixed_expression.clear();
   }
 
-  const DiagnosticList &Diagnostics() { return m_diagnostics; }
+  const DiagnosticList &Diagnostics() const { return m_diagnostics; }
+  DiagnosticList &Diagnostics() { return m_diagnostics; }
 
   bool HasFixIts() const {
     return llvm::any_of(m_diagnostics,

lldb/include/lldb/Symbol/Variable.h

@@ -91,6 +91,9 @@ class Variable : public UserID, public std::enable_shared_from_this<Variable> {
 
   bool IsInScope(StackFrame *frame);
 
+  /// Returns true if this variable is in scope at `addr` inside `block`.
+  bool IsInScope(const Block &block, const Address &addr);
+
   bool LocationIsValidForFrame(StackFrame *frame);
 
   bool LocationIsValidForAddress(const Address &address);

lldb/include/lldb/Symbol/VariableList.h

@@ -24,6 +24,9 @@ class VariableList {
   VariableList();
   virtual ~VariableList();
 
+  VariableList(VariableList &&) = default;
+  VariableList &operator=(VariableList &&) = default;
+
   void AddVariable(const lldb::VariableSP &var_sp);
 
   bool AddVariableIfUnique(const lldb::VariableSP &var_sp);

lldb/include/lldb/Target/Language.h

@@ -403,6 +403,28 @@ class Language : public PluginInterface {
     return nullptr;
   }
 
+  // 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

@@ -2001,6 +2001,11 @@ SwiftLanguage::AreEqualForFrameComparison(const SymbolContext &sc1,
   llvm_unreachable("unhandled enumeration in AreEquivalentFunctions");
 }
 
+std::string
+SwiftLanguage::GetParentNameIfClosure(llvm::StringRef mangled_name) const {
+  return SwiftLanguageRuntime::GetParentNameIfClosure(mangled_name);
+}
+
 //------------------------------------------------------------------
 // Static Functions
 //------------------------------------------------------------------

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

@@ -81,6 +81,9 @@ class SwiftLanguage : public Language {
   std::optional<bool>
   AreEqualForFrameComparison(const SymbolContext &sc1,
                              const SymbolContext &sc2) const override;
+
+  std::string
+  GetParentNameIfClosure(llvm::StringRef mangled_name) const override;
   //------------------------------------------------------------------
   // Static Functions
   //------------------------------------------------------------------

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

@@ -205,6 +205,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"
@@ -1496,4 +1498,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/Symbol/Variable.cpp

@@ -291,28 +291,9 @@ bool Variable::IsInScope(StackFrame *frame) {
       // this variable was defined in is currently
       Block *deepest_frame_block =
           frame->GetSymbolContext(eSymbolContextBlock).block;
-      if (deepest_frame_block) {
-        SymbolContext variable_sc;
-        CalculateSymbolContext(&variable_sc);
-
-        // Check for static or global variable defined at the compile unit
-        // level that wasn't defined in a block
-        if (variable_sc.block == nullptr)
-          return true;
-
-        // Check if the variable is valid in the current block
-        if (variable_sc.block != deepest_frame_block &&
-            !variable_sc.block->Contains(deepest_frame_block))
-          return false;
-
-        // If no scope range is specified then it means that the scope is the
-        // same as the scope of the enclosing lexical block.
-        if (m_scope_range.IsEmpty())
-          return true;
-
-        addr_t file_address = frame->GetFrameCodeAddress().GetFileAddress();
-        return m_scope_range.FindEntryThatContains(file_address) != nullptr;
-      }
+      Address frame_addr = frame->GetFrameCodeAddress();
+      if (deepest_frame_block)
+        return IsInScope(*deepest_frame_block, frame_addr);
     }
     break;
 
@@ -322,6 +303,27 @@ bool Variable::IsInScope(StackFrame *frame) {
   return false;
 }
 
+bool Variable::IsInScope(const Block &block, const Address &addr) {
+  SymbolContext variable_sc;
+  CalculateSymbolContext(&variable_sc);
+
+  // Check for static or global variable defined at the compile unit
+  // level that wasn't defined in a block
+  if (variable_sc.block == nullptr)
+    return true;
+
+  // Check if the variable is valid in the current block
+  if (variable_sc.block != &block && !variable_sc.block->Contains(&block))
+    return false;
+
+  // If no scope range is specified then it means that the scope is the
+  // same as the scope of the enclosing lexical block.
+  if (m_scope_range.IsEmpty())
+    return true;
+
+  return m_scope_range.FindEntryThatContains(addr.GetFileAddress()) != nullptr;
+}
+
 Status Variable::GetValuesForVariableExpressionPath(
     llvm::StringRef variable_expr_path, ExecutionContextScope *scope,
     GetVariableCallback callback, void *baton, VariableList &variable_list,