Fix dynamic import of libraries in Xcode playgrounds.

The Swift expression parser uses the fixit mechanism to force
SwiftASTContext to be reinitialized if it contains fatal
errors. Playgrounds disable fixit application and thus defeated the
mechanism. This became a problem after the late dylib notification was
fixed in 1348ae3. This patch detects
the situation where the fixit is identical to the original expression
and makes one retry even if fixits are disabled.

rdar://85431564
(cherry picked from commit e52de6b)

Author: adrian-prantl

lldb/packages/Python/lldbsuite/test/make/Makefile.rules

@@ -708,6 +708,20 @@ ifneq "$(DYLIB_HIDE_SWIFTMODULE)" ""
             -emit-library $(DYLIB_SWIFT_FLAGS) -o $@ \
             $(patsubst %.swiftmodule.o,,$(DYLIB_OBJECTS))
 else
+ifneq "$(FRAMEWORK)" ""
+	mkdir -p $(FRAMEWORK).framework/Versions/A/Headers
+	mkdir -p $(FRAMEWORK).framework/Versions/A/Modules
+	mkdir -p $(FRAMEWORK).framework/Versions/A/Resources
+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
+	(cd $(FRAMEWORK).framework/Versions; ln -sf A Current)
+	(cd $(FRAMEWORK).framework/; ln -sf Versions/A/Headers Headers)
+	(cd $(FRAMEWORK).framework/; ln -sf Versions/A/Modules Modules)
+	(cd $(FRAMEWORK).framework/; ln -sf Versions/A/Resources Resources)
+	(cd $(FRAMEWORK).framework/; ln -sf Versions/A/$(FRAMEWORK) $(FRAMEWORK))
+endif
 	$(SWIFTC) $(patsubst -g,,$(SWIFTFLAGS)) \
             -emit-library $(DYLIB_SWIFT_FLAGS) -o $@ $(DYLIB_OBJECTS)
 endif

lldb/source/Expression/UserExpression.cpp

@@ -264,15 +264,22 @@ UserExpression::Evaluate(ExecutionContext &exe_ctx,
 
   *fixed_expression = user_expression_sp->GetFixedText().str();
 
+  // Swift Playgrounds disable fixits, but SwiftASTContext may get
+  // poisoned (see SwiftASTContextForExpressions::ModulesDidLoad())
+  // during expression evaluation. When this happens we want to re-run
+  // the same expression with a freshly initialized SwiftASTContext.
+  bool is_replay = !options.GetAutoApplyFixIts() && expr == *fixed_expression;
+
   // If there is a fixed expression, try to parse it:
   if (!parse_success) {
     // Delete the expression that failed to parse before attempting to parse
     // the next expression.
     user_expression_sp.reset();
 
     execution_results = lldb::eExpressionParseError;
-    if (!fixed_expression->empty() && options.GetAutoApplyFixIts()) {
-      const uint64_t max_fix_retries = options.GetRetriesWithFixIts();
+    if (!fixed_expression->empty() &&
+        (options.GetAutoApplyFixIts() || is_replay)) {
+      const uint64_t max_fix_retries = is_replay ? 1 : options.GetRetriesWithFixIts();
       for (uint64_t i = 0; i < max_fix_retries; ++i) {
         // Try parsing the fixed expression.
         lldb::UserExpressionSP fixed_expression_sp(

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

@@ -1221,10 +1221,18 @@ struct SwiftASTContextError : public llvm::ErrorInfo<SwiftASTContextError> {
 /// This indicates an error in the SwiftASTContext.
 struct ModuleImportError : public llvm::ErrorInfo<ModuleImportError> {
   static char ID;
-  std::string Message;
-
-  ModuleImportError(llvm::Twine Message) : Message(Message.str()) {}
-  void log(llvm::raw_ostream &OS) const override { OS << "ModuleImport"; }
+  std::string msg;
+  bool is_explicit;
+
+  ModuleImportError(llvm::Twine message, bool is_explicit = false)
+      : msg(message.str()), is_explicit(is_explicit) {}
+  void log(llvm::raw_ostream &OS) const override {
+    if (is_explicit)
+      OS << "error while processing import statement:";
+    else
+      OS << "error while importing implicit dependency:";
+    OS << msg;
+  }
   std::error_code convertToErrorCode() const override {
     return inconvertibleErrorCode();
   }
@@ -1321,8 +1329,7 @@ static llvm::Expected<ParsedExpression> ParseAndImport(
     const char *msg = implicit_import_error.AsCString();
     if (!msg)
       msg = "error status positive, but import still failed";
-    return make_error<ModuleImportError>(llvm::Twine("in implicit-import:\n") +
-                                         msg);
+    return make_error<ModuleImportError>(msg);
   }
 
   swift::ImplicitImportInfo importInfo;
@@ -1471,8 +1478,7 @@ static llvm::Expected<ParsedExpression> ParseAndImport(
           const char *msg = auto_import_error.AsCString();
           if (!msg)
             msg = "error status positive, but import still failed";
-          return make_error<ModuleImportError>(
-              llvm::Twine("in user-import:\n") + msg);
+          return make_error<ModuleImportError>(msg, /*explicit=*/true);
     }
   }
 
@@ -1524,13 +1530,19 @@ unsigned SwiftExpressionParser::Parse(DiagnosticManager &diagnostic_manager,
                      m_sc, *m_exe_scope, m_options, repl, playground);
 
   if (!parsed_expr) {
+    bool user_import = false;
     bool retry = false;
     handleAllErrors(parsed_expr.takeError(),
                     [&](const ModuleImportError &MIE) {
+                      if (MIE.is_explicit) {
+                        // A new dylib may have poisoned the context.
+                        retry = true;
+                        user_import = true;
+                      }
                       if (swift_ast_ctx->GetClangImporter())
                         // Already on backup power.
                         diagnostic_manager.PutString(eDiagnosticSeverityError,
-                                                     MIE.Message);
+                                                     MIE.message());
                       else
                         // Discard the shared scratch context and retry.
                         retry = true;
@@ -1552,7 +1564,8 @@ unsigned SwiftExpressionParser::Parse(DiagnosticManager &diagnostic_manager,
     // fresh SwiftASTContext initialized with the flags from the
     // current lldb::Module / Swift dylib to avoid header search
     // mismatches.
-    m_sc.target_sp->SetUseScratchTypesystemPerModule(true);
+    if (!user_import)
+      m_sc.target_sp->SetUseScratchTypesystemPerModule(true);
     return 2;
   }
 

lldb/test/API/lang/swift/playgrounds/Dylib.swift

@@ -0,0 +1,2 @@
+
+public func f() -> String { return "Hello from the Dylib!" }

lldb/test/API/lang/swift/playgrounds/Makefile

@@ -6,7 +6,7 @@ SWIFT_SOURCES = PlaygroundStub.swift
 LD_EXTRAS := -Xlinker -rpath -Xlinker /usr/lib/swift
 LD_EXTRAS += -L. -lPlaygroundsRuntime
 
-PlaygroundStub: libPlaygroundsRuntime.dylib
+PlaygroundStub: libPlaygroundsRuntime.dylib Dylib.framework
 
 include Makefile.rules
 
@@ -15,3 +15,9 @@ libPlaygroundsRuntime.dylib: PlaygroundsRuntime.swift
 		DYLIB_SWIFT_SOURCES=PlaygroundsRuntime.swift \
 		DYLIB_NAME=PlaygroundsRuntime
 
+Dylib.framework: Dylib.swift
+	$(MAKE) -f $(MAKEFILE_RULES) \
+		FRAMEWORK=Dylib \
+		DYLIB_SWIFT_SOURCES=Dylib.swift \
+		DYLIB_NAME=Dylib \
+		SWIFTFLAGS_EXTRAS="-Xcc -DNEW_OPTION_FROM_DYLIB=1"

lldb/test/API/lang/swift/playgrounds/TestPlaygrounds.py

@@ -113,6 +113,8 @@ def do_test(self, force_target):
             process, breakpoint)
 
         self.assertEqual(len(threads), 1)
+        self.expect('settings set target.swift-framework-search-paths "%s"' %
+                    self.getBuildDir())
 
         contents = ""
 
@@ -147,4 +149,24 @@ def do_test(self, force_target):
         ret = self.frame().EvaluateExpression("get_output()")
         playground_output = ret.GetSummary()
         self.assertTrue(playground_output is not None)
-        self.assertTrue("=\\'23\\'" in playground_output)
+        self.assertIn("=\\'23\\'", playground_output)
+
+        # Test importing a library that adds new Clang options.
+        log = self.getBuildArtifact('types.log')
+        self.expect('log enable lldb types -f ' + log)
+        contents = "import Dylib\nf()\n"
+        res = self.frame().EvaluateExpression(contents, options)
+        ret = self.frame().EvaluateExpression("get_output()")
+        playground_output = ret.GetSummary()
+        self.assertTrue(playground_output is not None)
+        self.assertIn("Hello from the Dylib", playground_output)
+
+        # Scan through the types log to make sure the SwiftASTContext was poisoned.
+        logfile = open(log, "r")
+        found = 0
+        for line in logfile:
+            if 'New Swift image added' in line \
+               and 'Versions/A/Dylib' in line \
+               and 'ClangImporter needs to be reinitialized' in line:
+                    found += 1
+        self.assertEqual(found, 1)