Skip to content

Conversation

weihanglo
Copy link
Member

@weihanglo weihanglo commented Sep 27, 2024

What does this PR try to resolve?

Found this during PR review.

We could leverage #[serde(borrow)] for zero-copy deserialization for
messages from the compiler.

We can't use &str fields because they may contain escape sequences
in the future, which fails the deserialization.
See serde-rs/json#742

How should we test and review this PR?

Test suite passes?

Additional information

I didn't do any benchmark.

Found this during PR review.

We could leverage `#[serde(borrow)]` for zero-copy deserialization for
messages from the compiler.

We can't use `&str` fields because they may contain escape sequences
in the future, which fails the deserialization.
See serde-rs/json#742
`artifact` field is an (absolute) path to emitted artifact like
/home/projects/foo/target/debug/deps/libbar-73d672db2af2c9a8.rmeta

It is worth not copying them.
@rustbot
Copy link
Collaborator

rustbot commented Sep 27, 2024

r? @epage

rustbot has assigned @epage.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added A-build-execution Area: anything dealing with executing the compiler S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 27, 2024
@epage
Copy link
Contributor

epage commented Sep 29, 2024

@bors r+

@bors
Copy link
Contributor

bors commented Sep 29, 2024

📌 Commit d6b740f has been approved by epage

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 29, 2024
@bors
Copy link
Contributor

bors commented Sep 29, 2024

⌛ Testing commit d6b740f with merge 5e2878f...

@bors
Copy link
Contributor

bors commented Sep 29, 2024

☀️ Test successful - checks-actions
Approved by: epage
Pushing 5e2878f to master...

@bors bors merged commit 5e2878f into rust-lang:master Sep 29, 2024
22 checks passed
@weihanglo weihanglo deleted the deserialize branch September 29, 2024 03:41
bors added a commit to rust-lang-ci/rust that referenced this pull request Oct 5, 2024
Update cargo

17 commits in 80d82ca22abbee5fb7b51fa1abeb1ae34e99e88a..ad074abe3a18ce8444c06f962ceecfd056acfc73
2024-09-27 17:56:01 +0000 to 2024-10-04 18:18:15 +0000
- test: Remove the last of our custom json assertions (rust-lang/cargo#14576)
- docs(ref): Expand on MSRV (rust-lang/cargo#14636)
- docs: Minor re-grouping of pages (rust-lang/cargo#14620)
- docs(ref): Highleft whats left for msrv-policy (rust-lang/cargo#14638)
- Fix `cargo:version_number` - has only one `:` (rust-lang/cargo#14637)
- docs: Declare support level for each crate in our Charter / docs (rust-lang/cargo#14600)
- chore(deps): update tar to 0.4.42 (rust-lang/cargo#14632)
- docs(charter): Declare new Intentional Artifacts as 'small' changes (rust-lang/cargo#14599)
- fix: Remove implicit feature removal (rust-lang/cargo#14630)
- docs(config): make `--config <PATH>` more prominent (rust-lang/cargo#14631)
- chore(deps): update rust crate unicode-width to 0.2.0 (rust-lang/cargo#14624)
- chore(deps): update embarkstudios/cargo-deny-action action to v2 (rust-lang/cargo#14628)
- docs(ref): Clean up language for `package.rust-version` (rust-lang/cargo#14619)
- docs: clarify `target.'cfg(...)'`  doesnt respect cfg from build script (rust-lang/cargo#14312)
- test: relax compiler panic assertions (rust-lang/cargo#14618)
- refactor(compiler): zero-copy deserialization when possible (rust-lang/cargo#14608)
- test: add support for features in the sat resolver (rust-lang/cargo#14583)
@rustbot rustbot added this to the 1.83.0 milestone Oct 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-build-execution Area: anything dealing with executing the compiler S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants