[InterfaceGen] Print private/internal properties #19127
Conversation
All properties which contribute to the storage of a type should be printed, and their names should be hidden from interfaces. Print them with '_' as their name, and teach the parser to recognize these special patterns when parsing interface files. Partially resolves rdar://43810647
harlanhaskins
force-pushed
the
private-property
branch
from
c6f6867 to
ba5564a
Compare
lib/AST/ASTPrinter.cpp
Outdated
| @@ -64,13 +64,19 @@ void PrintOptions::clearSynthesizedExtension() { | |||
| TransformContext.reset(); | |||
| } | |||
|
|
|||
| static bool contributesToParentTypeStorage(const AbstractStorageDecl *ASD) { | |||
| return ASD->getDeclContext()->isTypeContext() && ASD->hasStorage() && | |||
There was a problem hiding this comment.
We only want this if the type context is non-resilient.
lib/Sema/TypeCheckDecl.cpp
Outdated
| if (dc->isTypeContext()) | ||
| // Property decls in type context must bind variables unless in a module | ||
| // interface, where private members don't bind variables but contribute | ||
| // to storage. |
There was a problem hiding this comment.
Is this still necessary? You are creating the VarDecl now.
| // RUN: %empty-directory(%t) | ||
| // R UN: %target-swift-frontend -emit-module -o %t/Test~partial.swiftmodule -module-name Test -primary-file %s | ||
| // R UN: %target-swift-frontend -merge-modules -emit-module -o %t/Test.swiftmodule %t/Test~partial.swiftmodule | ||
| // R UN: %target-swift-ide-test -print-module -module-to-print=Test -source-filename=x -I %t | %FileCheck %s |
There was a problem hiding this comment.
Forgot to re-enable these.
|
What’s the plan for properties that have internal types? |
|
"Not covered by this PR" :-) I haven't passed along to Harlan the thing you told me about value witnesses, that if a struct has private types in its fields IRGen just gives up and always uses value witnesses. (Yes? No?) |
|
Yes that is correct. We still need to know the size and alignment of the type though. I think the best solution for this and the class vtable layout issues is to include any referenced internal types in the interface output. |
|
Do we want to rename them/their properties to avoid exposing private implementation details? |
|
We'll have to for |
harlanhaskins
force-pushed
the
private-property
branch
from
d7b7c95 to
aaecce9
Compare
|
I've added a test to make sure the layout doesn't change if the type is declared in this file, in another module, or in an interface file. |
harlanhaskins
force-pushed
the
private-property
branch
from
058ad22 to
31b9b4b
Compare
| var internalVar: Int64 | ||
|
|
||
| // CHECK-NEXT: internal let _: Bool{{$}} | ||
| // RESILIENT-NOT: internal let _: Bool{{$}} |
There was a problem hiding this comment.
I'm curious whether this is printed as var or let in a resilient-with-@_fixed_contents context. Do we want to promise that a private let will stay constant in future versions? cc @slavapestov
(This doesn't have to block this PR.)
| // CHECK-EXEC: %[[MYCLASS_INIT:[0-9]+]] = call swiftcc [[MYCLASS]]* @"$S{{[^ ]+}}7MyClassCACycfC"(%swift.type* swiftself %{{[0-9]+}}) | ||
| let myClass = MyClass() | ||
|
|
||
| // These are uninteresting as they just call into the standard getter and setter. |
There was a problem hiding this comment.
I think it's worth CHECKing that they do.
There was a problem hiding this comment.
Discussed offline, since it ends up just reaching into the table at pretty opaque offsets (in this case, 21, 20, and 11) it's not a super interesting check...
|
@swift-ci please test |
|
Build failed |
|
Build failed |
|
@swift-ci please test |
|
Build failed |
|
Build failed |
|
⛵ |
All properties which contribute to the storage of a type should be
printed, and their names should be hidden from interfaces. Print them
with '_' as their name, and teach the parser to recognize these special
patterns when parsing interface files.
Partially resolves rdar://43810647