Noncopyable means no conformance to Copyable #68818
Merged
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 please smoke test |
slavapestov
reviewed
Sep 28, 2023
slavapestov
reviewed
Sep 28, 2023
slavapestov
reviewed
Sep 28, 2023
kavon
force-pushed
the
noncopyable-means-no-conformance
branch
from
925a901 to
a243dcb
Compare
|
@swift-ci smoke test checking what I broke |
|
@swift-ci please smoke test |
|
@swift-ci please smoke test macOS platform |
kavon
force-pushed
the
noncopyable-means-no-conformance
branch
from
2e11cca to
4dbfca7
Compare
|
@swift-ci please smoke test |
1 similar comment
|
@swift-ci please smoke test |
kavon
force-pushed
the
noncopyable-means-no-conformance
branch
3 times, most recently
from
2d83c52 to
c95d87d
Compare
slavapestov
reviewed
Oct 15, 2023
slavapestov
reviewed
Oct 15, 2023
slavapestov
reviewed
Oct 15, 2023
slavapestov
reviewed
Oct 15, 2023
slavapestov
reviewed
Oct 15, 2023
|
@swift-ci please smoke test |
kavon
force-pushed
the
noncopyable-means-no-conformance
branch
from
f6fd818 to
e49bf23
Compare
I've renamed the method to `TypeDecl::isNoncopyable`, because the query doesn't make sense for many other kinds of `ValueDecl`'s beyond the `TypeDecl`'s. In fact, it looks like no one was relying on that anyway. Thus, we now have a distinction where in Sema, you ask whether a `Type` or `TypeDecl` is "Noncopyable". But within SIL, we still preserve the notion of "move-only" since there is additionally the move-only type wrapper for types that otherwise support copying.
This implementation has the function execute a request to scan the inheritance clause of non-protocol nominals for a `~Copyable`. For protocols, we look in the requirement signature. This isn't our final state, as the GenericEnvironment needs to be queried in general to determine of a Type is noncopyable. So for now checking for a `~Copyable` only makes sense for Decls.
This visitor is generally useful for other kinds of marker protocols like `Copyable` that require structural checking over the storage of the struct or enum.
The generic signature for the built-in TupleDecl was incorrect, stating that the Element type must be Copyable. That in combination with an extension that says tuples are Copyable when Element is copyable was giving unconditional Copyable conformances for tuples that never actually get checked. Given that the existence of a conformance and no out-standing conditional requirements is now what is used to determine if a value is noncopyable, this was causing tuples with noncopyable elements to report that they are copyable in `isNoncopyable` requests.
We were allowing the catch-all "X is not compatible with generics yet" diagnostic to be emitted when there's a failure to satisfy a requirement for a conditional Copyable conformance. Instead, allow the existing infrastructure to diagnose such failures-to-conform to emit a diagnostic . This patch also adds a check to catch future catch-all diagnostics in asserts builds, since I'd like to phase out that diagnostic when NoncopyableGenerics is enabled.
Adds rudiementary support for searching `where` clauses for `~Copyable` constraints, but the implementation is known to have flaws.
kavon
force-pushed
the
noncopyable-means-no-conformance
branch
from
e49bf23 to
75d45fc
Compare
kavon
requested review from
ahoppen,
bnbarham,
CodaFi,
DougGregor,
rintaro,
hborla,
xedin,
AnthonyLatsis and
tshortli
as code owners
|
@swift-ci please test |
Turns out my caching was busted and `requiresProtocol` is fast enough to go without it.
Calling `mapTypeIntoContext` on n interface type with no type parameters is a no-op. In addition, we can avoid calling `getGenericEnvironmentOfContext` twice, which does a search of the DeclContext to find it.
kavon
force-pushed
the
noncopyable-means-no-conformance
branch
from
4b9a93f to
018ae15
Compare
|
@swift-ci please test |
Seems we already emit 6 duplicate diagnostics for this test, so what's one more? I'm not sure what change I made triggered this additional diagnostic.
|
@swift-ci please test and merge |
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.
More work that is a NFC unless using
-enable-experimental-feature NoncopyableGenericsThis change-set primarily overhauls the
isNoncopyablemethods ofTypeBaseandValueDecl:First, I've narrowed the scope of
ValueDecl::isNoncopyabletoTypeDecl::isNoncopyable. We are still definingTypeDecl::isNoncopyableas meaning "is always noncopyable" via checking for a~Copyablewritten on the decl.I'd eventually like this method to go away and/or be substantially changed into
decl->isNoncopyable()is rewritten asgetInterfaceType()->isNoncopyable()TypeDecl::implicitCopyableBehavior()returning one of{None, Conditional, Always}to reflect the different meanings of physically annotating the type with~Copyable.For
TypeBase::isNoncopyable, we now query whether the type conforms toCopyable. This change happens in coordination with actually synthesizing conformances toCopyablefor all nominal types when it makes sense to (i.e., it doesn't have~Copyablewritten on it, hence why the legacy-form ofTypeDecl::isNoncopyablelingers).