[NCGenerics] Even more fixes for building the stdlib (Part 3) #70548
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@swift-ci smoke test |
kavon
force-pushed
the
ncgenerics-stdlib-building-v3
branch
from
2184ed2 to
da14e36
Compare
|
@swift-ci please smoke test Linux |
1 similar comment
|
@swift-ci please smoke test Linux |
|
preset=buildbot_linux_ncg |
slavapestov
reviewed
Jan 3, 2024
lib/AST/ProtocolConformance.cpp
Outdated
| }; | ||
| }; | ||
|
|
||
| unsigned RecursionTracker = 0; |
There was a problem hiding this comment.
This is not thread safe, for when sourcekit spawns multiple ASTContexts in a single process, etc. The real fix for catching runaway recursion is to record this in the IFS and make the whole thing iterative instead of recursive, but we're not there yet. Let's just remove this part for now
kavon
force-pushed
the
ncgenerics-stdlib-building-v3
branch
2 times, most recently
from
8c7fae0 to
e853eb1
Compare
|
@swift-ci please test |
slavapestov
reviewed
Jan 5, 2024
slavapestov
reviewed
Jan 5, 2024
slavapestov
reviewed
Jan 5, 2024
lib/AST/ASTPrinter.cpp
Outdated
| @@ -1472,6 +1472,10 @@ static void reconstituteInverses(GenericSignature genericSig, | |||
| for (auto tp : typeParams) { | |||
| assert(tp); | |||
|
|
|||
| // Any generic parameter requiring a class could not have an inverse. | |||
| if (genericSig->requiresClass(tp)) | |||
There was a problem hiding this comment.
does <T where T: AnyObject, T: Copyable> minimize as <T where T: AnyObject>? It should via a concrete conformance but I believe that needs to be implemented in the requirement machine itself.
There was a problem hiding this comment.
Maybe instead of doing sig->requiresProtocol(paramTy, Copyable) you can map paramTy into the signature's environment and do a global lookup with the archetype to Copyable? Then you won't need this special case.
kavon
force-pushed
the
ncgenerics-stdlib-building-v3
branch
from
e853eb1 to
3c51681
Compare
|
@swift-ci smoke test |
kavon
force-pushed
the
ncgenerics-stdlib-building-v3
branch
from
3c51681 to
f445ee2
Compare
|
@swift-ci please test |
kavon
force-pushed
the
ncgenerics-stdlib-building-v3
branch
from
f445ee2 to
9bb45e8
Compare
|
@swift-ci please smoke test |
When building the stdlib with noncopyable generics enabled, requirements for Copyable and Escapable end up being inferred on the protocols Copyable and Escapable, themselves. So we need to opt-out Copyable from requiring Escapable, and vice versa.
Workaround for rdar://119950540 when dealing with invertible protocols. This should be sound because "its an invariant that subst() on a conformance returns the same thing as a global lookup with subst() of the conforming type, assuming you don't have overlapping conformances."
The "no implicit copy" parameters such as a `consuming` or `borrowing` parameter of an otherwise Copyable type was not being handled correctly by `conformsToInvertible`. We don't have conformances for SIL types in the AST, so they're supposed to be handled earlier. fixes rdar://120462547
This is a temporary measure until I get a chance to fix the symbol mangling. This round-tripper is exactly flagging the current known issue in that all of the symbols get extra `Copyable` and `Escapable` requirements mangled into them, and thus reconsituting the type from the mangled name doesn't give you the same type back.
In cases where the generic parameter is class-constrained, `GenericSignature::requiresProtocol` will not contain `Copyable` or `Escapable` because GenericSignature minimization will recognize that the class already requires them. Thus, because classes always require those protocols, we can simply ask if the generic parameter is required to be a class to determine if it had any inverses.
Instead of injecting Copyable & Escapable protocols into every ExistentialLayout, only add those protocols if the existing protocols don't already imply them. This simplifies things like `any Error` protocol, so it still only lists one protocol in its existential layout. But existentials like `any NoCopyP` still end up with a Copyable in its layout.
resolves rdar://120512544
In the Concurrency module, an unavailable Sendable extension triggers an error about unavailable Copyable conformance. This conformance came through in the SubstitutionMap of a BoundGenericType, so I think it's ok to just ignore these.
There are a bunch of static `collectExistentialConformances` copied around Sema and SILGen that are almost the same, save for whether they want to permit missing conformances and/or check conditional conformances. This commit requestifies and combines all but one of these functions into a `ModuleDecl::collectExistentialConformances`. The motivation for this clean-up is another place that will need this procedure.
The SILGen emission for `Builtin.createAsyncTask` performs an existential erasure and assumed that `any Any.Type` has no protocols in its ExistentialLayout, but it's now possible to have some there with NoncopyableGenerics.
kavon
force-pushed
the
ncgenerics-stdlib-building-v3
branch
from
dde546a to
5e050e6
Compare
|
@swift-ci please test |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
fixes rdar://120462547&120512544
Just another assorted set of changes as I fix things while getting the stdlib building with NoncopyableGenerics.