[Sema] Requestify inheritsSuperclassInitializers #27936
Conversation
This makes sure we don't attempt to synthesize a memberwise or default initializer for an invalid decl, or one in a module interface.
This commit introduces a request for computing whether a class inherits both designated and convenience initializers from its superclass. The shared logic of finding initializers which the subclass hasn't overriden has been factored out into `collectNonOveriddenSuperclassInits`.
Now that the computation of `inheritsSuperclassInitializers` has been split off into a request, we can avoid calling `addImplicitInheritedConstructorsToClass` for clang decls.
This commit removes some code that's no longer needed. In addition, now that we've requestified `inheritsSuperclassInitializers`, we can directly diagnose on non-inherited required convenience inits within the loop.
|
@swift-ci please test |
|
@swift-ci please test OS X platform |
|
@swift-ci please test source compatibility |
|
@swift-ci please test source compatibility release |
| } | ||
| for (auto member : decl->getMembers()) { | ||
| if (auto ctor = dyn_cast<ConstructorDecl>(member)) { | ||
| if (ctor->isRecursiveValidation()) |
There was a problem hiding this comment.
@slavapestov Is there a reason we need any of this as a preflight? It seems bogus to me.
There was a problem hiding this comment.
I did try removing it, and it passed the validation test suite, I just wasn't sure whether or not it might cause us to start emitting circularity diagnostics for code that we previously silently accepted, so thought I'd leave it for a future PR.
I'd be more than happy to remove it now if you and Slava don't think it'd be a problem :)
| // expected-error@-2 {{circular reference}} | ||
| // expected-note@-3 {{through reference here}} | ||
|
|
||
| init(x: Int) {} // expected-error {{circular reference}} |
There was a problem hiding this comment.
If you wanted to emit a better diagnostic, you could override diagnoseCycle and try to detect this case. It's probably not worth the extra hassle in this patch.
See also rdar://55263708
There was a problem hiding this comment.
Yeah, I talked with Slava about this – the goal is to eventually have a principled way to avoid these kinds of duplicate cycle diagnostics. For now though, I think it's okay.
Thanks for the review!
Continue to cache the InheritsSuperclassInits bit on the AST.
|
@swift-ci please test |
|
Build failed |
|
Build failed |
|
@CodaFi If it's good with you, I'll go ahead and merge this now and look into removing the |
|
@swift-ci please smoke test |
|
@hamishknight Merge away! But please squash this so the merge commit doesn't show up in the history. |
|
@swift-ci please smoke test Linux platform |
|
Looks like you picked up the SourceKit failure @swift-ci please smoke test Linux platform |
|
@swift-ci please smoke test Linux platform |
This PR splits off the computation of whether a class inherits its superclass' designated and convenience inits into a request. This leaves
addImplicitInheritedConstructorsToClassjust with the job of synthesizing designated init overrides.