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

Incorrect metadata on pretty-printed signature of String.append #4972

Closed
david-christiansen opened this issue Aug 9, 2024 · 2 comments · Fixed by #5757
Closed

Incorrect metadata on pretty-printed signature of String.append #4972

david-christiansen opened this issue Aug 9, 2024 · 2 comments · Fixed by #5757
Labels
bug Something isn't working P-medium We may work on this issue if we find the time

Comments

@david-christiansen
Copy link
Contributor

Description

The metadata that's attached to the pretty-printed representation of String.append's signature is incorrect for one of its argument types.

In particular, hovering the second argument's type gives much less information than the first argument's type, and "go to def" doesn't work on it.

Context

[Broader context that the issue occured in. If there was any prior discussion on the Lean Zulip, link it here as well.]

Steps to Reproduce

  1. Type #check String.append
  2. In the infoview, check the hovers associated with each argument

Expected behavior:
The first argument type and the return type should work consistently with the second argument type.

Actual behavior:
The first argument's type and the return type look like this:

Screenshot 2024-08-09 at 15 47 08 Screenshot 2024-08-09 at 15 47 36

the second argument's type looks like this:
Screenshot 2024-08-09 at 15 47 24

Versions

This can be seen in the latest nightly on live.lean-lang.org: https://live.lean-lang.org/#project=lean-nightly&code=%23check%20String.append%0A

I've also reproduced it in Lean 4.10.

In Lean 4.0 and 4.5, which uses the old format with the inaccessible names, the second one still has the wrong hover:
Screenshot 2024-08-09 at 15 50 17
Screenshot 2024-08-09 at 15 50 06

So this seems to be a fairly old bug.

Impact

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

@david-christiansen
Copy link
Contributor Author

After a bit more experimentation, this seems to apply to any function argument with a borrow annotation. Here's some examples:

  • Both arguments of String.get
  • Both arguments of Nat.add, Nat.mul, Nat.pow
  • The argument to UInt8.ofNat

@david-christiansen
Copy link
Contributor Author

This seems to be a general issue with metadata nodes and the delaborator. Here's a minimized failure:

import Lean

open Lean Elab Term Command in
open PrettyPrinter Delaborator in
elab "test" t:term : command => do
  let e ← liftTermElabM <| elabTerm t none
  logInfoAt t m!"{e} {Expr.mdata {} e}"

test String.append

The result is that the first occurrence has a correct hover, while the second doesn't:
Screenshot 2024-08-10 at 08 12 15
Screenshot 2024-08-10 at 08 12 25

I'd expect the annotations here to ignore mdata nodes when figuring out the meaning of some bit of rendered text.

@leanprover-bot leanprover-bot added the P-medium We may work on this issue if we find the time label Aug 16, 2024
kmill added a commit to kmill/lean4 that referenced this issue Oct 21, 2024
…tion errors can compound the issues. In any case, they can still be labeled.

improves addPPExplicitToExposeDiff when functions are overapplied

fixes mdata bugs in location RPC handler

fixes leanprover#4972
kmill added a commit to kmill/lean4 that referenced this issue Dec 1, 2024
Motivation: `sorry` should have an indeterminate value so that it's harder to make "fake" theorems about stubbed-out definitions. This PR makes each instance of `sorry` be non-defeq to any other. For example, this now fails:
```lean
example : (sorry : Nat) = sorry := rfl -- fails
```
However, this still succeeds:
```lean
def f (n : Nat) : Nat := sorry
example : f 0 = f 1 := rfl -- succeeds
```
One can be more careful by putting variables to the right of the colon:
```lean
def f : (n : Nat) → Nat := sorry
example : f 0 = f 1 := rfl -- fails
```

Details:

Adds `Lean.Meta.mkUniqueSorry`, which creates a sorry that is not defeq to any other sorry. It also encodes the source position into the term.

Makes the `sorry` term and `sorry` tactic create unique sorries.

Adds support to the LSP so that "go to definition" on `sorry` in the Infoview goes to the origin of that particular `sorry`.

Fixes `sorry` pretty printing: no more `sorryAx` in the Infoview.

Removes `Lean.Meta.mkSyntheticSorry` in favor of `Lean.Meta.mkSorry`.

pervasive mkUniqueSorry

unique -> labeled sorries. Turns out using unique sorries for elaboration errors can compound the issues. In any case, they can still be labeled.

improves addPPExplicitToExposeDiff when functions are overapplied

fixes mdata bugs in location RPC handler

fixes leanprover#4972

fix tests

revert comment

more unique
kmill added a commit to kmill/lean4 that referenced this issue Dec 8, 2024
Motivation: `sorry` should have an indeterminate value so that it's harder to make "fake" theorems about stubbed-out definitions. This PR makes each instance of `sorry` be non-defeq to any other. For example, this now fails:
```lean
example : (sorry : Nat) = sorry := rfl -- fails
```
However, this still succeeds:
```lean
def f (n : Nat) : Nat := sorry
example : f 0 = f 1 := rfl -- succeeds
```
One can be more careful by putting variables to the right of the colon:
```lean
def f : (n : Nat) → Nat := sorry
example : f 0 = f 1 := rfl -- fails
```

Details:

Adds `Lean.Meta.mkUniqueSorry`, which creates a sorry that is not defeq to any other sorry. It also encodes the source position into the term.

Makes the `sorry` term and `sorry` tactic create unique sorries.

Adds support to the LSP so that "go to definition" on `sorry` in the Infoview goes to the origin of that particular `sorry`.

Fixes `sorry` pretty printing: no more `sorryAx` in the Infoview.

Removes `Lean.Meta.mkSyntheticSorry` in favor of `Lean.Meta.mkSorry`.

pervasive mkUniqueSorry

unique -> labeled sorries. Turns out using unique sorries for elaboration errors can compound the issues. In any case, they can still be labeled.

improves addPPExplicitToExposeDiff when functions are overapplied

fixes mdata bugs in location RPC handler

fixes leanprover#4972

fix tests

revert comment

more unique
github-merge-queue bot pushed a commit that referenced this issue Dec 12, 2024
This PR makes it harder to create "fake" theorems about definitions that
are stubbed-out with `sorry` by ensuring that each `sorry` is not
definitionally equal to any other. For example, this now fails:
```lean
example : (sorry : Nat) = sorry := rfl -- fails
```
However, this still succeeds, since the `sorry` is a single
indeterminate `Nat`:
```lean
def f (n : Nat) : Nat := sorry
example : f 0 = f 1 := rfl -- succeeds
```
One can be more careful by putting parameters to the right of the colon:
```lean
def f : (n : Nat) → Nat := sorry
example : f 0 = f 1 := rfl -- fails
```
Most sources of synthetic sorries (recall: a sorry that originates from
the elaborator) are now unique, except for elaboration errors, since
making these unique tends to cause a confusing cascade of errors. In
general, however, such sorries are labeled. This enables "go to
definition" on `sorry` in the Infoview, which brings you to its origin.
The option `set_option pp.sorrySource true` causes the pretty printer to
show source position information on sorries.

**Details:**

* Adds `Lean.Meta.mkLabeledSorry`, which creates a sorry that is labeled
with its source position. For example, `(sorry : Nat)` might elaborate
to
  ```
sorryAx (Lean.Name → Nat) false
`[email protected]._hyg.153
  ```
It can either be made unique (like the above) or merely labeled. Labeled
sorries use an encoding that does not impact defeq:
  ```
sorryAx (Unit → Nat) false (Function.const Lean.Name ()
`[email protected]._hyg.174)
  ```

* Makes the `sorry` term, the `sorry` tactic, and every elaboration
failure create labeled sorries. Most are unique sorries, but some
elaboration errors are labeled sorries.

* Renames `OmissionInfo` to `DelabTermInfo` and adds configuration
options to control LSP interactions. One field is a source position to
use for "go to definition". This is used to implement "go to definition"
on labeled sorries.

* Makes hovering over a labeled `sorry` show something friendlier than
that full `sorryAx` expression. Instead, the first hover shows the
simplified ``sorry `«lean.foo:48:11»``. Hovering over that hover shows
the full `sorryAx`. Setting `set_option pp.sorrySource true` makes
`sorry` always start with printing with this source position
information.

* Removes `Lean.Meta.mkSyntheticSorry` in favor of `Lean.Meta.mkSorry`
and `Lean.Meta.mkLabeledSorry`.

* Changes `sorryAx` so that the `synthetic` argument is no longer
optional.

* Gives `addPPExplicitToExposeDiff` awareness of labeled sorries. It can
set `pp.sorrySource` when source positions differ.

* Modifies the delaborator framework so that delaborators can set Info
themselves without it being overwritten.

Incidentally closes #4972.

Inspired by [this Zulip
thread](https://leanprover.zulipchat.com/#narrow/channel/287929-mathlib4/topic/Is.20a.20.60definition_wanted.60.20keyword.20possible.3F/near/477260277).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working P-medium We may work on this issue if we find the time
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants