Skip to content

Commit 5380d8c

Browse files
committed
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.
1 parent cb545ed commit 5380d8c

File tree

3 files changed

+78
-17
lines changed

3 files changed

+78
-17
lines changed

lib/Sema/InstrumenterSupport.h

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,13 @@ class InstrumenterBase {
100100
parsedExpr = Added<T *>(dyn_cast<T>(E));
101101
return result;
102102
}
103-
103+
104+
// Type-check parsedExpr. Note that parsedExpr could end up being changed to
105+
// a different kind of expr by the type checker.
106+
bool doTypeCheckExpr(ASTContext &Ctx, DeclContext *DC, Expr *&parsedExpr) {
107+
return doTypeCheckImpl(Ctx, DC, parsedExpr);
108+
}
109+
104110
private:
105111
bool doTypeCheckImpl(ASTContext &Ctx, DeclContext *DC, Expr * &parsedExpr);
106112
};

lib/Sema/PlaygroundTransform.cpp

Lines changed: 22 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -764,13 +764,15 @@ class Instrumenter : InstrumenterBase {
764764
std::tie(ArgPattern, ArgVariable) = maybeFixupPrintArgument(AE);
765765

766766
AE->setFn(LoggerRef);
767-
Added<ApplyExpr *> AddedApply(AE); // safe because we've fixed up the args
768767

769-
if (!doTypeCheck(Context, TypeCheckDC, AddedApply)) {
768+
Expr *E = AE;
769+
if (!doTypeCheckExpr(Context, TypeCheckDC, E)) {
770770
return nullptr;
771771
}
772772

773-
return buildLoggerCallWithApply(AddedApply, AE->getSourceRange());
773+
Added<Expr *> AddedExpr(E); // safe because we've fixed up the args
774+
775+
return buildLoggerCallWithAddedExpr(AddedExpr, E->getSourceRange());
774776
}
775777

776778
Added<Stmt *> logPostPrint(SourceRange SR) {
@@ -890,25 +892,30 @@ class Instrumenter : InstrumenterBase {
890892
ArgumentList::forImplicitUnlabeled(Context, ArgsWithSourceRange);
891893
ApplyExpr *LoggerCall =
892894
CallExpr::createImplicit(Context, LoggerRef, ArgList);
893-
Added<ApplyExpr *> AddedLogger(LoggerCall);
894895

895-
if (!doTypeCheck(Context, TypeCheckDC, AddedLogger)) {
896+
Expr *E = LoggerCall;
897+
// Type-check the newly created logger call. Note that the type-checker is
898+
// free to change the expression type, so type-checking is performed
899+
// before wrapping in Added<>.
900+
if (!doTypeCheckExpr(Context, TypeCheckDC, E)) {
896901
return nullptr;
897902
}
898903

899-
return buildLoggerCallWithApply(AddedLogger, SR);
904+
Added<Expr *> AddedLogger(E);
905+
906+
return buildLoggerCallWithAddedExpr(AddedLogger, SR);
900907
}
901908

902-
// Assumes Apply has already been type-checked.
903-
Added<Stmt *> buildLoggerCallWithApply(Added<ApplyExpr *> Apply,
904-
SourceRange SR) {
909+
// Assumes expr has already been type-checked.
910+
Added<Stmt *> buildLoggerCallWithAddedExpr(Added<Expr *> AddedExpr,
911+
SourceRange SR) {
905912
std::pair<PatternBindingDecl *, VarDecl *> PV =
906-
buildPatternAndVariable(*Apply);
913+
buildPatternAndVariable(*AddedExpr);
907914

908-
DeclRefExpr *DRE =
909-
new (Context) DeclRefExpr(ConcreteDeclRef(PV.second), DeclNameLoc(),
910-
true, // implicit
911-
AccessSemantics::Ordinary, Apply->getType());
915+
DeclRefExpr *DRE = new (Context)
916+
DeclRefExpr(ConcreteDeclRef(PV.second), DeclNameLoc(),
917+
true, // implicit
918+
AccessSemantics::Ordinary, AddedExpr->getType());
912919

913920
UnresolvedDeclRefExpr *SendDataRef = new (Context)
914921
UnresolvedDeclRefExpr(SendDataName, DeclRefKind::Ordinary,
@@ -919,9 +926,8 @@ class Instrumenter : InstrumenterBase {
919926
auto *ArgList = ArgumentList::forImplicitUnlabeled(Context, {DRE});
920927
Expr *SendDataCall =
921928
CallExpr::createImplicit(Context, SendDataRef, ArgList);
922-
Added<Expr *> AddedSendData(SendDataCall);
923929

924-
if (!doTypeCheck(Context, TypeCheckDC, AddedSendData)) {
930+
if (!doTypeCheckExpr(Context, TypeCheckDC, SendDataCall)) {
925931
return nullptr;
926932
}
927933

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
// RUN: %empty-directory(%t)
2+
// RUN: cp %s %t/main.swift
3+
4+
// Build PlaygroundSupport module
5+
// 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
6+
7+
// -playground
8+
// RUN: %target-build-swift -swift-version 5 -Xfrontend -playground -o %t/main5a -I=%t %t/PlaygroundSupport.o %t/main.swift
9+
// RUN: %target-build-swift -swift-version 6 -Xfrontend -playground -o %t/main6a -I=%t %t/PlaygroundSupport.o %t/main.swift
10+
11+
// -pc-macro -playground
12+
// RUN: %target-build-swift -swift-version 5 -Xfrontend -pc-macro -Xfrontend -playground -o %t/main5b -I=%t %t/PlaygroundSupport.o %t/main.swift
13+
// RUN: %target-build-swift -swift-version 6 -Xfrontend -pc-macro -Xfrontend -playground -o %t/main6b -I=%t %t/PlaygroundSupport.o %t/main.swift
14+
15+
// RUN: %target-codesign %t/main5a
16+
// RUN: %target-codesign %t/main5b
17+
// RUN: %target-codesign %t/main6a
18+
// RUN: %target-codesign %t/main6b
19+
20+
// RUN: %target-run %t/main5a | %FileCheck %s
21+
// RUN: %target-run %t/main5b | %FileCheck %s
22+
// RUN: %target-run %t/main6a | %FileCheck %s
23+
// RUN: %target-run %t/main6b | %FileCheck %s
24+
25+
// REQUIRES: executable_test
26+
27+
// rdar://136459076
28+
29+
import PlaygroundSupport
30+
31+
func test() -> any Hashable {
32+
return 4
33+
}
34+
test()
35+
36+
func foo() {
37+
let opt: Any? = 32883296
38+
opt!
39+
}
40+
foo()
41+
42+
// CHECK: [{{.*}}] __builtin_log_scope_entry
43+
// CHECK-NEXT: [{{.*}}] __builtin_log[='4']
44+
// CHECK-NEXT: [{{.*}}] __builtin_log_scope_exit
45+
// CHECK-NEXT: [{{.*}}] __builtin_log[='4']
46+
// CHECK-NEXT: [{{.*}}] __builtin_log_scope_entry
47+
// CHECK-NEXT: [{{.*}}] __builtin_log[opt='Optional(32883296)']
48+
// CHECK-NEXT: [{{.*}}] __builtin_log[='32883296']
49+
// CHECK-NEXT: [{{.*}}] __builtin_log_scope_exit

0 commit comments

Comments
 (0)