SR-2673: @NSManaged property can't satisfy protocol requirement #5141
Conversation
Targeted fix for SR-2673: if a potential protocol witness is @NSManaged, add accessors the NSManaged way, not the stored property way. There's probably more weirdness around here, so I'll clone the bug to go through maybeAddAccessorsToVariable a lot more often. (Lazy properties could easily be broken in the same way.)
|
@swift-ci Please test macOS platform |
|
@swift-ci Please smoke test Linux platform |
|
@DougGregor or @jckarter, can you review this for the swift-3.0-branch? (Since it is a 3.0 regression.) |
|
What's the right fix long-term? It seems like maybeAddAccessorsToVariable() and addTrivialAccessorsToStorage() should be one and the same. |
|
Yep. Right now maybeAddAccessorsToVariable is a little too smart, though, and I don't feel comfortable cherry-picking changes to that method to the 3.0 branch. |
|
(The current method synthesizes accessors for implicit decls and for imported decls; if we don't want that behavior we should think about all the places we're using it.) |
|
In 2.2 I changed things so that all stored properties get synthesized accessors. Do you mind cleaning this up in master, and figuring out the right behavior in general? |
|
I'm happy to do that later. This is a targeted fix for Swift 3. (I made the clone, SR-2874.) |
|
Merging in order to kick off the 3.0 build soon, but still waiting on review. (Unless Slava is signing off on it?) |
…tlang#5141) Targeted fix for SR-2673: if a potential protocol witness is @NSManaged, add accessors the NSManaged way, not the stored property way. There's probably more weirdness around here, so I'll clone the bug to go through maybeAddAccessorsToVariable a lot more often. (Lazy properties could easily be broken in the same way.)
|
I'm okay with this targeted fix for Swift 3, but we definitely should clean this up on master. |
|
LGTM as a quick fix for 3 as well. |
4 participants
Targeted fix for SR-2673: if a potential protocol witness is
@NSManaged, add accessors the NSManaged way, not the stored property way.There's probably more weirdness around here, so I'll clone the bug to go through maybeAddAccessorsToVariable a lot more often. (Lazy properties could easily be broken in the same way.)