Skip to content

[ModuleInterface] Don't print extensions of internal types #19440

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Sep 26, 2018
Merged

[ModuleInterface] Don't print extensions of internal types #19440

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions lib/AST/ASTPrinter.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,14 @@ PrintOptions PrintOptions::printTextualInterfaceFile() {
}
}

// Skip extensions that extend things we wouldn't print.
if (auto *ED = dyn_cast<ExtensionDecl>(D)) {
if (!shouldPrint(ED->getExtendedNominal(), options))
return false;
// FIXME: We also need to check the generic signature for constraints
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is essentially already what the code in AccessRequests.cpp is doing. Isn't checking the extension's max access sufficient here?

Also I notice in shouldPrint() you have some special logic for typealiases. Please refactor this to use isPassthroughTypeAlias(), which is in Sema right now but could move to a method on TypeAliasDecl.

Copy link
Contributor Author

@jrose-apple jrose-apple Sep 21, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Max access doesn't take UFI into account. EDIT: …but I'll make sure not to duplicate the code when I do the right thing.

The logic that's there isn't for passthrough typealiases; it's for the typealiases synthesized in the body of generic types.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if it would be better if we changed associated type inference to not create those special typealiases for generic parameters at all.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, when it first went in Doug mentioned that that's where we want to go. We can remove this then.

// that we can't reference.
}

// Skip typealiases that just redeclare generic parameters.
if (auto *alias = dyn_cast<TypeAliasDecl>(D)) {
if (alias->isImplicit()) {
Expand Down
9 changes: 9 additions & 0 deletions test/ModuleInterface/access-filter.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,3 +102,12 @@ extension PublicStruct {
// CHECK: public private(set) static var secretlySettable: Int{{$}}
public private(set) static var secretlySettable: Int = 0
} // CHECK: {{^[}]$}}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add a CHECK-NOT for InternalStruct_BAD?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's one at the top of the file that just says "BAD" must not appear anywhere. :-)

extension InternalStruct_BAD: PublicProto {
internal static var dummy: Int { return 0 }
}

// CHECK: extension UFIStruct : PublicProto {{[{]$}}
extension UFIStruct: PublicProto {
internal static var dummy: Int { return 0 }
} // CHECK-NEXT: {{^[}]$}}