Skip to content

Commit e172c28

Browse files
author
Harlan Haskins
authored
[ParseableInterfaces] Stop replacing inaccessible properties with '_' (swiftlang#23782)
Turns out this isn't correct, since SROA can explode these structs into scalars in inlinable code. Put the logic in place to effectively disable it, and document the steps we need to take to make it work in the future.
1 parent d96aebd commit e172c28

File tree

3 files changed

+43
-18
lines changed

3 files changed

+43
-18
lines changed

lib/AST/ASTPrinter.cpp

Lines changed: 33 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -985,6 +985,33 @@ void PrintAST::printTypedPattern(const TypedPattern *TP) {
985985
printTypeLoc(TP->getTypeLoc());
986986
}
987987

988+
/// Determines if we are required to print the name of a property declaration,
989+
/// or if we can elide it by printing a '_' instead.
990+
static bool mustPrintPropertyName(VarDecl *decl, PrintOptions opts) {
991+
// If we're not allowed to omit the name, we must print it.
992+
if (!opts.OmitNameOfInaccessibleProperties) return true;
993+
994+
// If it contributes to the parent's storage, we must print it because clients
995+
// need to be able to directly access the storage.
996+
// FIXME: We might be able to avoid printing names for some of these
997+
// if we serialized references to them using field indices.
998+
if (contributesToParentTypeStorage(decl)) return true;
999+
1000+
// If it's public or @usableFromInline, we must print the name because it's a
1001+
// visible entry-point.
1002+
if (isPublicOrUsableFromInline(decl)) return true;
1003+
1004+
// If it has an initial value, we must print the name because it's used in
1005+
// the mangled name of the initializer expression generator function.
1006+
// FIXME: We _could_ figure out a way to generate an entry point
1007+
// for the initializer expression without revealing the name. We just
1008+
// don't have a mangling for it.
1009+
if (decl->hasInitialValue()) return true;
1010+
1011+
// If none of those are true, we can elide the name of the variable.
1012+
return false;
1013+
}
1014+
9881015
void PrintAST::printPattern(const Pattern *pattern) {
9891016
switch (pattern->getKind()) {
9901017
case PatternKind::Any:
@@ -995,16 +1022,13 @@ void PrintAST::printPattern(const Pattern *pattern) {
9951022
auto named = cast<NamedPattern>(pattern);
9961023
auto decl = named->getDecl();
9971024
recordDeclLoc(decl, [&]{
998-
if (Options.OmitNameOfInaccessibleProperties &&
999-
contributesToParentTypeStorage(decl) &&
1000-
!isPublicOrUsableFromInline(decl) &&
1001-
// FIXME: We need to figure out a way to generate an entry point
1002-
// for the initializer expression without revealing the name.
1003-
!decl->hasInitialValue())
1004-
Printer << "_";
1005-
else
1025+
// FIXME: This always returns true now, because of the FIXMEs listed in
1026+
// mustPrintPropertyName.
1027+
if (mustPrintPropertyName(decl, Options))
10061028
Printer.printName(named->getBoundName());
1007-
});
1029+
else
1030+
Printer << "_";
1031+
});
10081032
break;
10091033
}
10101034

test/ParseableInterface/private-stored-members.swift

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -21,12 +21,12 @@ public struct MyStruct {
2121
// COMMON-NEXT: let publicLet: [[BOOL:(Swift\.)?Bool]]{{$}}
2222
public let publicLet: Bool
2323

24-
// CHECK-NEXT: internal var _: [[INT64]]{{$}}
25-
// RESILIENT-NOT: internal var _: [[INT64]]{{$}}
24+
// CHECK-NEXT: internal var internalVar: [[INT64]]{{$}}
25+
// RESILIENT-NOT: internal var internalVar: [[INT64]]{{$}}
2626
var internalVar: Int64
2727

28-
// CHECK-NEXT: internal let _: [[BOOL]]{{$}}
29-
// RESILIENT-NOT: internal let _: [[BOOL]]{{$}}
28+
// CHECK-NEXT: internal let internalLet: [[BOOL]]{{$}}
29+
// RESILIENT-NOT: internal let internalLet: [[BOOL]]{{$}}
3030
let internalLet: Bool
3131

3232
// COMMON-NEXT: @usableFromInline
@@ -37,12 +37,12 @@ public struct MyStruct {
3737
// COMMON-NEXT: internal let ufiLet: [[BOOL]]{{$}}
3838
@usableFromInline let ufiLet: Bool
3939

40-
// CHECK-NEXT: private var _: [[INT64]]{{$}}
41-
// RESILIENT-NOT: private var _: [[INT64]]{{$}}
40+
// CHECK-NEXT: private var privateVar: [[INT64]]{{$}}
41+
// RESILIENT-NOT: private var privateVar: [[INT64]]{{$}}
4242
private var privateVar: Int64
4343

44-
// CHECK-NEXT: private let _: [[BOOL]]{{$}}
45-
// RESILIENT-NOT: private let _: [[BOOL]]{{$}}
44+
// CHECK-NEXT: private let privateLet: [[BOOL]]{{$}}
45+
// RESILIENT-NOT: private let privateLet: [[BOOL]]{{$}}
4646
private let privateLet: Bool
4747

4848
// CHECK-NOT: private var

test/ParseableInterface/stored-properties.swift

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,8 @@ public struct HasStoredProperties {
4949
// COMMON-NEXT: }
5050
public private(set) var storedPrivateSet: Int
5151

52-
// CHECK: private var _: [[BOOL]]
52+
// CHECK: private var privateVar: [[BOOL]]
53+
// RESILIENT-NOT: private var privateVar: [[BOOL]]
5354
private var privateVar: Bool
5455

5556
// CHECK: @_hasStorage @_hasInitialValue public var storedWithObserversInitialValue: [[INT]] {

0 commit comments

Comments
 (0)