Skip to content

[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
merged 39 commits into from
Aug 21, 2018

Conversation

ahoppen
Copy link
Member

@ahoppen ahoppen commented Aug 17, 2018

This PR cherry picks all swiftSyntax related changes from master to swift-5.0-branch.

ahoppen added 30 commits August 16, 2018 15:36
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.
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.
@ahoppen ahoppen requested a review from shahmishal August 17, 2018 16:04
@ahoppen
Copy link
Member Author

ahoppen commented Aug 17, 2018

@swift-ci Please test

@swift-ci
Copy link
Contributor

Build failed
Swift Test Linux Platform
Git Sha - dd006f1

@shahmishal
Copy link
Member

@swift-ci test Linux

@swift-ci
Copy link
Contributor

Build failed
Swift Test Linux Platform
Git Sha - dd006f1

@shahmishal shahmishal merged commit 6d0c9de into swiftlang:swift-5.0-branch Aug 21, 2018
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