Propagate non-optional name to typed ast #374
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
n-osborne
force-pushed
the
invariant-bis
branch
from
8596145
to
58c2cd4
Compare
n-osborne
force-pushed
the
invariant-bis
branch
2 times, most recently
from
80ee942
to
1cd90f4
Compare
shym
approved these changes
Jan 10, 2024
There was a problem hiding this comment.
I have a question and a suggestion.
- That seems a very natural part of the changes started with Make
withnecessary when declaring type invariants. #372. Was there a reason we didn’t do it then? In particular, do we expect to break many things with that change? (The commit message could provide a bit of such context) - I would suggest merging
print_invariant_vsintoprint_invariant; and, as part of reviewing old but moved code, I would have expected to seenewlinealso insepor is that implicitly done somewhere else?
Apart from those small remarks, this looks very good!
n-osborne
force-pushed
the
invariant-bis
branch
from
1cd90f4
to
bbe2b5f
Compare
|
Thanks for the review :-) |
n-osborne
force-pushed
the
invariant-bis
branch
2 times, most recently
from
38f14a4
to
86502ee
Compare
This commit completes the changes made in a previous PR (ocaml-gospel#372). It contains the actual change in the typed ast and hence is expected to break tools relying on the changed parts.
n-osborne
force-pushed
the
invariant-bis
branch
from
86502ee
to
df0a104
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR completes #372.
It contains a breaking change in the typed AST.