-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
Serialize span hygiene data #72121
Serialize span hygiene data #72121
Conversation
(rust_highfive has picked a reviewer for you, use r? to override) |
Let's see if there are any obvious performance issues with this approach: @bors try @rust-timer queue |
Awaiting bors try build completion |
⌛ Trying commit fa6ef57e1a2fa04dbf737d1307d82e402cc1f0ff with merge 8bdad37a5ef4b604e078804ddecf3ee24bc50748... |
This comment has been minimized.
This comment has been minimized.
☀️ Try build successful - checks-actions, checks-azure |
Queued 8bdad37a5ef4b604e078804ddecf3ee24bc50748 with parent 99cb9cc, future comparison URL. |
Finished benchmarking try commit 8bdad37a5ef4b604e078804ddecf3ee24bc50748, comparison URL. |
@bors try @rust-timer queue |
Awaiting bors try build completion |
This comment has been minimized.
This comment has been minimized.
e1393db
to
899f82d
Compare
@bors try @rust-timer queue |
Awaiting bors try build completion |
⌛ Trying commit 899f82d06559b5e53ab584b9579a3aa3a1a06811 with merge 0d27819f009a7737f26dc558d8350176ce8c5b07... |
@bors try @rust-timer queue |
Awaiting bors try build completion |
⌛ Trying commit 2679248592393888eb579e9495a726d9525b6028 with merge 6326e282a07844c03667b78cc4904c3e45b1a423... |
☀️ Try build successful - checks-actions, checks-azure |
Queued 6326e282a07844c03667b78cc4904c3e45b1a423 with parent 769d12e, future comparison URL. |
Finished benchmarking try commit 6326e282a07844c03667b78cc4904c3e45b1a423, comparison URL. |
This is going to require more work to reduce the performance impact. It may be necessary to re-visit assigning |
⌛ Testing commit f7235a8 with merge 4d20beecfc50d9db8ae1c80347207215e6b2ca10... |
@bors retry yield |
Your PR failed (pretty log, raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem. Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
⌛ Testing commit f7235a8 with merge 2b1adaeca4d2cd3de0a743769cf724499bda5eda... |
@bors retry |
@Dylan-DPC: I think this PR and #73265 shouldn't conflict. |
I think so as well but it's more as a precatuionary measure. that pr has been trying to get merged for a while now so don't want to add more latency. |
☀️ Test successful - checks-actions, checks-azure |
This was a perf loss, as expected. |
Fixes solana-labs#10933 Now that rust-lang/rust#72121 has been merged, using a `$crate` path from a nested `macro_rules!` will work properly across multiple crates. This allows us to update to a nightly build containing this PR, and stop using `::solana_sdk` to refer to the `respan!` macro.
Fixes solana-labs#10933 Now that rust-lang/rust#72121 has been merged, using a `$crate` path from a nested `macro_rules!` will work properly across multiple crates. This allows us to stop using `::solana_sdk` to reference to the `respan!` macro.
…sh, r=petrochenkov Hash parent ExpnData cc rust-lang#72121 (comment)
This nightly build includes both rust-lang/rust#72121 and a working `rustfmt`.
Fixes solana-labs#10933 Now that rust-lang/rust#72121 has been merged, using a `$crate` path from a nested `macro_rules!` will work properly across multiple crates. This allows us to stop using `::solana_sdk` to reference to the `respan!` macro.
Fixes #10933 Now that rust-lang/rust#72121 has been merged, using a `$crate` path from a nested `macro_rules!` will work properly across multiple crates. This allows us to stop using `::solana_sdk` to reference to the `respan!` macro.
See rust-lang/rust#72121. The PR fixes some hygiene issues and thus exposes that the originating expression comes from macros.
See rust-lang/rust#72121. The PR fixes some hygiene issues and thus exposes that the originating expression comes from macros.
See rust-lang/rust#72121. The PR fixes some hygiene issues and thus exposes that the originating expression comes from macros.
Fixes #68686
Fixes #70963
This PR serializies global hygiene data into both the incremental compilation cache and the crate metadata. This allows hygiene information to be preserved across compilation sessions (both incremental and cross-crate).
When serializing a
SyntaxContext
, we simply write out the raw id from the current compilation session. Whenever we deserialize aSyntaxContext
, we 'remap' the id to a fresh id in our current compilation session, and load the associatedSyntaxContextData
.As a result, some 'upstream'
SyntaxContextData
will end up getting duplicated in 'downstream' crates. This only happens when we actually need to use an 'upstream'SyntaxContext
, which occurs when we deserialize aSpan
that requires it.We serialize an
ExpnData
into the metadata of the crate which generated it. AnExpnId
is serialized as a reference into the crate which 'owns' the correspondingExpnData
, which avoids duplication in downstream crates.I've included a macros 2.0 test which requires hygiene serialization to compile successfully.
TODO:
Determine how many additional(We no longer createDefId
s we end up creating forExpnId
s - this may be significant forlibcore
, which uses macros heavily. Alternatively, we could try to compute aDefPathHash
without making a correspondingDefId
- however, this might significantly complicate the implementation.DefId
s)SyntaxContextData
in crate metadata.resolve_crate_root
behaves with deserialized hygiene data - the current logic may be wrong.rust/src/librustc_resolve/build_reduced_graph.rs
Line 892 in 4774f9b
src/test/ui/hygiene/cross_crate_hygiene.rs
passes means that this is working.rust/src/librustc_resolve/imports.rs
Lines 1389 to 1392 in 3d5d0f8