Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Uniquify all Sprite preterms in separate step prior elaboration #199

Closed
wants to merge 3 commits into from

Conversation

janvrany
Copy link
Collaborator

Yet another attempt to fix #139.

This PR α-renames all variables in a separate step prior elaboration.
It is similar to PR #197 but differ in several ways:

  • perform renaming as a separate step, not as part of [Chk-Lam].
  • renames all lets, not just function parameter binds
  • renames everything in single top-down traversal.

@shingarov
Copy link
Owner

I wonder if this can be somehow integrated with AlphaRenamer. Which leads us to further zoom in attention to things like #194 , which is just one manifestation of a more general issue, that in the upstream Sprite, there is a hierarchy of Exprs parallel to that in Fixpoint. For example, there are EVar and EApp etc in Fixpoint, and there are EVar and EApp etc in Sprite (and in the Sprite namespace the former are called F.EVar etc because of import qualified Language.Fixpoint.Types as F). Actually it's even worse: in Fixpoint itself there is duplication between the "fixpoint names" (F.*) and "horn names" (H.*). In addition to datatype names, there is similar duplication of function names.

In Smalltalk, most of the time we follow the convention that if some a is duplicated in Sprite and Fixpoint, the Fixpoint thing (F.a) is called a in Smalltalk, and the Sprite a is called Λa in Smalltalk. See, for example, Object>>predReft, or SInfo>>eliminatingSolverInfo.

But sometimes it's too unbearable to lug all this parallelism through the whole codebase. There is, for example, PInt ("literal integer constant"). I just don't bother distinguishing between the Sprite and the Fixpoint PInt. One day a hero will be born who will clean all this mess.

@janvrany
Copy link
Collaborator Author

janvrany commented Feb 22, 2024

I wonder if this can be somehow integrated with AlphaRenamer.

I was thinking the same but after digging deeper I decided not to, not now anyways. It seemed more than I could chew.

One day a hero will be born who will clean all this mess.

Yeah, and hopefully soon else a mere hero won't suffice. And gods are even scarcer to come by than heroes.

Both MA (prior this commit) and upstream Sprite (at the time of this
commit) had bug in type renaming where parameters were renamed but
not references to them in termination metric. All tests "appeared to
work" simply because the code used same (formal) parameter names in both
type signature and function declaration.

However in general case they do not need to be the same. Solution is
simple - also rename parameters in termination metric.
@janvrany janvrany marked this pull request as ready for review February 22, 2024 22:09
This commit α-renames all variables in a separate step prior elaboration
in order to avoid name clashes as demonstrated by `#testAlphaClash`.

This is similar to PR shingarov#197 but differ in several ways:

 * perform renaming as a separate step, not as part of [Chk-Lam].
 * renames all lets, not just function parameter binds
 * renames everything in single top-down traversal.

Fixes shingarov#139 (well, not quite yet)
@janvrany
Copy link
Collaborator Author

Closing as resolved via 3ac081f and #283

@janvrany janvrany closed this May 22, 2024
@janvrany janvrany deleted the pr/fix-139-v2 branch October 21, 2024 12:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

A bug in the α-renamer causes unsound "SAFE" judgement
2 participants