feat(swc/common): wrap serialized struct with versioned (#5060: Part 3) #5128
Conversation
This file contains 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
kwonoj
changed the title
kdy1
requested changes
Jul 7, 2022
kdy1
requested changes
Jul 7, 2022
crates/swc_common/src/comments.rs
Outdated
| @@ -548,6 +548,16 @@ pub struct Comment { | |||
| pub text: String, | |||
| } | |||
|
|
|||
| impl Default for Comment { | |||
There was a problem hiding this comment.
Is this still required, even if we add into_inner()?
There was a problem hiding this comment.
default was added to satisfy mem::take in VersionedSerializable::take. if there's other way default is not required mandatoriliy.
I'm bit unsure if I understood the suggestion correctly, mind elaborate bit more? The way I understood is let each struct implement Take::dummy() instead of default, and VersionedSerializable should utilize it for the take. Not sure where into_inner plays in here.
There was a problem hiding this comment.
into_inner take ownership of VersionedSerializable so it does not require take(&mut self.0.1).
There was a problem hiding this comment.
The Default constraint comes from it.
| @@ -36,6 +36,19 @@ pub struct Diagnostic { | |||
| pub suggestions: Vec<CodeSuggestion>, | |||
| } | |||
|
|
|||
| impl Default for Diagnostic { | |||
crates/swc_common/src/plugin.rs
Outdated
| @@ -41,6 +41,12 @@ pub enum PluginError { | |||
| Serialize(String), | |||
| } | |||
|
|
|||
| impl Default for PluginError { | |||
crates/swc_common/src/syntax_pos.rs
Outdated
| @@ -130,6 +130,12 @@ pub enum FileName { | |||
| Custom(String), | |||
| } | |||
|
|
|||
| impl Default for FileName { | |||
crates/swc_common/src/syntax_pos.rs
Outdated
| @@ -805,7 +811,7 @@ impl Sub<BytePos> for NonNarrowChar { | |||
| derive(rkyv::Archive, rkyv::Serialize, rkyv::Deserialize) | |||
| )] | |||
| #[cfg_attr(feature = "rkyv", archive_attr(repr(C), derive(bytecheck::CheckBytes)))] | |||
| #[derive(Clone)] | |||
| #[derive(Clone, Default)] | |||
crates/swc_plugin_proxy/src/memory_interop/read_returned_result_from_host.rs
Show resolved
Hide resolved
| @@ -60,7 +62,7 @@ where | |||
| pub fn read_returned_result_from_host<F, R>(f: F) -> Option<R> | |||
| where | |||
| F: FnOnce(i32) -> i32, | |||
| R: rkyv::Archive, | |||
| R: rkyv::Archive + std::default::Default, | |||
crates/swc_common/src/syntax_pos.rs
Outdated
| @@ -1193,6 +1210,7 @@ pub struct LineCol { | |||
| pub col: u32, | |||
| } | |||
|
|
|||
| #[derive(Default)] | |||
crates/swc_plugin_proxy/src/memory_interop/read_returned_result_from_host.rs
Outdated
Show resolved
Hide resolved
| @@ -85,11 +88,11 @@ where | |||
| pub fn read_returned_result_from_host_fallible<F, R>(f: F) -> Option<R> | |||
| where | |||
| F: FnOnce(i32) -> i32, | |||
| R: rkyv::Archive, | |||
| R: rkyv::Archive + std::default::Default, | |||
kwonoj
force-pushed
the
feat-versioned-serialized
branch
from
89966ab
to
e48bcbd
Compare
kwonoj
force-pushed
the
feat-versioned-serialized
branch
from
e48bcbd
to
509e766
Compare
kdy1
approved these changes
Jul 7, 2022
swc-bot
reviewed
Jul 7, 2022
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Description:
This PR actually wraps up structs with
VersionedSerializablefor the plugins.PluginSerializedBytes's serialize / deserialize interface now explicitly acceptsVersionedSerializableonly. Most changes are straightforward, with implementingdefaultfor few structs & couple ofcopyaround diagnostic / comment to workaround moves.There is one item I need to revisit later (main...kwonoj:swc:feat-versioned-serialized#diff-b2b47f82dd3bafcc79a1e0e6ec2ccb75624c020e295e2a65c03d01058b56da92R178) since
Result<T>is not possible to useVersionedSerializable::take()due to lacks of default.BREAKING CHANGE:
Related issue (if exists):
#5060