[ParseableInterface] Print conformances inherited via private protos #20169
Conversation
|
@swift-ci Please test |
| @@ -400,6 +401,134 @@ static void printImports(raw_ostream &out, ModuleDecl *M) { | |||
| } | |||
| } | |||
|
|
|||
| // FIXME: Copied from ASTPrinter.cpp... | |||
There was a problem hiding this comment.
I think Harlan's moving these into a header file anyway in another PR. Once that lands, I'll move PrintOptions::printParseableInterfaceFile up to Frontend, and then we'll only need one copy of these helpers.
| if (!shouldPrint(NTD)) | ||
| continue; | ||
| if (auto ty = TL.getType()) { | ||
| bool foundUnprintable = ty.findIf([shouldPrint](Type subTy) { |
There was a problem hiding this comment.
Can you factor this out? Especially if it's gonna be reused in #20146.
There was a problem hiding this comment.
It's not exactly the same query, though: the shouldPrint here is an argument to the function the way ASTPrinter is currently written. I suppose Type itself could have an overload of findIf that passes GenericTypeDecls.
| return scope.isPublic(); | ||
| } | ||
|
|
||
| static bool isPublicOrUsableFromInline(Type ty) { |
There was a problem hiding this comment.
Ah! Okay, yes, this copy should probably make its way into the header introduced in #20004 (or whichever PR I land that contains just the cleanup)
| } | ||
|
|
||
| void | ||
| printSynthesizedExtensionIfNeeded(raw_ostream &out, |
There was a problem hiding this comment.
Can you add a comment here explaining the motivation for why we're printing the empty extensions after the types? I think it's useful to mention that we are printing conformances to public types that are hidden by conformances to private types, and that they're all satisfied by the requirements that have already been printed.
There was a problem hiding this comment.
Oops. Yes, this whole scheme deserves commenting.
There was a problem hiding this comment.
It kind of stinks to print the extensions at the end of the file rather than next to the type, but I don't see a good way around that.
| // NEGATIVE-NOT: typealias Element = | ||
| // CHECK: public func inference(_: Int){{$}} | ||
| public func inference(_: Int) {} | ||
| // CHECK: public typealias Inferred = Swift.Int | ||
| } // CHECK: {{^}$}} | ||
|
|
||
|
|
||
| public protocol PublicProto {} |
There was a problem hiding this comment.
Seems the CHECKs are missing here
There was a problem hiding this comment.
…right. I was manually inspecting the output but didn't actually write down the CHECK lines I wanted.
|
Most concerns addressed. @swift-ci Please smoke test |
| ExistentialLayout layout = inheritedTy->getExistentialLayout(); | ||
| for (ProtocolType *protoTy : layout.getProtocols()) | ||
| whichProtocols.push_back(protoTy->getDecl()); | ||
| // FIXME: This ignores layout constraints, but currently we don't support |
There was a problem hiding this comment.
You're ignoring superclass constraints too.
|
Wait, the example I gave doesn't make sense, but you should still make sure it does something reasonable with subclass existentials. |
|
Ooh, you're right, I totally forgot to test those. It's okay to ignore superclass constraints because we're printing the class itself, but… …oh, no, that's not correct; we allow specifying the superclass like that, don't we. Yikes. Okay, I may do that in a follow-up PR. |
|
@jrose-apple It looks like we allow this: But not this: Which is a bit weird, but I don't think we intended for subclass existentials to work there at all. So it might be that there's nothing to test. |
In this code:
private protocol MyProto: Hashable {}
public struct MyStruct: MyProto {}
Being Hashable is part of MyStruct's public API, even though it's not
written explicitly. If we're not going to require people to write it
explicitly, we need to make sure it gets printed.
rdar://problem/44662501
|
I think we allow that for people who make typealiases of subclass existentials, but if we already check them for access then I'm happy. Added two more tests. @swift-ci Please smoke test |
|
@slavapestov @harlanhaskins Ready to go? |
In this code...
...being Hashable is part of MyStruct's public API, even though it's not written explicitly. If we're not going to require people to write it explicitly, we need to make sure it gets printed.
rdar://problem/44662501