-
Notifications
You must be signed in to change notification settings - Fork 440
Pass length of source file to C parsing actions #261
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
Conversation
@swift-ci Please test |
@@ -192,7 +192,7 @@ public enum SyntaxParser { | |||
swiftparse_parser_set_diagnostic_handler(c_parser, diagHandler) | |||
} | |||
|
|||
let c_top = swiftparse_parse_string(c_parser, source) | |||
let c_top = swiftparse_parse_string(c_parser, source, source.utf8.count) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe passing String
to const char *
calls String.utf8CString
which always copies the buffer. Probably
let c_top = source.withUTF8 {
$0.withMemoryRebound(to: CChar.self) { buf in
swiftparse_parse_string(c_parser, buf.baseAddress, buf.count)
}
}
is faster unless it needs to be null terminated.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It needs to be null-terminated, otherwise lvm::MemoryBuffer::getMemBuffer
is not happy (also we need the terminating null char to lex it as the EOF token).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, in that case, I suggest
let c_top = source.withCString { buf in
swiftparse_parse_string(c_parser, buf, source.utf8.count)
}
withCString
doesn't copy the buffer if the underlying string buffer is null terminated.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good 👍
Previously, the string was implicitly terminated by the first null character. But a source file might contain a null character which shouldn't terminate it.
af5448b
to
cb72454
Compare
…l-mode Fix and improve parallel mode
Previously, the string was implicitly terminated by the first null character. But a source file might contain a null character which shouldn't terminate it.