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

m!"{expr}" doesn't work as of 4.9.0-rc1 #4432

Closed
3 tasks done
llllvvuu opened this issue Jun 12, 2024 · 1 comment · Fixed by #4433
Closed
3 tasks done

m!"{expr}" doesn't work as of 4.9.0-rc1 #4432

llllvvuu opened this issue Jun 12, 2024 · 1 comment · Fixed by #4433
Labels
bug Something isn't working

Comments

@llllvvuu
Copy link
Contributor

llllvvuu commented Jun 12, 2024

Prerequisites

Please put an X between the brackets as you perform the following steps:

Description

As of 4.9.0-rc1, specifically the change to MessageData.ofExpr in 9f6bbfa, the following no longer works:

import Lean

def main : IO Unit := do
  let foo := Lean.mkConst `foo
  let bar ← m!"{foo}".toString
  IO.println s!"Expr: {foo}"
  IO.println s!"MessageData: {bar}"

#eval main

Feel free to close if this is intended behavior.

If not, MessageData.ofExpr (and similar functions) could be changed back to something like this:.

def ofExpr (e : Expr) : MessageData :=
  .ofLazy (fun ctx? => do
            let msg ← ofFormatWithInfos <$> match ctx? with
              | .none => pure (format (toString e))
              | .some ctx => ppExprWithInfos ctx e
            return Dynamic.mk msg)
          (fun mctx => instantiateMVarsCore mctx e |>.1.hasSyntheticSorry)

Context

Batteries' lake exe runLinter is now printing "(invalid MessageData.lazy, missing context)" which is showing up in Batteries and Mathlib CI.

leanprover-community/batteries#838

Steps to Reproduce

Try the above code in 4.9.0-rc1 vs 4.8.0.

Expected behavior: Prints "MessageData: foo"

Actual behavior: Prints "MessageData: (invalid MessageData.lazy, missing context)"

Versions

4.9.0-rc1

Impact

Add 👍 to issues you consider important. If others are impacted by this issue, please ask them to add 👍 to it.

@llllvvuu llllvvuu added the bug Something isn't working label Jun 12, 2024
@llllvvuu llllvvuu changed the title m!"{expr}" doesn't work m!"{expr}" doesn't work as of 4.9.0-rc1 Jun 12, 2024
@llllvvuu llllvvuu changed the title m!"{expr}" doesn't work as of 4.9.0-rc1 m!"{expr}" doesn't work as of 4.9.0-rc1 Jun 12, 2024
@nomeata
Copy link
Collaborator

nomeata commented Jun 12, 2024

I didn't expect much petty printing of expressions without a context, as how else would you show local variables. But for closed expressions and in particular global names it does make sense. Recovering the previous functionality in ofExpr is reasonable, do you want to prepare a PR for that?

github-merge-queue bot pushed a commit that referenced this issue Jun 14, 2024
This restores the behavior prior to
9f6bbfa
for `MessageData.ofSyntax` `MessageData.ofExpr`, and
`MessageData.ofLevel` while staying within the new `.ofLazy` paradigm.

Also adds some documentation to help developers understand the missing
context issue.

Closes #4432

---------

Co-authored-by: Joachim Breitner <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants