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

Delaboration of re-exported names #2524

Closed
hargoniX opened this issue Sep 8, 2023 · 4 comments · Fixed by #5688 · May be fixed by #5689
Closed

Delaboration of re-exported names #2524

hargoniX opened this issue Sep 8, 2023 · 4 comments · Fixed by #5688 · May be fixed by #5689

Comments

@hargoniX
Copy link
Contributor

hargoniX commented Sep 8, 2023

Description

Delaborating things that contain names of re-exported types can chose the wrong name for the type.
This most notably causes incorrect doc-gen4 renders: https://leanprover.zulipchat.com/#narrow/stream/270676-lean4/topic/Confusing.20delaboration.20in.20.20doc-gen/near/388158478

Steps to Reproduce

Delaboration of Lake.Name:

import Lean
import Lake

#eval show Lean.MetaM _ from do
  let ⟨fmt, _⟩ ← Lean.PrettyPrinter.ppExprWithInfos (Lean.mkConst ``Lean.Name)
  println! fmt

Further minified:

import Lean
import Lake

open Lean
#eval show MetaM _ from do
  unresolveNameGlobal ``Name

Expected behavior: We get Lean.Name

Actual behavior: We get Lake.Name

Reproduces how often: Always

@Kha
Copy link
Member

Kha commented Oct 9, 2023

@tydeu Let's just turn these into abbrevs? I'm not sure export is the right tool here. One of these names will always be printed as the other with it.

@tydeu
Copy link
Member

tydeu commented Oct 9, 2023

@Kha Printing it always as Lean.Name would be what I would expect here (i.e., for the original name to be the canonical name rather than the alias). With Lake.Name, I am not trying to create a distinct type, but just have Lean.Name available when I open Lake. But it is certainly possible I misunderstand the proper use of export.

kmill added a commit to kmill/lean4 that referenced this issue Oct 13, 2024
This replaces `export Lean (Name NameMap)` and `export System (SearchPath FilePath)` with the relevant `open` commands. This fixes docgen output so that it can refer to, for example, `Lean.Name` instead of `Lake.Name`.

The reason for these `export`s was convenience: by doing `open Lake` you could get these aliases for free. However, aliases affect pretty printing, and the Lake aliases took precedence. We don't want to disable pretty printing re-exported names because this can be a valid pattern (names could incrementally get re-exported from namespace to parent namespace).

In the future we might implement a feature to be able to `scope export` some names.

Closes leanprover#2524
kmill added a commit to kmill/lean4 that referenced this issue Oct 13, 2024
…n-API" `export`s

Heuristic: an "API" `export` is one that exports a declaration to a parent namespace. This PR makes it so that aliases created by `export` from exporting a name into some parallel namespace are not considered.

This is motivated by Lake's use of `export` to make some declarations like `Lean.Name` permanently available in the `Lake` namespace. A consequence to this is that in docgen, we see `Lean.Name` pretty printed as `Lake.Name`.

Another solution would be to create a `scoped open` command that auto-opens some names whenever a namespace is activated. If that is ever implemented, it would be easy to revert the changes in this PR.

Closes leanprover#2524
@kmill
Copy link
Collaborator

kmill commented Oct 13, 2024

I have two competing solutions to this problem:

  1. Change Lake. fix: have Lake not create core aliases into Lake namespace #5688 removes the exports and adds the relevant open commands.
  2. Change pretty printing. fix: when pretty printing constant names, do not use aliases from "non-API exports" #5689 makes pretty printing only consider aliases into parent namespaces.

I'd prefer option 1 if @tydeu would be ok with it (and if it wouldn't cause problems for lakefiles), but otherwise I'll leave it to @Kha and the triage team to decide which of these two solutions (if either) might be acceptable.

There's a third option, which is to implement a scoped open command, which would have the effect that Mac wants but which wouldn't create global aliases into the Lake namespace.

@tydeu
Copy link
Member

tydeu commented Oct 15, 2024

@kmill I am fine with a temporary stop-gap measure, but I would very much want this fixed in the future. Having to manually open common names is cumbersome. Furthermore, forgetting to do so can also lead to confusing problems with auto-implicits. Fortunately, your scoped open idea sounds like great solution!

kmill added a commit to kmill/lean4 that referenced this issue Oct 20, 2024
This replaces `export Lean (Name NameMap)` and `export System (SearchPath FilePath)` with the relevant `open` commands. This fixes docgen output so that it can refer to, for example, `Lean.Name` instead of `Lake.Name`.

The reason for these `export`s was convenience: by doing `open Lake` you could get these aliases for free. However, aliases affect pretty printing, and the Lake aliases took precedence. We don't want to disable pretty printing re-exported names because this can be a valid pattern (names could incrementally get re-exported from namespace to parent namespace).

In the future we might implement a feature to be able to `scope export` some names.

Closes leanprover#2524
github-merge-queue bot pushed a commit that referenced this issue Oct 20, 2024
This replaces `export Lean (Name NameMap)` and `export System
(SearchPath FilePath)` with the relevant `open` commands. This fixes
docgen output so that it can refer to, for example, `Lean.Name` instead
of `Lake.Name`.

The reason for these `export`s was convenience: by doing `open Lake` you
could get these aliases for free. However, aliases affect pretty
printing, and the Lake aliases took precedence. We don't want to disable
pretty printing re-exported names because this can be a valid pattern
(names could incrementally get re-exported from namespace to parent
namespace).

In the future we might implement a feature to be able to `scoped open`
some names.

Breaking change: Lakefiles that refer to `FilePath` may need to change
this to `System.FilePath` or otherwise add `open System (FilePath)`.

Closes #2524
tobiasgrosser pushed a commit to opencompl/lean4 that referenced this issue Oct 27, 2024
…rover#5688)

This replaces `export Lean (Name NameMap)` and `export System
(SearchPath FilePath)` with the relevant `open` commands. This fixes
docgen output so that it can refer to, for example, `Lean.Name` instead
of `Lake.Name`.

The reason for these `export`s was convenience: by doing `open Lake` you
could get these aliases for free. However, aliases affect pretty
printing, and the Lake aliases took precedence. We don't want to disable
pretty printing re-exported names because this can be a valid pattern
(names could incrementally get re-exported from namespace to parent
namespace).

In the future we might implement a feature to be able to `scoped open`
some names.

Breaking change: Lakefiles that refer to `FilePath` may need to change
this to `System.FilePath` or otherwise add `open System (FilePath)`.

Closes leanprover#2524
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants