-
Notifications
You must be signed in to change notification settings - Fork 10.5k
[5.0] Cherry pick swiftSyntax related changes to swift-5.0-branch #18792
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
Merged
shahmishal
merged 39 commits into
swiftlang:swift-5.0-branch
from
ahoppen:swift-5.0-branch
Aug 21, 2018
Merged
[5.0] Cherry pick swiftSyntax related changes to swift-5.0-branch #18792
shahmishal
merged 39 commits into
swiftlang:swift-5.0-branch
from
ahoppen:swift-5.0-branch
Aug 21, 2018
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…generated by Xcode
SourcePresence and ID are already shared between the two node kinds and the node's length will soon be cached in RawSyntax as well. By making it a struct, we will be able to compute the node's length when it is being constructed in the initialiser.
For ScalarTraits, a buffer was always created on the heap to which the scalar string value was written just to be copied to the output buffer again. In case the value already exists in a memory buffer it is way cheaper to avoid the heap allocation and copy it straight to the output buffer.
Most keys are string literals and if they are converted directly to StringRef, the string length can be computed at compile time, increasing performance.
The recommended way forward is to use the SyntaxClassifier on the Swift side. By removing the C++ SyntaxClassifier, we can also eliminate the -force-libsyntax-based-processing option that was used to bootstrap incremental parsing and would generate the syntax map from a syntax tree.
This will allow us to switch to a more efficient serialization format in the future.
…ental transfer This way we will be able to avoid reclassifying these nodes for syntax highlighting since we know they haven't changed.
AbsolutePosition being a mutable reference type easily leads to bugs where an AbsolutePosition is modified. Making it immutable eliminates this issue. Furthermore, the introduction of SourceLength should allow easier manipulation of AbsolutePositions on the client side. We still cannot make AbsolutePosition a value type since it is used inside AtomicCache, but the immutability gives the same safety.
We will need this to transfer a binary encoded syntax tree in the future
We were always returning true from those functions in SKEditorConsumer and false in the test consumers. On the client side we would then ignore the return value. So it's clearer to have the functions not return anything.
This is faster than invoking getAbsolutePosition which needs to walk the tree again.
This will allow us to experiment with other serialization formats in the future more easily.
Heuristics have shown that this prevents several new mallocs and speeds incremental syntax tree transfer up.
…ialized completely
We cannot use unowned strings for token texts of incrementally parsed syntax trees since the source buffer to which reused nodes refer will have been freed for reused nodes. Always copying the token text whenever OwnedString is passed is too expensive. A reference counted copy of the string allows us to keep the token's string alive across incremental parses while eliminating unnecessary copies.
Previously RawSyntaxCacheNode was not increasing the RawSyntax refCount and accessing a cached node would fail if the node had been deleted in the meantime. We weren't hitting this so far, because all nodes were allocated using a bump allocator and thus the underlying memory never freed.
This resolves a memory leak.
AbsolutePosition had value semantics anyway, the only reason it was a reference type was so that we can use it in AtomicCache. But that can be worked around by boxing it into a reference type.
@swift-ci Please test |
Build failed |
@swift-ci test Linux |
Build failed |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR cherry picks all
swiftSyntax
related changes frommaster
toswift-5.0-branch
.