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

draft: refinement to #2818 #2823

Closed
wants to merge 5 commits into from
Closed

Conversation

nomeata
Copy link
Collaborator

@nomeata nomeata commented Nov 5, 2023

Don't look here, I suggest to disucss at #2818.

I just want separate CI, will be merged into #2818 if it works well.

This didn't work before
```
def f (n : Nat) : Nat :=
  match n with
  | 0 => 0
  | n + 1 => (f) n
```
because the `RecApp` metadata marker gets in the way. More practically
relevant, such code is to be produced when using `rw` or `simp` in
recursive theorems (see included test case).

We can fix this by preprocessing the definitions and floating the
`.mdata` marker out of applications.

For structural recursion, there already exists a `preprocess` function;
this now also floats out `.mdata` markers.

For well-founded recursion, this introduces an analogous `preprocess`
function.

Fixes #2810.

One test case output changes: With the `.mdata` out of the way, we get a
different error message. Seems fine.

Alternative approaches are:

* Leaving the `.mdata` marker where it is, and looking around it.
  Tried in #2813, but not nice (many many places where `withApp` etc.
  need to be adjusted).
* Moving the `.mdata` _inside_ the application, so that `withApp` still
  works. Tried in #2814. Also not nice, the invariant that the `.mdata`
  is around the `.const` is tedious to maintain.
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.

1 participant