Skip to content

Commit

Permalink
Auto merge of #93066 - nnethercote:infallible-decoder, r=bjorn3
Browse files Browse the repository at this point in the history
Make `Decodable` and `Decoder` infallible.

`Decoder` has two impls:
- opaque: this impl is already partly infallible, i.e. in some places it
  currently panics on failure (e.g. if the input is too short, or on a
  bad `Result` discriminant), and in some places it returns an error
  (e.g. on a bad `Option` discriminant). The number of places where
  either happens is surprisingly small, just because the binary
  representation has very little redundancy and a lot of input reading
  can occur even on malformed data.
- json: this impl is fully fallible, but it's only used (a) for the
  `.rlink` file production, and there's a `FIXME` comment suggesting it
  should change to a binary format, and (b) in a few tests in
  non-fundamental ways. Indeed #85993 is open to remove it entirely.

And the top-level places in the compiler that call into decoding just
abort on error anyway. So the fallibility is providing little value, and
getting rid of it leads to some non-trivial performance improvements.

Much of this PR is pretty boring and mechanical. Some notes about
a few interesting parts:
- The commit removes `Decoder::{Error,error}`.
- `InternIteratorElement::intern_with`: the impl for `T` now has the same
  optimization for small counts that the impl for `Result<T, E>` has,
  because it's now much hotter.
- Decodable impls for SmallVec, LinkedList, VecDeque now all use
  `collect`, which is nice; the one for `Vec` uses unsafe code, because
  that gave better perf on some benchmarks.

r? `@bjorn3`
  • Loading branch information
bors committed Jan 23, 2022
2 parents 1e40679 + 37fbd91 commit 84322ef
Show file tree
Hide file tree
Showing 39 changed files with 726 additions and 783 deletions.
5 changes: 3 additions & 2 deletions compiler/rustc_ast/src/ast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2418,8 +2418,9 @@ impl<S: Encoder> rustc_serialize::Encodable<S> for AttrId {
}

impl<D: Decoder> rustc_serialize::Decodable<D> for AttrId {
fn decode(d: &mut D) -> Result<AttrId, D::Error> {
d.read_nil().map(|_| crate::attr::mk_attr_id())
fn decode(d: &mut D) -> AttrId {
d.read_unit();
crate::attr::mk_attr_id()
}
}

Expand Down
8 changes: 4 additions & 4 deletions compiler/rustc_ast/src/ptr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -115,8 +115,8 @@ impl<T> fmt::Pointer for P<T> {
}

impl<D: Decoder, T: 'static + Decodable<D>> Decodable<D> for P<T> {
fn decode(d: &mut D) -> Result<P<T>, D::Error> {
Decodable::decode(d).map(P)
fn decode(d: &mut D) -> P<T> {
P(Decodable::decode(d))
}
}

Expand Down Expand Up @@ -204,8 +204,8 @@ impl<S: Encoder, T: Encodable<S>> Encodable<S> for P<[T]> {
}

impl<D: Decoder, T: Decodable<D>> Decodable<D> for P<[T]> {
fn decode(d: &mut D) -> Result<P<[T]>, D::Error> {
Ok(P::from_vec(Decodable::decode(d)?))
fn decode(d: &mut D) -> P<[T]> {
P::from_vec(Decodable::decode(d))
}
}

Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_ast/src/tokenstream.rs
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ impl<S: Encoder> Encodable<S> for LazyTokenStream {
}

impl<D: Decoder> Decodable<D> for LazyTokenStream {
fn decode(_d: &mut D) -> Result<Self, D::Error> {
fn decode(_d: &mut D) -> Self {
panic!("Attempted to decode LazyTokenStream");
}
}
Expand Down
10 changes: 5 additions & 5 deletions compiler/rustc_data_structures/src/fingerprint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -149,10 +149,10 @@ impl<E: rustc_serialize::Encoder> Encodable<E> for Fingerprint {

impl<D: rustc_serialize::Decoder> Decodable<D> for Fingerprint {
#[inline]
fn decode(d: &mut D) -> Result<Self, D::Error> {
fn decode(d: &mut D) -> Self {
let mut bytes = [0u8; 16];
d.read_raw_bytes_into(&mut bytes)?;
Ok(Fingerprint::from_le_bytes(bytes))
d.read_raw_bytes_into(&mut bytes);
Fingerprint::from_le_bytes(bytes)
}
}

Expand Down Expand Up @@ -195,8 +195,8 @@ impl<E: rustc_serialize::Encoder> Encodable<E> for PackedFingerprint {

impl<D: rustc_serialize::Decoder> Decodable<D> for PackedFingerprint {
#[inline]
fn decode(d: &mut D) -> Result<Self, D::Error> {
Fingerprint::decode(d).map(PackedFingerprint)
fn decode(d: &mut D) -> Self {
Self(Fingerprint::decode(d))
}
}

Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_data_structures/src/svh.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,8 @@ impl<S: Encoder> Encodable<S> for Svh {
}

impl<D: Decoder> Decodable<D> for Svh {
fn decode(d: &mut D) -> Result<Svh, D::Error> {
d.read_u64().map(u64::from_le).map(Svh::new)
fn decode(d: &mut D) -> Svh {
Svh::new(u64::from_le(d.read_u64()))
}
}

Expand Down
5 changes: 1 addition & 4 deletions compiler/rustc_driver/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -597,10 +597,7 @@ impl RustcDefaultCalls {
let rlink_data = fs::read_to_string(file).unwrap_or_else(|err| {
sess.fatal(&format!("failed to read rlink file: {}", err));
});
let codegen_results: CodegenResults =
json::decode(&rlink_data).unwrap_or_else(|err| {
sess.fatal(&format!("failed to decode rlink: {}", err));
});
let codegen_results: CodegenResults = json::decode(&rlink_data);
let result = compiler.codegen_backend().link(sess, codegen_results, &outputs);
abort_on_err(result, sess);
} else {
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_errors/src/json/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ fn test_positions(code: &str, span: (u32, u32), expected_output: SpanTestData) {

let bytes = output.lock().unwrap();
let actual_output = str::from_utf8(&bytes).unwrap();
let actual_output: TestData = decode(actual_output).unwrap();
let actual_output: TestData = decode(actual_output);

assert_eq!(expected_output, actual_output)
})
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_errors/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -99,8 +99,8 @@ impl Hash for ToolMetadata {

// Doesn't really need to round-trip
impl<D: Decoder> Decodable<D> for ToolMetadata {
fn decode(_d: &mut D) -> Result<Self, D::Error> {
Ok(ToolMetadata(None))
fn decode(_d: &mut D) -> Self {
ToolMetadata(None)
}
}

Expand Down
15 changes: 3 additions & 12 deletions compiler/rustc_incremental/src/persist/load.rs
Original file line number Diff line number Diff line change
Expand Up @@ -158,14 +158,7 @@ pub fn load_dep_graph(sess: &Session) -> DepGraphFuture {
// Decode the list of work_products
let mut work_product_decoder = Decoder::new(&work_products_data[..], start_pos);
let work_products: Vec<SerializedWorkProduct> =
Decodable::decode(&mut work_product_decoder).unwrap_or_else(|e| {
let msg = format!(
"Error decoding `work-products` from incremental \
compilation session directory: {}",
e
);
sess.fatal(&msg)
});
Decodable::decode(&mut work_product_decoder);

for swp in work_products {
let mut all_files_exist = true;
Expand Down Expand Up @@ -203,8 +196,7 @@ pub fn load_dep_graph(sess: &Session) -> DepGraphFuture {
LoadResult::Error { message } => LoadResult::Error { message },
LoadResult::Ok { data: (bytes, start_pos) } => {
let mut decoder = Decoder::new(&bytes, start_pos);
let prev_commandline_args_hash = u64::decode(&mut decoder)
.expect("Error reading commandline arg hash from cached dep-graph");
let prev_commandline_args_hash = u64::decode(&mut decoder);

if prev_commandline_args_hash != expected_hash {
if report_incremental_info {
Expand All @@ -220,8 +212,7 @@ pub fn load_dep_graph(sess: &Session) -> DepGraphFuture {
return LoadResult::DataOutOfDate;
}

let dep_graph = SerializedDepGraph::decode(&mut decoder)
.expect("Error reading cached dep-graph");
let dep_graph = SerializedDepGraph::decode(&mut decoder);

LoadResult::Ok { data: (dep_graph, prev_work_products) }
}
Expand Down
8 changes: 4 additions & 4 deletions compiler/rustc_index/src/vec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -395,8 +395,8 @@ macro_rules! newtype_index {

(@serializable $type:ident) => (
impl<D: ::rustc_serialize::Decoder> ::rustc_serialize::Decodable<D> for $type {
fn decode(d: &mut D) -> Result<Self, D::Error> {
d.read_u32().map(Self::from_u32)
fn decode(d: &mut D) -> Self {
Self::from_u32(d.read_u32())
}
}
impl<E: ::rustc_serialize::Encoder> ::rustc_serialize::Encodable<E> for $type {
Expand Down Expand Up @@ -527,8 +527,8 @@ impl<S: Encoder, I: Idx, T: Encodable<S>> Encodable<S> for &IndexVec<I, T> {
}

impl<D: Decoder, I: Idx, T: Decodable<D>> Decodable<D> for IndexVec<I, T> {
fn decode(d: &mut D) -> Result<Self, D::Error> {
Decodable::decode(d).map(|v| IndexVec { raw: v, _marker: PhantomData })
fn decode(d: &mut D) -> Self {
IndexVec { raw: Decodable::decode(d), _marker: PhantomData }
}
}

Expand Down
18 changes: 5 additions & 13 deletions compiler/rustc_macros/src/serialize.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ fn decodable_body(
quote! {
::rustc_serialize::Decoder::read_struct(
__decoder,
|__decoder| { ::std::result::Result::Ok(#construct) },
|__decoder| { #construct },
)
}
}
Expand All @@ -57,7 +57,7 @@ fn decodable_body(
.enumerate()
.map(|(idx, vi)| {
let construct = vi.construct(|field, index| decode_field(field, index, false));
quote! { #idx => { ::std::result::Result::Ok(#construct) } }
quote! { #idx => { #construct } }
})
.collect();
let names: TokenStream = variants
Expand All @@ -82,8 +82,7 @@ fn decodable_body(
|__decoder, __variant_idx| {
match __variant_idx {
#match_inner
_ => return ::std::result::Result::Err(
::rustc_serialize::Decoder::error(__decoder, #message)),
_ => panic!(#message),
}
})
}
Expand All @@ -95,9 +94,7 @@ fn decodable_body(
s.bound_impl(
quote!(::rustc_serialize::Decodable<#decoder_ty>),
quote! {
fn decode(
__decoder: &mut #decoder_ty,
) -> ::std::result::Result<Self, <#decoder_ty as ::rustc_serialize::Decoder>::Error> {
fn decode(__decoder: &mut #decoder_ty) -> Self {
#decode_body
}
},
Expand Down Expand Up @@ -127,12 +124,7 @@ fn decode_field(field: &syn::Field, index: usize, is_struct: bool) -> proc_macro
#__decoder, #opt_field_name #decode_inner_method)
};

quote! {
match #decode_call {
::std::result::Result::Ok(__res) => __res,
::std::result::Result::Err(__err) => return ::std::result::Result::Err(__err),
}
}
quote! { #decode_call }
}

pub fn type_encodable_derive(mut s: synstructure::Structure<'_>) -> proc_macro2::TokenStream {
Expand Down
Loading

0 comments on commit 84322ef

Please sign in to comment.