Merge pull request #77350 from swiftlang/eng/chrismiles/Instrumenter-buildPatternAndVariable-crash

Fixes build logger call crashing with ImplicitOpenExistentials.

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,30 @@ 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;
     }
 
-    return buildLoggerCallWithApply(AddedLogger, SR);
+    Added<Expr *> AddedLogger(E);
+
+    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 +926,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