Conversation
|
There are usually two changes needed when porting a file to the new Subtype: Instead of use Depending on the situation, this varies a little. In DDH_hybrid, I had to remove some extra cloning because a type was duplicated. In Poly, I had to change a |
|
I think that I am now convinced that we should allow some tag to forbid the instantiation of abstract types / operators / axioms. This would make this PR much more robust. I am ok to merge this PR as-is anyway and to change it once the aforementioned mechanism is implemented. |
|
This PR is clearly progress with regards to the existing As it comes to the incompatibilities, the @dominique-unruh Did you allow maintainers to push to the PR branch? If so, I volunteer to make a pass. |
|
@chdoc Yes, the PR needs a cleanup and thank you for proposing yourself! |
|
@chdoc Yes, I allowed maintainer edits. And I'd be happy if you could do a pass. (I'll have a look at the changes to get a feeling for the style.) |
Okay, I will make a pass. Can't promise that I will get to that today, will try though. |
|
First of all, thank you for adding some momentum to this. This solution doesn't interact well with instantiations. I'm surprised that there are no failures in the stdlib from this. The following code fails with While this case can be easily worked around by instantiating I could see this being fixed by unifying the |
|
I made a pass. I called the partial inverse @oskgo This looks like an opacity/conversion issue. After all, the theory |
|
Perhaps a more realistic example: @chdoc Yes, there's also something weird going on here, but I don't think it makes much of a difference. If we can unify the Of course there is always the alternative to not use subtypes until the top level, and just axiomatize everything instead (or come up with some other heuristic that prevents diamond dependencies with subtypes), but this isn't ideal either. Does this mean that EDIT: By the way, I think it's totally fair to say that diamond dependencies just have to deal with it for now and do some hack instead, possibly involving the |
|
@oskgo That being said, maybe the following approach could help in situations as the one you describe. We keep the I'd be happy add this but we should agree what the names should be. ( |
|
@dominique-unruh It is a general problem of the cloning mechanism, but as far as I can tell it only applies to inductive type definitions right now. EasyCrypt lets you instantiate theories with equal code with each other except if they define an inductively defined type. This is part of why your subtype theory works. Only issue is that many abstract theories don't define inductive types, even if they use subtypes, and this change is a regression for those. Keeping the old subtype theory around and using that for abstract theories sounds good. |
I admit I don't fully understand this, but are you saying there would be less compatibility problems if the new instead of ? Because I don't think the inductive type here isn't really necessary, I think. (I put it in so that one cannot accidentally use |
|
Alternatively, we could classify the fact that theory instantiation doesn't allow merging identical inductive type definitions as a bug, and fix that first. (Or independently.) @chdoc can i leave you to lead discussions here and make sure that any blockers (or even just sources of unnecessary complexity) that can be construed as EC bugs are raised and discussed in the Dev meeting? |
|
@dominique-unruh The following code might help with understanding: Making |
I can confirm that making this change makes the example from @oskgo check without problem. (@oskgo beat me to it, because I was tinkering with other cloning variants)
A-priori I see nothing wrong with this, but we should be careful. From memory, I thought that constructor names - unlike modules/procedures - had to be globally unique. However, this seems to (no longer?) be the case: @strub Was there a (recent) change concerning this?
I will try. |
|
Here is one way that might work (and maintain a separation between Logic.ec: Subtype.eca: Side note: one way people find lemmas is by printing theories. And printing (instances of) the new |
|
@chdoc If you think that adding a wrapper type to the stdlib isn't too much "pollution" of the stdlib, I'd be happy to use that solution.
Maybe we rename the naughty type into |
How about Anyway, this is really just a temporary stopgap measure until we get something along the lines of: that excludes types from instantiation. |
|
I liked the idea of having |
I agree but for the following reasons I would keep it in the less scary form:
On the long run, I'd like to: get rid of this type anyway (via |
Note that for this to work |
|
As I see, @chdoc already renamed the type. So from my side, all is set? Should I do anything else or is this ready for merge?
Probably, so that will be something to keep an eye on when the |
True, and this actually worries me a bit. If we had a proper |
|
@fdupress How worried are you about the impact of this change on other projects that are not (yet) in our CI? |
It's going to be a problem for any use of It's just something to keep in mind for the Glad we could get diamond cloning working. I have been wanting movement towards |
|
Yes, the (potential) issues with It's just a data point that maybe, rather than having cloning restrictions for abstract theories, we just want a (primitive) Waiting for @alleystoughton and @fdupress to respond, but this looks good to me. |
Not worried about the ones I have sight of: very little exotic (as in, not also done in stdlib or examples) use of subtypes. Kyber and Dilithium most likely to go wonky: I'll give them a check tomorrow and perhaps pull them and the rest into the external CI. (Kyber and Dilithium are checked with expected cost, so I'll still need to run a manual check.) |
|
Seems fine to me! |
|
Looking at Dilithium, I'm not too worried. there are only two uses of Subtype, one of them will require the "default fix". In fact, Kyber and Dilithium both use quotients, which - for similar reasons as for subtypes - come with an unprovable axiom. So maybe this should receive a similar treatment or be implemented using subtypes+choice. |
Yes. That's a defect of the cloning mechanism for types. We should allow merging identical induction type. Let's fix that and we can merge this PR as is. |
|
Note only that I started implementing a proper native "subtype" mechanism -- with value dependency. So I expect the Subtype theory to be pruned at some point. |
A replacement for
Subtypethat permits usingproof *without impossible proof obligations.Partly resolves #275.
See #275 (comment) for some more explanations.