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.

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

[SyntaxText] Always use memcmp #2617

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

[SyntaxText] Always use memcmp #2617

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
32 changes: 10 additions & 22 deletions Sources/SwiftSyntax/SyntaxText.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ private import Darwin
private import Glibc
#elseif canImport(Musl)
private import Musl
#elseif canImport(ucrt)
private import ucrt
#endif
#else
#if canImport(Darwin)
Expand All @@ -25,6 +27,8 @@ import Darwin
import Glibc
#elseif canImport(Musl)
import Musl
Copy link
Member Author

@rintaro rintaro Apr 22, 2024 •
edited

Choose a reason for hiding this comment

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

In case anyone is wondering if Musl has memcmp: http://git.musl-libc.org/cgit/musl/tree/include/string.h#n30 http://git.musl-libc.org/cgit/musl/tree/src/string/memcmp.c

#elseif canImport(ucrt)
import ucrt
#endif
#endif

Expand Down Expand Up @@ -131,7 +135,7 @@ public struct SyntaxText: Sendable {
while start <= stop {
// Force unwrappings are safe because we know 'self' and 'other' are both
// not empty.
if compareMemory(self.baseAddress! + start, other.baseAddress!, other.count) {
if memcmp(self.baseAddress! + start, other.baseAddress!, numericCast(other.count)) == 0 {
return start..<(start + other.count)
} else {
start += 1
Expand Down Expand Up @@ -194,11 +198,11 @@ extension SyntaxText: Hashable {
// SwiftSyntax use cases, comparing the same SyntaxText instances is
// extremely rare, and checking it causes extra branch.
// The most common usage is comparing parsed text with a static text e.g.
// `token.text == "func"`. In such cases `compareMemory`(`memcmp`) is
// optimized to a `cmp` or similar opcode if either operand is a short static
// text. So the same-baseAddress shortcut doesn't give us a huge performance
// boost even if they actually refer the same memory.
return compareMemory(lBase, rBase, lhs.count)
// `token.text == "func"`. In such cases `memcmp` is optimized to a `cmp` or
// similar opcode if either operand is a short static text. So the
// same-baseAddress shortcut doesn't give us a huge performance boost even
// if they actually refer the same memory.
return memcmp(lBase, rBase, numericCast(lhs.count)) == 0
}

/// Hash the contents of this ``SyntaxText`` into `hasher`.
Expand Down Expand Up @@ -270,19 +274,3 @@ extension String {
}
}
}

private func compareMemory(
_ s1: UnsafePointer<UInt8>,
_ s2: UnsafePointer<UInt8>,
_ count: Int
) -> Bool {
precondition(count >= 0)
#if canImport(Darwin)
return Darwin.memcmp(s1, s2, count) == 0
#elseif canImport(Glibc)
return Glibc.memcmp(s1, s2, count) == 0
#else
return UnsafeBufferPointer(start: s1, count: count)
.elementsEqual(UnsafeBufferPointer(start: s2, count: count))
#endif
}