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: more than one optional argument can be omitted while pretty printing #4854

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

kmill
Copy link
Collaborator

@kmill kmill commented Jul 27, 2024

Before, the delaborator was conservative about omitting optional arguments, only omitting the very last one. Now it can omit arbitrarily long sequences of optional arguments from the end.

For simplicity of implementation, every optional argument is delaborated and then potentially discarded. It could save state and lazily delaborate, but we're running under the hypothesis that most optional arguments are for very simple values (like true, false, or a numeric literal), so it is unlikely that efficiency gains, if any, are worth it. In particular, in the future structure constructors will have optional arguments, but unexpandStructureInstance assumes none of the optional fields are omitted.

Closes #4812

…nting

Before, the delaborator was conservative about omitting optional arguments, only omitting the very last one. Now it can omit arbitrarily long sequences of optional arguments from the end.

For simplicity of implementation, every optional argument is delaborated and then potentially discarded. It could save state and lazily delaborate, but we're running under the hypothesis that most optional arguments are for very simple values (like `true`, `false`, or a numeric literal), so it is unlikely that efficiency gains, if any, are worth it. In particular, in the future structure constructors will have optional arguments, but `unexpandStructureInstance` assumes none of the optional fields are omitted.

Closes leanprover#4812
@github-actions github-actions bot added the toolchain-available A toolchain is available for this PR, at leanprover/lean4-pr-releases:pr-release-NNNN label Jul 27, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/batteries that referenced this pull request Jul 27, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/mathlib4 that referenced this pull request Jul 27, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/batteries that referenced this pull request Jul 27, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/mathlib4 that referenced this pull request Jul 27, 2024
@leanprover-community-mathlib4-bot
Copy link
Collaborator

leanprover-community-mathlib4-bot commented Jul 27, 2024

Mathlib CI status (docs):

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
builds-mathlib CI has verified that Mathlib builds against this PR toolchain-available A toolchain is available for this PR, at leanprover/lean4-pr-releases:pr-release-NNNN
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Printing of multiple optional argument
2 participants