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

add proof of decEq x x = Yes Refl #3905

Merged
merged 2 commits into from
Jul 12, 2017

Conversation

clayrat
Copy link
Contributor

@clayrat clayrat commented Jul 9, 2017

No description provided.


||| Everything is decidably equal to itself
total decEqSelfIsYes : DecEq a => {x : a} -> decEq x x = Yes Refl
decEqSelfIsYes {x} with (decEq x x)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why should we have this in the prelude? Has it come up in practice for you? I imagine it's not so useful, because if I already know that something is the same as another, then I can just use Refl directly rather than calling decEq on it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm doing an Idris translation of "Software foundations" and this has already come up twice when trying to prove things about functions defined via decEq. Typically you get to the point where the arguments are the same and then there's an error that smth like Decidable.Equality.Nat implementation of Decidable.Equality.DecEq, method decEq n n could not be reduced to get to the goal. Doing rewrite decEqSelfIsYes {x=n} solves this.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, that makes sense. It would be useful for this kind of context to be part of the PR message in the future.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, I'll try to be more informative in the future!

@melted
Copy link
Contributor

melted commented Jul 10, 2017 via email

@ahmadsalim
Copy link

I think @melted is hinting that base or contrib are more appropriate for this function.

@clayrat
Copy link
Contributor Author

clayrat commented Jul 10, 2017

My reasoning is that since decEq is in Prelude, being able to prove things about it is sorta fundamental. It would be nice if we didn't need this lemma, but I couldn't find another way. If you think this is too much of a hack, I can close the PR; or, if you agree this is useful, I can move it to base/contrib - just tell me which is it :)

@david-christiansen
Copy link
Contributor

Given that this is something that has come up in practice, I think it's OK in the Prelude along with DecEq. If it's there, then :search and :apropos can find it, while putting it in base or contrib seems likely to ensure that it's never found by anyone but the author.

But for a Prelude change, we also need a CHANGELOG entry. Then I'm happy.

@clayrat
Copy link
Contributor Author

clayrat commented Jul 12, 2017

Done.

@david-christiansen
Copy link
Contributor

I'm happy with a merge, but I don't want to do it unilaterally - it sounds like @melted has concerns about library bloat and like @ahmadsalim may as well. What do the two of you think, given that it has been useful?

@melted
Copy link
Contributor

melted commented Jul 12, 2017 via email

@ahmadsalim
Copy link

Yeah, LGTM.

@david-christiansen
Copy link
Contributor

@melted This would indeed be good to fix, and it's an interesting thing to do research on. Referring to "proofy" things by their propositional content rather than their name might be one approach. But we're not there yet.

@david-christiansen
Copy link
Contributor

Thanks, @clayrat!

@david-christiansen david-christiansen merged commit 05fbd11 into idris-lang:master Jul 12, 2017
@ahmadsalim
Copy link

@david-christiansen I am not sure I understand, how do :search and :apropos work today, and why would they not work with base and contrib

@david-christiansen
Copy link
Contributor

They only search what's imported.

@melted
Copy link
Contributor

melted commented Jul 13, 2017

@david-christiansen don't they search what's loaded? Base is losded but not imported.

@david-christiansen
Copy link
Contributor

There's an optional parameter to get them to do that, but it makes them a lot slower and doesn't get used a lot in practice.

@justjoheinz
Copy link
Contributor

justjoheinz commented Jul 13, 2017 via email

@jfdm
Copy link
Contributor

jfdm commented Jul 13, 2017

@justjoheinz the syntax is:

:search (<comma sep. list of packages>) <expr>

For example:

:search (foo,bar,foobar) Int -> Int

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.

6 participants