search

tweag / pirouette

Language-generic workbench for building static analysis
MIT License
47 stars 2 forks source link

Remove expandAllNonRec #145

Closed 0xd34df00d closed 2 years ago

0xd34df00d commented 2 years ago

It's not used elsewhere for quite some time, and it also has no tests/spec.

I was going through the transformations updating the comments, cleaning some things up, etc, and was curious whether we want to keep things we don't use anywhere, or whether we better remove them (git remembers everything after all!).

Making this as a separate PR, so it could be rejected easily in case you don't think it's useful!

mmontin commented 2 years ago

I think it's useful to remove whatever is no longer used. There's already a lot to get into if one wants to get used to Pirouette so there's definitely value in trimming dead code. I'm curious though why this code is no longer used and what it was used for before.

VictorCMiraldo commented 2 years ago

This makes sense! In fact, if pirouette were to become really language agnostic at some point, we can't have a expandAllNonRec that fixes a specific reduction semantics. Is it call-by-value, call-by-need or call-by-name? On that topic, I actually think we should refactor elimEvenOddMutRec in two parts: one which separates the mutually-recursive classes and another which expands terms.

To answer @mmontin, we had expandAllNonRec from back in the day when TLA+ was the target, it wasn't particularly important at any point tbh, given that it doesn't really make a difference and it has no longer been in use for a while now. Hence, +1 for removing it.