Various issues with Key-Value Observing implementation

[triplef]

The KVO implementation in GNUstep is incorrect and incomplete in various regards, e.g.:

• the context pointer is used internally by GNUstep and can not be used by the user as intended by the API
• missing remove observer method with context parameter (see #323)
• missing support for dependent keys (+keyPathsForValuesAffectingX: methods)

We’ve been using the KVO implementation from WinObjC (1, 2, 3), which we worked on with Microsoft on stabilization and performance optimizations, and which we integrated in GNUstep. Since we’re using KVO extensively in our app I’m confident to say that the result (including some additional fixes not part of WinObjC) is an almost perfect re-implementation of KVO, including support for dependent key paths with transitive dependencies which is very complex and hard to get right. Performance is still slightly lower than Apple’s implementation but fairly good after spending considerable amount of time on optimizations.

I’d be happy to open a pull request with our integration (WinObjC is MIT-licenced), however the implementation is in Objective-C++ using C++ templates which are not easy to adapt to C.

I wanted to leave this here to at least document these issues. Please let me know if I should still open this PR and either (a) it might get accepted with Objective-C++ or (b) someone else would be willing to migrate the code to plain Objective-C.

[gcasa]

I think the determination of how to proceed with this should be made by @rfm. Although I am okay with the integration of MIT code, we would need to note that in our documentation. I think it's important for us to be as compatible as possible with the Apple implementation. Whether that means using the code you mention or fixing our own implementation.

[fredkiefer]

Hi Frederik,

I think the most important part that you may contribute are test cases. Just open up a merge request with plenty of not supported or failing test cases. That way we see which funtionality is missing or wrong. Of course addressing performance issues is a bit harder but these are a second step.

[gcasa]

Hi Frederik,

I think the most important part that you may contribute are test cases. Just open up a merge request with plenty of not supported or failing test cases. That way we see which funtionality is missing or wrong. Of course addressing performance issues is a bit harder but these are a second step.

100% agree.

[rfm]

Thanks for the offer. It sounds like it coudl be very useful, I agree that we really want ObjC code rather than C++, both because multiple languages in a codebase are a pain for maintainability, and because ObjC++ is also an issue for portability. Looking at the code would be good even if it's hard to translate.

As Fred says, what we most want are testcases, because I have absolutely no code using KVO (and I think that's similar for most other developers), so we need help understanding how it's intended to behave.

Greg has provided code to expose contexts (the remove method), which looks reasonable to me, but without testcases we don't know if it's really correct.

From what you say, that leaves support for dependent keys missing, but I don't really know what's needed for that (what the system is supposed to do), so help there would be very useful.