Refactor the lexer to store lexer errors in the tokens instead of having a diagnosticsHandler #1191
Conversation
|
@swift-ci Please test |
Sources/SwiftParser/Lexer.swift
Outdated
| self.advance(while: { $0.isValidIdentifierContinuationCodePoint }) | ||
| return (.integerLiteral, [.isErroneous]) | ||
| return LexerResult(.integerLiteral) |
There was a problem hiding this comment.
This appears to accept the literal 0x
There was a problem hiding this comment.
This one is still an issue too
There was a problem hiding this comment.
Since this is not a regression, I just opened #1214 so we can fix it in a follow-up PR.
Sources/SwiftSyntax/LexerError.swift
Outdated
| /// If the token has a lexical error, this defines the type of the error. | ||
| /// `lexerErrorOffset` in the token will specify at which offset the error | ||
| /// occurred. | ||
| public enum LexerError { |
There was a problem hiding this comment.
Any thoughts on something like:
public struct LexerError {
public enum Kind {
case ...
}
public let kind: Kind
public let byteOffset: UInt16 // No need for alias, it's only defined in one place now
public init(...) { ... }
}
And then just having the one public var lexerError: LexerError field and a
public var hasLexerError: Bool {
return lexerError != nil
}
?
This would also simplify all the comments that mention "if there's an error, lexerErrorOffset will specify the offset".
As an aside, I'd personally be fine with UInt8 as the offset type. Do people really have more than 255 characters for a token?
There was a problem hiding this comment.
That’s a much better idea. No idea why I modeled it the way I did…
As an aside, I'd personally be fine with UInt8 as the offset type. Do people really have more than 255 characters for a token?
String literals are currently single tokens and those can reasonably be bigger than 255 characters, I think.
There was a problem hiding this comment.
String literals are currently single tokens and those can reasonably be bigger than 255 characters, I think.
Oh, unfortunate.
ahoppen
force-pushed
the
ahoppen/lexer-errors-in-token
branch
from
80cd97b to
f80753d
Compare
| @@ -169,6 +169,11 @@ public struct TokenSyntax: SyntaxProtocol, SyntaxHashable { | |||
| public func childNameForDiagnostics(_ index: SyntaxChildrenIndex) -> String? { | |||
| return nil | |||
| } | |||
|
|
|||
| /// If the token has a lexial error, the type of the error. | |||
There was a problem hiding this comment.
| /// If the token has a lexial error, the type of the error. | |
| /// If the token has a lexical error, the type of the error. |
| if tokenView.hasLexerError || tokenView.presence == .missing { | ||
| if tokenView.lexerError != nil || tokenView.presence == .missing { |
There was a problem hiding this comment.
IMO it wouldn't hurt to add a hasLexerError, but up to you.
ahoppen
force-pushed
the
ahoppen/lexer-errors-in-token
branch
from
f80753d to
58554b0
Compare
ahoppen
changed the title
ahoppen
changed the title
ahoppen
changed the title
ahoppen
force-pushed
the
ahoppen/lexer-errors-in-token
branch
from
58554b0 to
13e60bb
Compare
|
@swift-ci Please test |
ahoppen
force-pushed
the
ahoppen/lexer-errors-in-token
branch
from
13e60bb to
b72c418
Compare
…ing a diagnosticsHandler Instead of marking a token as having an error and relying on re-lexing to emit a diagnostic, store the type of error and the offset at which it occurred in the token itself. This is a more sound design IMO and will continue to work even if we introduce state into the lexer (e.g. whether we are currently in a string literal or inside a conflict marker range).
ahoppen
force-pushed
the
ahoppen/lexer-errors-in-token
branch
from
b72c418 to
8345eac
Compare
|
@swift-ci Please test |
Instead of marking a token as having an error and relying on re-lexing to emit a diagnostic, store the type of error and the offset at which it occurred in the token itself.
This is a more sound design IMO and will continue to work even if we introduce state into the lexer (e.g. whether we are currently in a string literal or inside a conflict marker range).