Skip to content

Conversation

@rintaro
Copy link
Member

@rintaro rintaro commented May 22, 2024

  • Stop using TokenSequence. Instead, visit RawSyntax tree manually
  • Avoid parsing trivia, and creating Array of RawTriviaPiece. Instead, scan the whole token text directly if available
  • Collect #sourceLocation() directives in one-pass

@rintaro rintaro requested review from ahoppen and bnbarham as code owners May 22, 2024 20:32
@rintaro
Copy link
Member Author

rintaro commented May 22, 2024

@swift-ci Please test

@rintaro rintaro requested a review from hamishknight May 22, 2024 21:13
var position: AbsolutePosition = .startOfFile
let addLine = { (lineLength: SourceLength) in
var sourceLocationDirectives: [(sourceLine: Int, arguments: SourceLocationDirectiveArguments?)] = []
let prefix = tree.raw.forEachLineLength { lineLength in
Copy link
Member

Choose a reason for hiding this comment

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

prefix doesn’t really make sense as a variable name here, right? Shouldn’t it be sourceFileLength or something? Also, the usage of prefix in RawSyntax.forEachLineLength + the comment on it seem pretty confusing to me. Not your doing but might be worth to clean that up along the way.

Copy link
Member Author

Choose a reason for hiding this comment

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

This prefix is more like lastLineLength. I changed the naming so.
I didn't changed the naming in forEachLineLength functions, because I wasn't able to come up with better naming. It's like "the length of the current line so far"

@rintaro rintaro force-pushed the perf-computelines branch from e54f8d1 to 6a81e49 Compare May 23, 2024 02:06
@rintaro
Copy link
Member Author

rintaro commented May 23, 2024

@swift-ci Please test

@rintaro
Copy link
Member Author

rintaro commented May 23, 2024

@swift-ci Please test Windows

@rintaro
Copy link
Member Author

rintaro commented May 23, 2024

@swift-ci Please test

@rintaro
Copy link
Member Author

rintaro commented May 23, 2024

@swift-ci Please test macOS

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.

3 participants