Skip to content

Commit a16372e

Browse files
dbukowskidbukowski
authored
Merge pull request #71293 from apple/eng/dbukowski/disable-logging-moveonly-types-in-playground-transform
[PlaygroundTransform] Disable logging `~Copyable` types as they can’t be passed to the generic logging function
2 parents 68e3488 + e0cbc62 commit a16372e

File tree

4 files changed

+96
-5
lines changed

4 files changed

+96
-5
lines changed

‎lib/Sema/PlaygroundTransform.cpp‎

Lines changed: 26 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -429,7 +429,7 @@ class Instrumenter : InstrumenterBase {
429429
++EI;
430430
}
431431
}
432-
} else {
432+
} else if (shouldLog(AE->getSrc())) {
433433
std::pair<PatternBindingDecl *, VarDecl *> PV =
434434
buildPatternAndVariable(AE->getSrc());
435435
DeclRefExpr *DRE = new (Context)
@@ -521,7 +521,7 @@ class Instrumenter : InstrumenterBase {
521521
}
522522
Handled = true; // Never log ()
523523
}
524-
if (!Handled) {
524+
if (!Handled && shouldLog(E)) {
525525
// do the same as for all other expressions
526526
std::pair<PatternBindingDecl *, VarDecl *> PV =
527527
buildPatternAndVariable(E);
@@ -539,7 +539,7 @@ class Instrumenter : InstrumenterBase {
539539
}
540540
}
541541
} else {
542-
if (E->getType()->getCanonicalType() != Context.TheEmptyTupleType) {
542+
if (E->getType()->getCanonicalType() != Context.TheEmptyTupleType && shouldLog(E)) {
543543
std::pair<PatternBindingDecl *, VarDecl *> PV =
544544
buildPatternAndVariable(E);
545545
Added<Stmt *> Log = buildLoggerCall(
@@ -559,7 +559,7 @@ class Instrumenter : InstrumenterBase {
559559
} else if (auto *S = Element.dyn_cast<Stmt *>()) {
560560
S->walk(CF);
561561
if (auto *RS = dyn_cast<ReturnStmt>(S)) {
562-
if (RS->hasResult()) {
562+
if (RS->hasResult() && shouldLog(RS->getResult())) {
563563
std::pair<PatternBindingDecl *, VarDecl *> PV =
564564
buildPatternAndVariable(RS->getResult());
565565
DeclRefExpr *DRE = new (Context) DeclRefExpr(
@@ -620,7 +620,7 @@ class Instrumenter : InstrumenterBase {
620620
if (PL && Options.LogFunctionParameters) {
621621
size_t EI = 0;
622622
for (const auto &PD : *PL) {
623-
if (PD->hasName()) {
623+
if (PD->hasName() && shouldLog(PD)) {
624624
DeclBaseName Name = PD->getName();
625625
Expr *PVVarRef = new (Context)
626626
DeclRefExpr(PD, DeclNameLoc(), /*implicit=*/true,
@@ -657,6 +657,10 @@ class Instrumenter : InstrumenterBase {
657657
// after or instead of the expression they're looking at. Only call this
658658
// if the variable has an initializer.
659659
Added<Stmt *> logVarDecl(VarDecl *VD) {
660+
if (!shouldLog(VD)) {
661+
return nullptr;
662+
}
663+
660664
if (isa<ConstructorDecl>(TypeCheckDC) && VD->getNameStr().equals("self")) {
661665
// Don't log "self" in a constructor
662666
return nullptr;
@@ -673,6 +677,10 @@ class Instrumenter : InstrumenterBase {
673677
if (auto *DRE = dyn_cast<DeclRefExpr>(*RE)) {
674678
VarDecl *VD = cast<VarDecl>(DRE->getDecl());
675679

680+
if (!shouldLog(VD)) {
681+
return nullptr;
682+
}
683+
676684
if (isa<ConstructorDecl>(TypeCheckDC) && VD->getBaseName() == "self") {
677685
// Don't log "self" in a constructor
678686
return nullptr;
@@ -686,6 +694,10 @@ class Instrumenter : InstrumenterBase {
686694
Expr *B = MRE->getBase();
687695
ConcreteDeclRef M = MRE->getMember();
688696

697+
if (!shouldLog(M.getDecl())) {
698+
return nullptr;
699+
}
700+
689701
if (isa<ConstructorDecl>(TypeCheckDC) && digForName(B) == "self") {
690702
// Don't log attributes of "self" in a constructor
691703
return nullptr;
@@ -785,6 +797,15 @@ class Instrumenter : InstrumenterBase {
785797
return std::make_pair(PBD, VD);
786798
}
787799

800+
bool shouldLog(ASTNode node) {
801+
// Don't try to log ~Copyable types, as we can't pass them to the generic logging functions yet.
802+
if (auto *VD = dyn_cast_or_null<ValueDecl>(node.dyn_cast<Decl *>()))
803+
return VD->hasInterfaceType() ? !VD->getInterfaceType()->isNoncopyable() : true;
804+
if (auto *E = node.dyn_cast<Expr *>())
805+
return !E->getType()->isNoncopyable();
806+
return true;
807+
}
808+
788809
Added<Stmt *> buildLoggerCall(Added<Expr *> E, SourceRange SR,
789810
StringRef Name) {
790811
Expr *NameExpr = new (Context) StringLiteralExpr(

‎test/PlaygroundTransform/noncopyable_assignment.swift‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
// RUN: %empty-directory(%t)
2+
// RUN: cp %s %t/main.swift
3+
// 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
4+
// RUN: %target-build-swift -Xfrontend -playground -o %t/main -I=%t %t/PlaygroundSupport.o %t/main.swift
5+
// RUN: %target-codesign %t/main
6+
// RUN: %target-run %t/main | %FileCheck %s
7+
// RUN: %target-build-swift -Xfrontend -pc-macro -Xfrontend -playground -o %t/main2 -I=%t %t/PlaygroundSupport.o %t/main.swift -module-name main
8+
// RUN: %target-codesign %t/main2
9+
// RUN: %target-run %t/main2 | %FileCheck %s
10+
// REQUIRES: executable_test
11+
12+
import PlaygroundSupport
13+
14+
struct A: ~Copyable {}
15+
16+
var a: A
17+
a = A()
18+
19+
struct B {}
20+
21+
var b: B
22+
b = B()
23+
// note: a should not be logged (at least until move-only types can be passed to the generic logging functions)
24+
// CHECK: [{{.*}}] __builtin_log[b='B()']

‎test/PlaygroundTransform/noncopyable_declaration.swift‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
// RUN: %empty-directory(%t)
2+
// RUN: cp %s %t/main.swift
3+
// 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
4+
// RUN: %target-build-swift -Xfrontend -playground -o %t/main -I=%t %t/PlaygroundSupport.o %t/main.swift
5+
// RUN: %target-codesign %t/main
6+
// RUN: %target-run %t/main | %FileCheck %s
7+
// RUN: %target-build-swift -Xfrontend -pc-macro -Xfrontend -playground -o %t/main2 -I=%t %t/PlaygroundSupport.o %t/main.swift -module-name main
8+
// RUN: %target-codesign %t/main2
9+
// RUN: %target-run %t/main2 | %FileCheck %s
10+
// REQUIRES: executable_test
11+
12+
import PlaygroundSupport
13+
14+
struct A: ~Copyable {}
15+
16+
let a = A()
17+
18+
struct B {}
19+
20+
let b = B()
21+
// note: a should not be logged (at least until move-only types can be passed to the generic logging functions)
22+
// CHECK: [{{.*}}] __builtin_log[b='B()']

‎test/PlaygroundTransform/noncopyable_functions.swift‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
// RUN: %empty-directory(%t)
2+
// RUN: cp %s %t/main.swift
3+
// 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
4+
// RUN: %target-build-swift -Xfrontend -playground -o %t/main -I=%t %t/PlaygroundSupport.o %t/main.swift
5+
// RUN: %target-codesign %t/main
6+
// RUN: %target-run %t/main | %FileCheck %s
7+
// RUN: %target-build-swift -Xfrontend -pc-macro -Xfrontend -playground -o %t/main2 -I=%t %t/PlaygroundSupport.o %t/main.swift -module-name main
8+
// RUN: %target-codesign %t/main2
9+
// RUN: %target-run %t/main2 | %FileCheck %s
10+
// REQUIRES: executable_test
11+
12+
import PlaygroundSupport
13+
14+
struct A: ~Copyable {}
15+
16+
func f(_ a: consuming A) -> A {
17+
return a
18+
}
19+
20+
f(A())
21+
// note: a should not be logged (at least until move-only types can be passed to the generic logging functions)
22+
// CHECK: [{{.*}}] __builtin_log_scope_entry
23+
// CHECK-NEXT: [{{.*}}] __builtin_log_scope_exit
24+
// CHECK-NOT: __builtin_log

0 commit comments

Comments
 (0)