[Type checker] Finalize referenced members of class extensions. #18199
Conversation
When we reference a member of a class extension, we need to compute its overrides, @objc bit, and ‘dynamic’ status because SILGen may rely on them. Do that consistently by adding such members to the list of declarations to “finalize”. Rework the finalization logic to handle the computation of each of those bits, and never remove a finalized declaration from the set: rather, process new declarations as they enter the set, to avoid looping or extra state bits. Fixes rdar://problem/42440686.
|
@swift-ci please smoke test |
|
@swift-ci please test source compatibility |
Due to some short-circuiting in the serialization logic, we never added @objc getters/setters to a module’s Objective-C method table.
We handle layout through finalization of the declaration, which already has its own approach to eliminating redundant work. We don’t need an extra bit getting in the way.
|
@swift-ci please test source compatibility |
1 similar comment
|
@swift-ci please test source compatibility |
| @available(OSX, introduced: 10.52) | ||
| func takes10_52Introduced10_52(o: OtherIntroduced10_52) { | ||
| } | ||
|
|
||
| // expected-error@+1{{'OtherIntroduced10_52' is only available on OS X 10.52 or newer}} | ||
| var propOf10_52: OtherIntroduced10_52 = |
There was a problem hiding this comment.
This would be a regression. Maybe we should skip finalizing declarations if any errors have been emitted?
There was a problem hiding this comment.
We had that, and I removed it, because it suppresses a ton of unrelated errors.
There was a problem hiding this comment.
I mean, this way we're doing unnecessary work in other files.
There was a problem hiding this comment.
We're finalizing those declarations because something needed them from the other file. In this case, they're methods of a class so we are computing the layout. Prior to my change we would skip a little work once we had produced an error, but "stop doing any work if there's any error anywhere" doesn't seem like the right default policy.
There was a problem hiding this comment.
It might be more than a little work, though—it's all structs and enums you've used throughout the file, transitively. It seems like the same reasoning to me as "don't go on to SILGen if there are errors, even though you could still get some dataflow diagnostics".
There was a problem hiding this comment.
I have an idea. Once we've encountered an error, we'll stop finalizing declarations from other source files. That way, you'll still get diagnostics in your own source file, but we won't do unnecessary work elsewhere. PR is up at #18259.
There was a problem hiding this comment.
That makes a lot of sense! I forgot that finalization is now the only way to get certain diagnostics from decls within the current file.
Whenever we visit a declaration via the DeclChecker, add it to the list of declarations to finalize. This makes sure that we can centralize the notion of “finalize for SILGen” and that it will be called for everything in the source file being processed.
DougGregor
force-pushed
the
finalize-class-extension-members
branch
from
994e69b to
6a8d321
Compare
|
@swift-ci please test source compatibility |
|
@swift-ci please smoke test |
When we reference a member of a class extension, we need to
compute its overrides, @objc bit, and ‘dynamic’ status because SILGen
may rely on them. Do that consistently by adding such members to the
list of declarations to “finalize”.
Rework the finalization logic to handle the computation of each of those
bits, and never remove a finalized declaration from the set: rather,
process new declarations as they enter the set, to avoid looping or
extra state bits.
Fixes rdar://problem/42440686.