Skip to content

Make Instance without a body always open a proof #9274

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

Merged
merged 1 commit into from
Jan 29, 2019

Conversation

maximedenes
Copy link
Member

This is a follow-up on #9270. It makes Instance without a body always open a proof.

The standard library was easy to fix. The only remaining open problem is how to make backward compatible fixes on downstream devs. This will probably require introducing a deprecated compatibility option.

Based on #9270 and #9273.

@maximedenes
Copy link
Member Author

After this PR, the status of Instance (without Refine Instance Mode) is clear: with a body, it never opens a proof. Without, always.

@SkySkimmer
Copy link
Contributor

Instead of this I would rather replace (if List.is_empty k.cl_props then Some (Inl subst) else None) by constant None. Essentially Instance is replacing on the fly no body with := {} if the class was declared as a trivial class, if we want no-body Instance to always open a proof we shouldn't do that.
Also I would rewrite Instance whatever : Class. to Instance whatever : Class := {}..

@maximedenes
Copy link
Member Author

@SkySkimmer I think you are reading the code incorrectly. Look at the diff, it is about much more than trivial classes.

The change you suggest breaks much more, as it changes the semantics of Instance foo : C.

So I prefer the current version, where the only thing we do is to make Instance foo : C always open a proof (even if solved automatically), without changing the semantics.

@maximedenes maximedenes changed the title Instance nobody open proof Make Instance without a body always open a proof Dec 22, 2018
@SkySkimmer
Copy link
Contributor

Look at the diff, it is about much more than trivial classes.

I don't know stdlib classes enough to tell, can you explain more?

The change you suggest breaks much more, as it changes the semantics

Isn't changing the semantics the point?

@maximedenes
Copy link
Member Author

I don't know stdlib classes enough to tell, can you explain more?

In fact, I was wrong, they are all trivial. It is just that we unbundle a lot of stuff.

I'll try your change, as it may be a more backward compatible way of doing things. Thanks!

@maximedenes maximedenes force-pushed the instance-nobody-open-proof branch from 83c66a9 to af7b776 Compare January 10, 2019 16:49
@maximedenes maximedenes force-pushed the instance-nobody-open-proof branch from af7b776 to 09e50ba Compare January 23, 2019 17:10
maximedenes added a commit to maximedenes/bignums that referenced this pull request Jan 23, 2019
maximedenes added a commit to maximedenes/math-classes that referenced this pull request Jan 23, 2019
maximedenes added a commit to maximedenes/corn that referenced this pull request Jan 23, 2019
maximedenes added a commit to maximedenes/coq-ext-lib that referenced this pull request Jan 24, 2019
maximedenes added a commit to maximedenes/fiat-crypto that referenced this pull request Jan 24, 2019
maximedenes added a commit to maximedenes/QuickChick that referenced this pull request Jan 24, 2019
maximedenes added a commit to maximedenes/GeoCoq that referenced this pull request Jan 24, 2019
maximedenes added a commit to maximedenes/VST that referenced this pull request Jan 24, 2019
maximedenes added a commit to maximedenes/HoTT that referenced this pull request Jan 24, 2019
maximedenes added a commit to maximedenes/fiat-crypto that referenced this pull request Jan 24, 2019
@maximedenes maximedenes force-pushed the instance-nobody-open-proof branch from 09e50ba to 309cf3d Compare January 24, 2019 15:46
JasonGross pushed a commit to mit-plv/fiat that referenced this pull request Jan 24, 2019
JasonGross pushed a commit to mit-plv/fiat-crypto that referenced this pull request Jan 24, 2019
Copy link
Member

@ppedrot ppedrot left a comment

Choose a reason for hiding this comment

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

LGTM. It looks like it needs many overlays though.

@maximedenes
Copy link
Member Author

LGTM. It looks like it needs many overlays though.

Not really, as all the patches I submitted are backward compatible, so they can be integrated already.

JasonGross pushed a commit to mit-plv/fiat-crypto that referenced this pull request Jan 26, 2019
Copy link
Member

@gares gares left a comment

Choose a reason for hiding this comment

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

finally!

Copy link
Member

@mattam82 mattam82 left a comment

Choose a reason for hiding this comment

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

Congrats!

@gares gares self-assigned this Jan 29, 2019
@gares gares added the kind: redesign The same functionality is being re-implemented in a different way. label Jan 29, 2019
@gares gares added this to the 8.10+beta1 milestone Jan 29, 2019
@gares gares merged commit 309cf3d into rocq-prover:master Jan 29, 2019
gares added a commit that referenced this pull request Jan 29, 2019
Reviewed-by: gares
Reviewed-by: mattam82
Reviewed-by: ppedrot
@maximedenes maximedenes deleted the instance-nobody-open-proof branch February 5, 2019 09:48
erikmd added a commit to erikmd/CoqEAL that referenced this pull request Feb 11, 2019
* Add "… := {}" for trivial Instances (cf. rocq-prover/rocq#9274)

* Rename a few bound variables to avoid clash with section variables.
SkySkimmer pushed a commit to SkySkimmer/lambda-rust that referenced this pull request Feb 12, 2019
amintimany pushed a commit to amintimany/iris that referenced this pull request Jun 13, 2019
SkySkimmer pushed a commit to SkySkimmer/stdpp that referenced this pull request Oct 15, 2020
thery pushed a commit to thery/bignums that referenced this pull request Oct 4, 2021
Boutry pushed a commit to GeoCoq/GeoCoq that referenced this pull request Mar 10, 2024
Boutry pushed a commit to GeoCoq/GeoCoq that referenced this pull request Mar 10, 2024
Boutry pushed a commit to GeoCoq/GeoCoq that referenced this pull request Mar 10, 2024
Boutry pushed a commit to GeoCoq/GeoCoq that referenced this pull request Mar 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind: redesign The same functionality is being re-implemented in a different way.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants