Skip to content

go/parser: scanner errors do not cause a bailout #70726

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.

Sign up for GitHub

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

Jump to bottom
Open
mateusz834 opened this issue Dec 8, 2024 · 3 comments
Open

go/parser: scanner errors do not cause a bailout #70726

mateusz834 opened this issue Dec 8, 2024 · 3 comments
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
Backlog

Comments

@mateusz834
Copy link
Member

While investigating #70725, i noticed that, errors from the scanner are passed directly to p.errors:

go/src/go/parser/parser.go

Lines 69 to 71 in 8c3e391

p.file = file
eh := func(pos token.Position, msg string) { p.errors.Add(pos, msg) }
p.scanner.Init(p.file, src, eh, scanner.ScanComments)

Per the docs i would assume that the parsing should stop after 10 error (including scanner errors):

go/src/go/parser/interface.go

Line 57 in 8c3e391

AllErrors = SpuriousErrors // report all errors (not just the first 10 on different lines)

It should probably call the p.errors instead:

go/src/go/parser/parser.go

Lines 260 to 281 in 8c3e391

func (p *parser) error(pos token.Pos, msg string) {
if p.trace {
defer un(trace(p, "error: "+msg))
}
epos := p.file.Position(pos)
// If AllErrors is not set, discard errors reported on the same line
// as the last recorded error and stop parsing if there are more than
// 10 errors.
if p.mode&AllErrors == 0 {
n := len(p.errors)
if n > 0 && p.errors[n-1].Pos.Line == epos.Line {
return // discard - likely a spurious error
}
if n > 10 {
panic(bailout{})
}
}
p.errors.Add(epos, msg)
}

CC @griesemer @adonovan
@mateusz834 mateusz834 added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Dec 8, 2024
@mateusz834 mateusz834 changed the title go/parser: scanner errors do not cause a bailout, when AllErrors is not set go/parser: scanner errors do not cause a bailout Dec 9, 2024
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/634538 mentions this issue: go/parser: report scanner error by p.errors

@gabyhelp
Copy link

gabyhelp commented Dec 9, 2024

Related Code Changes

(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.)

@griesemer griesemer added this to the Backlog milestone Dec 10, 2024
@griesemer
Copy link
Contributor

Note that initializing the scanner with Scanner.Init may lead to an error because Init reads the first character. This would cause the parser's error handler to be called before the parser is fully set up (hence the current structure). The scanner initialization may have to happen just before calling Next.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Milestone
Backlog
Development

No branches or pull requests
4 participants
@griesemer @gopherbot @mateusz834 @gabyhelp