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: shorten auto-generated instance names #3089

Merged
merged 12 commits into from
Apr 13, 2024

Conversation

kmill
Copy link
Collaborator

@kmill kmill commented Dec 18, 2023

Implements a new method to generate instance names for anonymous instances that uses a heuristic that tends to produce shorter names. A design goal is to make them relatively unique within projects and definitely unique across projects, while also using accessible names so that they can be referred to as needed, both in Lean code and in discussions.

The new method also takes into account binders provided to the instance, and it adds project-based suffixes. Despite this, a median new name is 73% its original auto-generated length. (Compare: old generated names and new generated names.)

Some notes:

  • The naming is sensitive to what is explicitly provided as a binder vs what is provided via a variable. It does not make use of variables since, when names are generated, it is not yet known which variables are used in the body of the instance.
  • If the instance name refers to declarations in the current "project" (given by the root module), then it does not add a suffix. Otherwise, it adds the project name as a suffix to protect against cross-project collisions.
  • set_option trace.Elab.instance.mkInstanceName true can be used to see what name the auto-generator would give, even if the instance already has an explicit name.

There were a number of instances that were referred to explicitly in meta code, and these have been given explicit names.

Removes the unused Lean.Elab.mkFreshInstanceName along with the Command state's nextInstIdx.

Fixes #2343

@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 Dec 18, 2023
leanprover-community-mathlib4-bot added a commit to leanprover-community/mathlib4 that referenced this pull request Dec 18, 2023
@leanprover-community-mathlib4-bot leanprover-community-mathlib4-bot added the breaks-mathlib This is not necessarily a blocker for merging: but there needs to be a plan label Dec 18, 2023
@leanprover-community-mathlib4-bot
Copy link
Collaborator

leanprover-community-mathlib4-bot commented Dec 18, 2023

leanprover-community-mathlib4-bot added a commit to leanprover-community/mathlib4 that referenced this pull request Dec 18, 2023
@kmill kmill added the full-ci label Dec 18, 2023
leanprover-community-mathlib4-bot added a commit to leanprover-community/mathlib4 that referenced this pull request Dec 20, 2023
@kbuzzard kbuzzard mentioned this pull request Jan 28, 2024
1 task
@semorrison
Copy link
Collaborator

I've rebased this onto nightly-with-mathlib to see if we can get a build.

@kmill kmill force-pushed the anon-inst-name branch 2 times, most recently from 9753d39 to 8db32f0 Compare March 30, 2024 23:55
leanprover-community-mathlib4-bot added a commit to leanprover-community/batteries that referenced this pull request Mar 31, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/mathlib4 that referenced this pull request Mar 31, 2024
@leanprover-community-mathlib4-bot leanprover-community-mathlib4-bot added builds-mathlib CI has verified that Mathlib builds against this PR and removed breaks-mathlib This is not necessarily a blocker for merging: but there needs to be a plan labels Mar 31, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/batteries that referenced this pull request Mar 31, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/mathlib4 that referenced this pull request Mar 31, 2024
@kmill kmill marked this pull request as ready for review March 31, 2024 20:00
leanprover-community-mathlib4-bot added a commit to leanprover-community/batteries that referenced this pull request Mar 31, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/mathlib4 that referenced this pull request Mar 31, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/batteries that referenced this pull request Apr 2, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/mathlib4 that referenced this pull request Apr 2, 2024
@leanprover-community-mathlib4-bot leanprover-community-mathlib4-bot added breaks-mathlib This is not necessarily a blocker for merging: but there needs to be a plan and removed builds-mathlib CI has verified that Mathlib builds against this PR labels Apr 2, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/batteries that referenced this pull request Apr 2, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/mathlib4 that referenced this pull request Apr 2, 2024
@leanprover-community-mathlib4-bot leanprover-community-mathlib4-bot added builds-mathlib CI has verified that Mathlib builds against this PR and removed breaks-mathlib This is not necessarily a blocker for merging: but there needs to be a plan labels Apr 2, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/batteries that referenced this pull request Apr 2, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/mathlib4 that referenced this pull request Apr 2, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/batteries that referenced this pull request Apr 3, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/mathlib4 that referenced this pull request Apr 3, 2024
@kmill kmill added this pull request to the merge queue Apr 12, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Apr 12, 2024
@kmill kmill added this pull request to the merge queue Apr 13, 2024
Merged via the queue into leanprover:master with commit 1c20b53 Apr 13, 2024
20 checks passed
@digama0
Copy link
Collaborator

digama0 commented Apr 17, 2024

Master seems to be broken, I think due to test changes from this PR

@kmill
Copy link
Collaborator Author

kmill commented Apr 17, 2024

Fascinating, it was triggered by the stage0 update. Fixing the tests...

Kha pushed a commit that referenced this pull request Apr 17, 2024
#3089 caused the stage0 update to cause a number of tests to start
failing because they were using the old instance names.
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.

ridiculously long instance names
4 participants