Merge pull request #9560 from adrian-prantl/module-import-hint

adrian-prantl · web-flow · commit 5232c25289d0 · 2024-11-12T13:52:39.000-08:00
[lldb] Add a hint for potential missing module imports in the express…
diff --git a/lldb/packages/Python/lldbsuite/test/make/Swift.rules b/lldb/packages/Python/lldbsuite/test/make/Swift.rules
@@ -253,9 +253,11 @@ ifneq "$(FRAMEWORK)" ""
 	mkdir -p $(FRAMEWORK).framework/Versions/A/Headers
 	mkdir -p $(FRAMEWORK).framework/Versions/A/Modules
 	mkdir -p $(FRAMEWORK).framework/Versions/A/Resources
+ifeq "$(DYLIB_HIDE_SWIFTMODULE)" ""
 ifneq "$(MODULENAME)" ""
 	mkdir -p $(FRAMEWORK).framework/Versions/A/Modules/$(MODULENAME).swiftmodule
 	cp -r $(MODULENAME).swiftmodule $(FRAMEWORK).framework/Versions/A/Modules/$(MODULENAME).swiftmodule/$(ARCH).swiftmodule
+endif
 endif
 	(cd $(FRAMEWORK).framework/Versions; ln -sf A Current)
 	(cd $(FRAMEWORK).framework/; ln -sf Versions/A/Headers Headers)
diff --git a/lldb/source/Plugins/ExpressionParser/Swift/SwiftExpressionParser.cpp b/lldb/source/Plugins/ExpressionParser/Swift/SwiftExpressionParser.cpp
@@ -1654,6 +1654,26 @@ RedirectCallFromSinkToTrampolineFunction(llvm::Module &module,
   return true;
 }
 
+/// Add additional context that is otherwise hard to convey.
+static void AnnotateDiagnostics(DiagnosticManager &diagnostic_manager) {
+  bool append_missing_library_note = false;
+  for (auto &diag : diagnostic_manager.Diagnostics()) {
+    if (!diag || diag->GetSeverity() != eSeverityError)
+      continue;
+    const auto &detail = diag->GetDetail();
+    if (StringRef(detail.message).contains("has no member")) {
+      append_missing_library_note = true;
+      break;
+    }
+  }
+  if (append_missing_library_note)
+    diagnostic_manager.AddDiagnostic(
+        "If the member name is spelled correctly, this error could mean that "
+        "it depends on a (private) type from a module that has not been "
+        "imported into the current context.",
+        eSeverityInfo, eDiagnosticOriginLLDB);
+}
+
 SwiftExpressionParser::ParseResult
 SwiftExpressionParser::Parse(DiagnosticManager &diagnostic_manager,
                              uint32_t first_line, uint32_t last_line) {
@@ -1675,6 +1695,7 @@ SwiftExpressionParser::Parse(DiagnosticManager &diagnostic_manager,
            "error expected");
     expr_diagnostics->PrintDiagnostics(diagnostic_manager, buffer_id,
                                        first_line, last_line);
+    AnnotateDiagnostics(diagnostic_manager);
   };
 
   // In the case of playgrounds, we turn all rewriting functionality off.
diff --git a/lldb/source/Plugins/LanguageRuntime/Swift/SwiftLanguageRuntime.cpp b/lldb/source/Plugins/LanguageRuntime/Swift/SwiftLanguageRuntime.cpp
@@ -298,14 +298,14 @@ class SwiftLanguageRuntimeStub {
   llvm::Expected<uint32_t> GetNumChildren(CompilerType type,
                                           ExecutionContextScope *exe_scopej) {
     STUB_LOG();
-    return 0;
+    return llvm::createStringError("runtime not loaded");
   }
 
-  std::optional<std::string> GetEnumCaseName(CompilerType type,
+  llvm::Expected<std::string> GetEnumCaseName(CompilerType type,
                                               const DataExtractor &data,
                                               ExecutionContext *exe_ctx) {
     STUB_LOG();
-    return {};
+    return llvm::createStringError("runtime not loaded");
   }
 
   std::pair<SwiftLanguageRuntime::LookupResult, std::optional<size_t>>
@@ -2417,7 +2417,7 @@ SwiftLanguageRuntime::GetNumChildren(CompilerType type,
   FORWARD(GetNumChildren, type, exe_scope);
 }
 
-std::optional<std::string> SwiftLanguageRuntime::GetEnumCaseName(
+llvm::Expected<std::string> SwiftLanguageRuntime::GetEnumCaseName(
     CompilerType type, const DataExtractor &data, ExecutionContext *exe_ctx) {
   FORWARD(GetEnumCaseName, type, data, exe_ctx);
 }
diff --git a/lldb/source/Plugins/LanguageRuntime/Swift/SwiftLanguageRuntime.h b/lldb/source/Plugins/LanguageRuntime/Swift/SwiftLanguageRuntime.h
@@ -344,7 +344,7 @@ class SwiftLanguageRuntime : public LanguageRuntime {
 
   /// Determine the enum case name for the \p data value of the enum \p type.
   /// This is performed using Swift reflection.
-  std::optional<std::string> GetEnumCaseName(CompilerType type,
+  llvm::Expected<std::string> GetEnumCaseName(CompilerType type,
                                               const DataExtractor &data,
                                               ExecutionContext *exe_ctx);
 
diff --git a/lldb/source/Plugins/LanguageRuntime/Swift/SwiftLanguageRuntimeDynamicTypeResolution.cpp b/lldb/source/Plugins/LanguageRuntime/Swift/SwiftLanguageRuntimeDynamicTypeResolution.cpp
@@ -952,15 +952,20 @@ findFieldWithName(const std::vector<swift::reflection::FieldInfo> &fields,
   return {SwiftLanguageRuntime::eFound, child_indexes.size()};
 }
 
-std::optional<std::string> SwiftLanguageRuntimeImpl::GetEnumCaseName(
+llvm::Expected<std::string> SwiftLanguageRuntimeImpl::GetEnumCaseName(
     CompilerType type, const DataExtractor &data, ExecutionContext *exe_ctx) {
   using namespace swift::reflection;
   using namespace swift::remote;
   auto *ti = GetSwiftRuntimeTypeInfo(type, exe_ctx->GetFramePtr());
   if (!ti)
-    return {};
+    return llvm::createStringError("could not get runtime type info for " +
+                                   type.GetMangledTypeName().GetStringRef());
+
+  // FIXME: Not reported as an error. There seems to be an odd
+  // compiler optimization happening with single-case payload carrying
+  // enums, which report their type as the inner type.
   if (ti->getKind() != TypeInfoKind::Enum)
-    return {};
+    return "";
 
   auto *eti = llvm::cast<EnumTypeInfo>(ti);
   PushLocalBuffer((int64_t)data.GetDataStart(), data.GetByteSize());
@@ -972,7 +977,7 @@ std::optional<std::string> SwiftLanguageRuntimeImpl::GetEnumCaseName(
 
   // TODO: uncomment this after fixing projection for every type: rdar://138424904
   // LogUnimplementedTypeKind(__FUNCTION__, type);
-  return {};
+  return llvm::createStringError("unimplemented enum kind");
 }
 
 std::pair<SwiftLanguageRuntime::LookupResult, std::optional<size_t>>
diff --git a/lldb/source/Plugins/LanguageRuntime/Swift/SwiftLanguageRuntimeImpl.h b/lldb/source/Plugins/LanguageRuntime/Swift/SwiftLanguageRuntimeImpl.h
@@ -134,7 +134,7 @@ class SwiftLanguageRuntimeImpl {
   std::optional<unsigned> GetNumFields(CompilerType type,
                                         ExecutionContext *exe_ctx);
 
-  std::optional<std::string> GetEnumCaseName(CompilerType type,
+  llvm::Expected<std::string> GetEnumCaseName(CompilerType type,
                                               const DataExtractor &data,
                                               ExecutionContext *exe_ctx);
 
diff --git a/lldb/source/Plugins/TypeSystem/Swift/TypeSystemSwiftTypeRef.cpp b/lldb/source/Plugins/TypeSystem/Swift/TypeSystemSwiftTypeRef.cpp
@@ -45,6 +45,7 @@
 #include "llvm/ADT/ScopeExit.h"
 
 #include <algorithm>
+#include <cerrno>
 #include <sstream>
 #include <type_traits>
 
@@ -4412,30 +4413,36 @@ bool TypeSystemSwiftTypeRef::DumpTypeValue(
     }
     case Node::Kind::Enum:
     case Node::Kind::BoundGenericEnum: {
+      std::string error;
       if (exe_scope)
         if (auto runtime =
                 SwiftLanguageRuntime::Get(exe_scope->CalculateProcess())) {
           ExecutionContext exe_ctx;
           exe_scope->CalculateExecutionContext(exe_ctx);
-          if (auto case_name = runtime->GetEnumCaseName(
-                  {weak_from_this(), type}, data, &exe_ctx)) {
+          auto case_name = runtime->GetEnumCaseName({weak_from_this(), type},
+                                                    data, &exe_ctx);
+          if (case_name && !case_name->empty()) {
             s.PutCString(*case_name);
             return true;
           }
+          if (!case_name)
+            error = toString(case_name.takeError());
         }
 
       // No result available from the runtime, fallback to the AST. This occurs
-      // for some Clang imported enums
+      // for some Clang imported enums.
       if (auto *swift_ast_context =
               GetSwiftASTContextFromExecutionScope(exe_scope)) {
         ExecutionContext exe_ctx;
         exe_scope->CalculateExecutionContext(exe_ctx);
-        return swift_ast_context->DumpTypeValue(
-            ReconstructType(type, &exe_ctx), s, format, data, data_offset,
-            data_byte_size, bitfield_bit_size, bitfield_bit_offset, exe_scope,
-            is_base_class);
+        if (swift_ast_context->DumpTypeValue(
+                ReconstructType(type, &exe_ctx), s, format, data, data_offset,
+                data_byte_size, bitfield_bit_size, bitfield_bit_offset,
+                exe_scope, is_base_class))
+          return true;
       }
-      return {};
+      s << error;
+      return false;
     }
     case Node::Kind::TypeAlias:
     case Node::Kind::BoundGenericTypeAlias: {
@@ -4449,7 +4456,7 @@ bool TypeSystemSwiftTypeRef::DumpTypeValue(
             ReconstructType(type, exe_scope), s, format, data, data_offset,
             data_byte_size, bitfield_bit_size, bitfield_bit_offset, exe_scope,
             is_base_class);
-      return {};
+      return false;
     }
     default:
       assert(false && "Unhandled node kind");
diff --git a/lldb/test/API/lang/swift/expression/error_missing_type/Library.swift b/lldb/test/API/lang/swift/expression/error_missing_type/Library.swift
@@ -0,0 +1,10 @@
+private enum E {
+case WithString(String)
+case OtherCase
+}
+
+public struct S {
+  private var e : E { get { return .WithString("hidden") } }
+}
+
+public func getS() -> S { return S() }
diff --git a/lldb/test/API/lang/swift/expression/error_missing_type/Makefile b/lldb/test/API/lang/swift/expression/error_missing_type/Makefile
@@ -0,0 +1,14 @@
+SWIFT_SOURCES := main.swift
+SWIFTFLAGS_EXTRAS := -I.
+LD_EXTRAS = -lLibrary -L$(BUILDDIR)
+all: libLibrary.dylib a.out
+
+lib%.dylib: %.swift
+	$(MAKE) MAKE_DSYM=NO DYLIB_ONLY=YES \
+		DYLIB_HIDE_SWIFTMODULE=YES \
+		DYLIB_NAME=$(shell basename $< .swift) \
+		DYLIB_SWIFT_SOURCES=$(shell basename $<) \
+		VPATH=$(SRCDIR) -I $(SRCDIR) -f $(MAKEFILE_RULES) all
+	rm -f Library.private.swiftinterface
+
+include Makefile.rules
diff --git a/lldb/test/API/lang/swift/expression/error_missing_type/TestSwiftExpressionErrorMissingType.py b/lldb/test/API/lang/swift/expression/error_missing_type/TestSwiftExpressionErrorMissingType.py
@@ -0,0 +1,19 @@
+import lldb
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test.decorators import *
+import lldbsuite.test.lldbutil as lldbutil
+
+class TestSwiftExpressionErrorMissingType(TestBase):
+    NO_DEBUG_INFO_TESTCASE = True
+
+    @swiftTest
+    def test(self):
+        """Test an extra hint inserted by LLDB for missing module imports"""
+        self.build()
+        os.remove(self.getBuildArtifact("Library.swiftmodule"))
+        lldbutil.run_to_source_breakpoint(self, 'break here',
+                                          lldb.SBFileSpec('main.swift'),
+                                          extra_images=['Library'])
+
+        self.expect('expression s.e', error=True,
+                    substrs=['spelled', 'correctly', 'module', 'import'])
diff --git a/lldb/test/API/lang/swift/expression/error_missing_type/main.swift b/lldb/test/API/lang/swift/expression/error_missing_type/main.swift
@@ -0,0 +1,3 @@
+import Library
+let s = getS()
+print("break here")
diff --git a/lldb/test/API/lang/swift/expression/error_reporting/TestSwiftExpressionErrorReporting.py b/lldb/test/API/lang/swift/expression/error_reporting/TestSwiftExpressionErrorReporting.py
@@ -3,7 +3,7 @@
 from lldbsuite.test.decorators import *
 import lldbsuite.test.lldbutil as lldbutil
 
-class TestSwiftExpressionErrorReportingy(TestBase):
+class TestSwiftExpressionErrorReporting(TestBase):
     NO_DEBUG_INFO_TESTCASE = True
 
     @swiftTest

Original file line number	Diff line number	Diff line change
`@@ -298,14 +298,14 @@ class SwiftLanguageRuntimeStub {`
`298`	`298`	`llvm::Expected<uint32_t> GetNumChildren(CompilerType type,`
`299`	`299`	`ExecutionContextScope *exe_scopej) {`
`300`	`300`	`STUB_LOG();`
`301`		`- return 0;`
	`301`	`+ return llvm::createStringError("runtime not loaded");`
`302`	`302`	`}`
`303`	`303`
`304`		`- std::optional<std::string> GetEnumCaseName(CompilerType type,`
	`304`	`+ llvm::Expected<std::string> GetEnumCaseName(CompilerType type,`
`305`	`305`	`const DataExtractor &data,`
`306`	`306`	`ExecutionContext *exe_ctx) {`
`307`	`307`	`STUB_LOG();`
`308`		`- return {};`
	`308`	`+ return llvm::createStringError("runtime not loaded");`
`309`	`309`	`}`
`310`	`310`
`311`	`311`	`std::pair<SwiftLanguageRuntime::LookupResult, std::optional<size_t>>`
`@@ -2417,7 +2417,7 @@ SwiftLanguageRuntime::GetNumChildren(CompilerType type,`
`2417`	`2417`	`FORWARD(GetNumChildren, type, exe_scope);`
`2418`	`2418`	`}`
`2419`	`2419`
`2420`		`-std::optional<std::string> SwiftLanguageRuntime::GetEnumCaseName(`
	`2420`	`+llvm::Expected<std::string> SwiftLanguageRuntime::GetEnumCaseName(`
`2421`	`2421`	`CompilerType type, const DataExtractor &data, ExecutionContext *exe_ctx) {`
`2422`	`2422`	`FORWARD(GetEnumCaseName, type, data, exe_ctx);`
`2423`	`2423`	`}`
Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,3 @@`
	`1`	`+import Library`
	`2`	`+let s = getS()`
	`3`	`+print("break here")`