links-lang / links

Links: Linking Theory to Practice for the Web
http://www.links-lang.org
Other
332 stars 42 forks source link

Fix assert error with relational lenses #1143

Closed SimonJF closed 2 years ago

SimonJF commented 2 years ago

The Lens type traversal was unimplemented and filled in with an assert false. As a result, all RL code fails.

I don't think there is any really sensible default traversal due to the complexity of the Lens types, so I have just filled it in with the identity. This doesn't stop someone implementing a traversal -- they'll just need to write one for Lens.Type.t type and plug it in as usual.

jamescheney commented 2 years ago

Yes. I have been meaning to review and refactor the lens subsystem at some point before my memories (and ability to armtwist @rudihorn) fade. We had been holding off in the hope of figuring out what a good general-purpose language extensibility capability that could express lenses would look like but didn't get there (Rudi's thesis has a chapter showing how to embed lenses in Haskell though, so if we just added type classes, families and GADTs then I think we would be mostly there.) This should be recorded as an enhancement issue.

SimonJF commented 2 years ago

Thanks for the quick merge!

For the avoidance of doubt, I think splitting a lot of the lens code was a reasonable decision at the time and in keeping with best practices if we wanted a fully modular design for Links extensions. I just don't think this is the direction we ended up going for other extensions, so I agree that it's probably better to merge into the main codebase at some point.

My point was mainly that the nature of Lens types (and 'sorts', in RL terminology) is sufficiently different from Links types (since it needs to track, for example, predicates) that it's not appropriate to extend the general Types visitor to support lens sorts. Rather, it's reasonable to expect someone wanting to do a lens-specific type traversal to implement their own visitor over lens sorts for that purpose.