Skip to content

fix: respect preresolved names at resolveConst*#583

Merged
leodemoura merged 2 commits intoleanprover:masterfrom
Kha:resolve-const-pre
Jul 30, 2021
Merged

fix: respect preresolved names at resolveConst*#583
leodemoura merged 2 commits intoleanprover:masterfrom
Kha:resolve-const-pre

Conversation

@Kha
Copy link
Copy Markdown
Member

@Kha Kha commented Jul 22, 2021

This makes sure we can properly quote e.g. deriving clauses and avoids a suspicious eraseMacroScopes call (though not at Elab.Syntax, since categories do not have to be declaration names)

@Kha
Copy link
Copy Markdown
Member Author

Kha commented Jul 22, 2021

@leodemoura While I think this is definitely the correct way to go, there's no hurry in reviewing or merging it

@[builtinTermElab doubleQuotedName] def elabDoubleQuotedName : TermElab := fun stx _ => do
match stx[1].isNameLit? with
| some val => toExpr (← resolveGlobalConstNoOverloadWithInfo stx[1] val)
| some val => toExpr (← resolveGlobalConstNoOverloadCore val) -- TODO: restore info
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@leodemoura I think (double) quoted names should not be literals but use the regular ident parser. Otherwise they will not work correctly when quoted since they currently lack the preresolved information. Is there any reason not to change their parser?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I would need a context switch to answer this question. Could you please add comments here explaining the issue? It would also be useful to open a new issue with examples that do not work.
I am assuming this PR can be merged independently of this issue. That is, it is not introducing any new issue.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

With the current state of the PR we would lose go-to-definition on double-quoted names, so we might want to resolve the ident issue first. Opened #586.

This makes sure we can properly quote e.g. `deriving` clauses and avoids
a suspicious `eraseMacroScopes` call (though not at `Elab.Syntax`, since
categories do not have to be declaration names)
@Kha Kha force-pushed the resolve-const-pre branch from 1fa5c13 to 13c2de7 Compare July 27, 2021 13:26
@Kha
Copy link
Copy Markdown
Member Author

Kha commented Jul 27, 2021

Added a straightforward fix for #586 that changes the syntax of double-quoted names, but not single quoted ones, after noticing that we do use the latter with non-idents such as `_.

@Kha Kha force-pushed the resolve-const-pre branch from 13c2de7 to 3b0e24e Compare July 27, 2021 13:33
@leodemoura leodemoura merged commit 32bea73 into leanprover:master Jul 30, 2021
ChrisHughes24 pushed a commit to ChrisHughes24/lean4 that referenced this pull request Dec 2, 2022
Co-authored-by: Scott Morrison <scott.morrison@gmail.com>
ChrisHughes24 pushed a commit to ChrisHughes24/lean4 that referenced this pull request Dec 2, 2022
* [x] depends on leanprover#583 

The final section `/-! # units -/` has not been ported, because it still depends on some earlier files.

Co-authored-by: Scott Morrison <scott.morrison@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants