[Sema] Fix derived conformances crasher. #21785
Conversation
Fix crasher regarding bad interaction between `AdditiveArithmetic` and `Differentiable` derived conformances, where both code paths attempt to synthesize memberwise initializers. Move `getMemberwiseInitializer` to a shared location.
|
@swift-ci Please test tensorflow |
| // - This ad-hoc logic accepts empty struct constructors generated via | ||
| // `TypeChecker::defineDefaultConstructor`. Alternatively, | ||
| // `defaultDefaultConstructor` could be edited to mark empty | ||
| // constructors as memberwise initializers. |
There was a problem hiding this comment.
It seems cleaner then to say that the default constructor is a memberwise init in this case. Also, this could be a method on NominalTypeDecl instead of a top-level function.
There was a problem hiding this comment.
Yes, both of those points make sense. I was hesitant to make those two changes (editing NominalTypeDecl and defineDefaultConstructor) and diverging from master.
What do you recommend? I can make those two changes, mark with SWIFT_ENABLE_TENSORFLOW, and upstream to master later (but without any tests).
There was a problem hiding this comment.
I think the fact that the default init will become a memberwise init rather than have a body can be observed with a -dump-ast or -emit-silgen rest. If this helps you proceed I don’t see any problem upstreamg it now
There was a problem hiding this comment.
I'm sorry, could you please be specific what changes you'd like to see? I'm a bit hazy so that'd be much appreciated!
Are you suggesting that the logic in getMemberwiseInitializer (introduced by this PR) be changed so that default constructors are also considered memberwise initializers? Or that defaultDefaultConstructor should mark all returned constructors as memberwise initializers?
Some observations:
- The derived conformances logic assumes that
getMemberwiseInitializerreturns a constructor whose parameter count is equal to the number of stored properties in the nominal type. Marking default constructors (which take zero parameters) as memberwise initializers changes this contract and would complicate caller code. - There doesn't exist a flag like
isMemberwiseInitializer()for default initializers. We could check other conditions likeisSynthesized(), but those aren't necessarily accurate (other initializers could be synthesized).
|
Subsumed by #21871. |
Fix crasher regarding bad interaction between
AdditiveArithmeticandDifferentiablederived conformances, where both code paths attempt tosynthesize memberwise initializers.
Move
getMemberwiseInitializerto a shared location.