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

fix: mainModuleName should use srcSearchPath #4066

Merged
merged 2 commits into from
May 8, 2024

Conversation

digama0
Copy link
Collaborator

@digama0 digama0 commented May 5, 2024

As reported on Zulip. The mainModuleName was being set incorrectly when browsing lean core sources, resulting in failure of cross-file server requests like "Find References". Because the srcSearchPath is generated asynchronously, we store it as a Task Name which is resolved some time before the header is finished parsing. (I don't think the .get here will ever block, because the srcSearchPath will be ready by the time the initial command snap is requested.)

@digama0 digama0 requested review from mhuisi and Kha as code owners May 5, 2024 01:31
@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 May 5, 2024
@leanprover-community-mathlib4-bot
Copy link
Collaborator

leanprover-community-mathlib4-bot commented May 5, 2024

Mathlib CI status (docs):

  • ❗ Mathlib CI can not be attempted yet, as the nightly-testing-2024-05-04 tag does not exist there yet. We will retry when you push more commits. If you rebase your branch onto nightly-with-mathlib, Mathlib CI should run now. (2024-05-05 01:48:59)
  • ✅ Mathlib branch lean-pr-testing-4066 has successfully built against this PR. (2024-05-08 02:04:12) View Log
  • ✅ Mathlib branch lean-pr-testing-4066 has successfully built against this PR. (2024-05-08 03:12:41) View Log
  • ✅ Mathlib branch lean-pr-testing-4066 has successfully built against this PR. (2024-05-08 10:03:51) View Log

@kim-em kim-em added the full-ci label May 8, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/batteries that referenced this pull request May 8, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/mathlib4 that referenced this pull request May 8, 2024
@leanprover-community-mathlib4-bot leanprover-community-mathlib4-bot added the builds-mathlib CI has verified that Mathlib builds against this PR label May 8, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/batteries that referenced this pull request May 8, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/mathlib4 that referenced this pull request May 8, 2024
@Kha
Copy link
Member

Kha commented May 8, 2024

@digama0 Marc and I were a bit concerned about the additional asynchronous control flow, so I pushed a refactoring that simplifies the whole context setup for the language processor. Makes sense?

@digama0
Copy link
Collaborator Author

digama0 commented May 8, 2024

Yes, LGTM.

leanprover-community-mathlib4-bot added a commit to leanprover-community/batteries that referenced this pull request May 8, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/mathlib4 that referenced this pull request May 8, 2024
Copy link
Contributor

@mhuisi mhuisi left a comment

Choose a reason for hiding this comment

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

LGTM

@Kha Kha added this pull request to the merge queue May 8, 2024
Merged via the queue into leanprover:master with commit 5814a45 May 8, 2024
21 checks passed
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.

5 participants