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

feat(logic): make some decidability proofs [inline] #1393

Merged
merged 5 commits into from
Sep 11, 2019
Merged

feat(logic): make some decidability proofs [inline] #1393

merged 5 commits into from
Sep 11, 2019

Conversation

fpvandoorn
Copy link
Member

@fpvandoorn fpvandoorn requested a review from a team as a code owner September 3, 2019 17:33
@semorrison
Copy link
Collaborator

Perhaps we should add a test that demonstrates that inlining is working?

It may be hard to do this without generating output, I'm not sure.

@khoek
Copy link
Collaborator

khoek commented Sep 9, 2019

A little bit crazy, but the test could have (in the expression a and b) a just evaluate to true, and b create a file called sadness.txt or something, and then the test could be decided by whether the file exists at the end.

@semorrison
Copy link
Collaborator

semorrison commented Sep 9, 2019 via email

@khoek
Copy link
Collaborator

khoek commented Sep 9, 2019

Not to fear, @semorrison, the following line suffices as a test

#eval if ff ∧ (@undefined_core bool "uh-oh") then tt else tt

It fails without the inlining.

@khoek
Copy link
Collaborator

khoek commented Sep 9, 2019

I mean, I guess we could also just check whether those defns have the inline attribute...

But I've pushed a test for and and or which is solution-agnostic

@khoek
Copy link
Collaborator

khoek commented Sep 9, 2019

All of the other logical connectives don't eagerly evaluate (or there is no notion of such), as far as I could see (the only interesting case is implies I think, and it was fine)

@fpvandoorn
Copy link
Member Author

fpvandoorn commented Sep 10, 2019

Travis successfully checked that your test fails without inlining. :)
Now I added logic/basic as an import.

EDIT: Thanks for adding a test, that is useful to have indeed.

If Travis succeeds now: I think this PR is good to go.

@robertylewis robertylewis added the ready-to-merge All that is left is for bors to build and merge this PR. (Remember you need to say `bors r+`.) label Sep 10, 2019
@mergify mergify bot merged commit 45fe081 into master Sep 11, 2019
@mergify mergify bot deleted the inline branch September 11, 2019 01:49
anrddh pushed a commit to anrddh/mathlib that referenced this pull request May 15, 2020
…nity#1393)

* feat(logic): make some decidability proofs [inline]

* inline more decidability proofs

* test

* import logic.basic in test
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-to-merge All that is left is for bors to build and merge this PR. (Remember you need to say `bors r+`.)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants