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: lean CLI option to print messages as JSON #3939

Merged
merged 6 commits into from
Apr 22, 2024

Conversation

tydeu
Copy link
Member

@tydeu tydeu commented Apr 17, 2024

Adds a --json option to the lean CLI. When used, the Lean frontend will print messages as JSON objects using the default ToJson encoding for the Message structure. This allows consumers (such as Lake) to handle Lean output in a more intelligent, well-structured way.

Message has been refactored into BaseMessage, Message, and SerialMessage to enable deriving ToJson/ FromJson instances automatically for BaseMessage / SerialMessage. SerialMessage is a Message with its MessageData eagerly serialized to a String.

@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 Apr 17, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/batteries that referenced this pull request Apr 17, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/mathlib4 that referenced this pull request Apr 17, 2024
@leanprover-community-mathlib4-bot leanprover-community-mathlib4-bot added the builds-mathlib CI has verified that Mathlib builds against this PR label Apr 17, 2024
@leanprover-community-mathlib4-bot
Copy link
Collaborator

leanprover-community-mathlib4-bot commented Apr 17, 2024

Mathlib CI status (docs):

leanprover-community-mathlib4-bot added a commit to leanprover-community/batteries that referenced this pull request Apr 17, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/mathlib4 that referenced this pull request Apr 17, 2024
@tydeu tydeu marked this pull request as ready for review April 17, 2024 23:18
@tydeu tydeu requested a review from Kha as a code owner April 17, 2024 23:18
@tydeu tydeu added the awaiting-review Waiting for someone to review the PR label Apr 17, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/batteries that referenced this pull request Apr 17, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/mathlib4 that referenced this pull request Apr 17, 2024
@tydeu tydeu added will-merge-soon …unless someone speaks up and removed awaiting-review Waiting for someone to review the PR labels Apr 18, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/batteries that referenced this pull request Apr 18, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/mathlib4 that referenced this pull request Apr 18, 2024
Comment on lines 239 to 242
@[inline] def toMessage (msg : SerialMessage) : Message :=
{msg with data := msg.data}

instance : Coe SerialMessage Message := ⟨SerialMessage.toMessage⟩
Copy link
Member

Choose a reason for hiding this comment

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

Is this used? Seems like a slightly strange direction

Copy link
Member Author

Choose a reason for hiding this comment

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

No, but this is potentially useful for Lean consumers of the JSON messages as they can report them back as normal messages. Since it sounds like we are unsure of this direction, I will remove the coercion.

return {msg with data := ← msg.data.toString}

protected def toString (msg : Message) (includeEndPos := false) : IO String := do
-- Remark: We inline here to avoid a copy in the case were `msg` is shared
Copy link
Member

Choose a reason for hiding this comment

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

I don't understand this comment. A copy of what? Should we care, what use case are we talking about that could make this relevant?

Copy link
Member Author

@tydeu tydeu Apr 19, 2024

Choose a reason for hiding this comment

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

If msg is not shared, Lean can just replace its data with its serialized version before applying toString. However, If the msg is shared, serializing it will require constructing (and allocating) and new BaseMessage with the same fields but serialized data (i.e., the "copy"). This will then be immediately deconstructed by the toString function. With the inline, we avoid this temporary allocation (as the serialization and destruction occur in the same function and thus Lean is smart enough to optimize it away).

leanprover-community-mathlib4-bot added a commit to leanprover-community/batteries that referenced this pull request Apr 19, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/mathlib4 that referenced this pull request Apr 19, 2024
@tydeu tydeu added this pull request to the merge queue Apr 22, 2024
@tydeu tydeu removed this pull request from the merge queue due to a manual request Apr 22, 2024
@tydeu tydeu added this pull request to the merge queue Apr 22, 2024
Merged via the queue into leanprover:master with commit d95e741 Apr 22, 2024
11 checks passed
@tydeu tydeu deleted the messagesAsJson branch April 24, 2024 09:35
github-merge-queue bot pushed a commit that referenced this pull request Sep 25, 2024
Fixes a mixed up between the parameter and global variable for
`json_output` the occurred during some name juggling in #3939.
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 will-merge-soon …unless someone speaks up
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants