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(frontends/lean/parser): start positions of trailing parser (NuD) nodes #785

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

digama0
Copy link
Member

@digama0 digama0 commented Nov 18, 2022

As observed by @eric-wieser , the node for notation[$] in

theorem foo : true :=
id $ trivial

was covering only the span $ trivial instead of id $ trivial, meaning that the id child is outside of the span of its parent node. The fix is to take the start position from the start position of the first child instead of the current parser position when we parse a trailing node. (We also want to use this position for most other activities like constructing multiple expressions all with the same position due to notation expansion, but not for the error message when the parse is invalid.)

@eric-wieser
Copy link
Member

eric-wieser commented Nov 18, 2022

CI seems unhappy with this test:

example := [tt]++[]
--^ "command": "info"

which no longer produces

{"record":{"full-id":"has_append.append","source":,"state":"⊢ list bool","type":"Π {α : Type} [self : has_append α], α → α → α"},"response":"ok","seq_num":16}

@@ -1425,14 +1425,15 @@ static pair<notation::transition, parse_table> const * get_non_skip(list<pair<no
expr parser::parse_notation(parse_table t, expr * left) {
check_system("parse_notation");
lean_assert(curr() == token_kind::Keyword);
auto p = pos();
auto cur_pos = pos();
auto p = left ? expr_ast(*left).m_start : cur_pos;
Copy link
Member

@eric-wieser eric-wieser Nov 18, 2022

Choose a reason for hiding this comment

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

I don't understand this change; I'm probably blind, but it appears that nothing uses p below any more?

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