Fixes build logger call crashing with ImplicitOpenExistentials.

When ImplicitOpenExistentials was enabled (default in Swift language mode 6) the Instrumenter would crash the compiler when building logger calls. This was due to an incorrect assumption that the newly created apply expr wouldn't change type when type-checked. However, the type checker is free to change the kind of expression and did so in circumstances where the call expr was wrapped in an open existential expr.

Author: chrismiles

lib/Sema/InstrumenterSupport.h

@@ -100,7 +100,13 @@ class InstrumenterBase {
     parsedExpr = Added<T *>(dyn_cast<T>(E));
     return result;
   }
-  
+
+  // Type-check parsedExpr. Note that parsedExpr could end up being changed to
+  // a different kind of expr by the type checker.
+  bool doTypeCheckExpr(ASTContext &Ctx, DeclContext *DC, Expr *&parsedExpr) {
+    return doTypeCheckImpl(Ctx, DC, parsedExpr);
+  }
+
 private:
   bool doTypeCheckImpl(ASTContext &Ctx, DeclContext *DC, Expr * &parsedExpr);
 };

lib/Sema/PlaygroundTransform.cpp

@@ -764,13 +764,15 @@ class Instrumenter : InstrumenterBase {
     std::tie(ArgPattern, ArgVariable) = maybeFixupPrintArgument(AE);
 
     AE->setFn(LoggerRef);
-    Added<ApplyExpr *> AddedApply(AE); // safe because we've fixed up the args
 
-    if (!doTypeCheck(Context, TypeCheckDC, AddedApply)) {
+    Expr *E = AE;
+    if (!doTypeCheckExpr(Context, TypeCheckDC, E)) {
       return nullptr;
     }
 
-    return buildLoggerCallWithApply(AddedApply, AE->getSourceRange());
+    Added<Expr *> AddedExpr(E); // safe because we've fixed up the args
+
+    return buildLoggerCallWithAddedExpr(AddedExpr, E->getSourceRange());
   }
 
   Added<Stmt *> logPostPrint(SourceRange SR) {
@@ -890,25 +892,33 @@ class Instrumenter : InstrumenterBase {
         ArgumentList::forImplicitUnlabeled(Context, ArgsWithSourceRange);
     ApplyExpr *LoggerCall =
         CallExpr::createImplicit(Context, LoggerRef, ArgList);
-    Added<ApplyExpr *> AddedLogger(LoggerCall);
 
-    if (!doTypeCheck(Context, TypeCheckDC, AddedLogger)) {
+    Expr *E = LoggerCall;
+    // Type-check the newly created logger call. Note that the type-checker is
+    // free to change the expression type, so type-checking is performed
+    // before wrapping in Added<>.
+    if (!doTypeCheckExpr(Context, TypeCheckDC, E)) {
+      return nullptr;
+    }
+
+    Added<Expr *> AddedLogger(E);
+    if (*AddedLogger == nullptr) {
       return nullptr;
     }
 
-    return buildLoggerCallWithApply(AddedLogger, SR);
+    return buildLoggerCallWithAddedExpr(AddedLogger, SR);
   }
 
-  // Assumes Apply has already been type-checked.
-  Added<Stmt *> buildLoggerCallWithApply(Added<ApplyExpr *> Apply,
-                                         SourceRange SR) {
+  // Assumes expr has already been type-checked.
+  Added<Stmt *> buildLoggerCallWithAddedExpr(Added<Expr *> AddedExpr,
+                                             SourceRange SR) {
     std::pair<PatternBindingDecl *, VarDecl *> PV =
-        buildPatternAndVariable(*Apply);
+        buildPatternAndVariable(*AddedExpr);
 
-    DeclRefExpr *DRE =
-        new (Context) DeclRefExpr(ConcreteDeclRef(PV.second), DeclNameLoc(),
-                                  true, // implicit
-                                  AccessSemantics::Ordinary, Apply->getType());
+    DeclRefExpr *DRE = new (Context)
+        DeclRefExpr(ConcreteDeclRef(PV.second), DeclNameLoc(),
+                    true, // implicit
+                    AccessSemantics::Ordinary, AddedExpr->getType());
 
     UnresolvedDeclRefExpr *SendDataRef = new (Context)
         UnresolvedDeclRefExpr(SendDataName, DeclRefKind::Ordinary,
@@ -919,9 +929,8 @@ class Instrumenter : InstrumenterBase {
     auto *ArgList = ArgumentList::forImplicitUnlabeled(Context, {DRE});
     Expr *SendDataCall =
         CallExpr::createImplicit(Context, SendDataRef, ArgList);
-    Added<Expr *> AddedSendData(SendDataCall);
 
-    if (!doTypeCheck(Context, TypeCheckDC, AddedSendData)) {
+    if (!doTypeCheckExpr(Context, TypeCheckDC, SendDataCall)) {
       return nullptr;
     }
 

test/PlaygroundTransform/existential.swift

@@ -0,0 +1,49 @@
+// RUN: %empty-directory(%t)
+// RUN: cp %s %t/main.swift
+
+// Build PlaygroundSupport module
+// RUN: %target-build-swift -whole-module-optimization -module-name PlaygroundSupport -emit-module-path %t/PlaygroundSupport.swiftmodule -parse-as-library -c -o %t/PlaygroundSupport.o %S/Inputs/SilentPCMacroRuntime.swift %S/Inputs/PlaygroundsRuntime.swift
+
+// -playground
+// RUN: %target-build-swift -swift-version 5 -Xfrontend -playground -o %t/main5a -I=%t %t/PlaygroundSupport.o %t/main.swift
+// RUN: %target-build-swift -swift-version 6 -Xfrontend -playground -o %t/main6a -I=%t %t/PlaygroundSupport.o %t/main.swift
+
+// -pc-macro -playground
+// RUN: %target-build-swift -swift-version 5 -Xfrontend -pc-macro -Xfrontend -playground -o %t/main5b -I=%t %t/PlaygroundSupport.o %t/main.swift
+// RUN: %target-build-swift -swift-version 6 -Xfrontend -pc-macro -Xfrontend -playground -o %t/main6b -I=%t %t/PlaygroundSupport.o %t/main.swift
+
+// RUN: %target-codesign %t/main5a
+// RUN: %target-codesign %t/main5b
+// RUN: %target-codesign %t/main6a
+// RUN: %target-codesign %t/main6b
+
+// RUN: %target-run %t/main5a | %FileCheck %s
+// RUN: %target-run %t/main5b | %FileCheck %s
+// RUN: %target-run %t/main6a | %FileCheck %s
+// RUN: %target-run %t/main6b | %FileCheck %s
+
+// REQUIRES: executable_test
+
+// rdar://136459076
+
+import PlaygroundSupport
+
+func test() -> any Hashable {
+    return 4
+}
+test()
+
+func foo() {
+    let opt: Any? = 32883296
+    opt!
+}
+foo()
+
+// CHECK: [{{.*}}] __builtin_log_scope_entry
+// CHECK-NEXT: [{{.*}}] __builtin_log[='4']
+// CHECK-NEXT: [{{.*}}] __builtin_log_scope_exit
+// CHECK-NEXT: [{{.*}}] __builtin_log[='4']
+// CHECK-NEXT: [{{.*}}] __builtin_log_scope_entry
+// CHECK-NEXT: [{{.*}}] __builtin_log[opt='Optional(32883296)']
+// CHECK-NEXT: [{{.*}}] __builtin_log[='32883296']
+// CHECK-NEXT: [{{.*}}] __builtin_log_scope_exit