From 1acbe7573dc32f917f51a784a36b7afc690900e3 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Tue, 7 Jun 2022 13:30:45 +1000 Subject: Use delayed error handling for `Encodable` and `Encoder` infallible. There are two impls of the `Encoder` trait: `opaque::Encoder` and `opaque::FileEncoder`. The former encodes into memory and is infallible, the latter writes to file and is fallible. Currently, standard `Result`/`?`/`unwrap` error handling is used, but this is a bit verbose and has non-trivial cost, which is annoying given how rare failures are (especially in the infallible `opaque::Encoder` case). This commit changes how `Encoder` fallibility is handled. All the `emit_*` methods are now infallible. `opaque::Encoder` requires no great changes for this. `opaque::FileEncoder` now implements a delayed error handling strategy. If a failure occurs, it records this via the `res` field, and all subsequent encoding operations are skipped if `res` indicates an error has occurred. Once encoding is complete, the new `finish` method is called, which returns a `Result`. In other words, there is now a single `Result`-producing method instead of many of them. This has very little effect on how any file errors are reported if `opaque::FileEncoder` has any failures. Much of this commit is boring mechanical changes, removing `Result` return values and `?` or `unwrap` from expressions. The more interesting parts are as follows. - serialize.rs: The `Encoder` trait gains an `Ok` associated type. The `into_inner` method is changed into `finish`, which returns `Result, !>`. - opaque.rs: The `FileEncoder` adopts the delayed error handling strategy. Its `Ok` type is a `usize`, returning the number of bytes written, replacing previous uses of `FileEncoder::position`. - Various methods that take an encoder now consume it, rather than being passed a mutable reference, e.g. `serialize_query_result_cache`. --- compiler/rustc_span/src/def_id.rs | 16 ++++++------ compiler/rustc_span/src/hygiene.rs | 47 +++++++++++++++------------------- compiler/rustc_span/src/lib.rs | 52 ++++++++++++++++++-------------------- compiler/rustc_span/src/symbol.rs | 4 +-- 4 files changed, 55 insertions(+), 64 deletions(-) (limited to 'compiler/rustc_span/src') diff --git a/compiler/rustc_span/src/def_id.rs b/compiler/rustc_span/src/def_id.rs index 2bd0880a7c4..a1533fe46b3 100644 --- a/compiler/rustc_span/src/def_id.rs +++ b/compiler/rustc_span/src/def_id.rs @@ -41,8 +41,8 @@ impl fmt::Display for CrateNum { /// As a local identifier, a `CrateNum` is only meaningful within its context, e.g. within a tcx. /// Therefore, make sure to include the context when encode a `CrateNum`. impl Encodable for CrateNum { - default fn encode(&self, s: &mut E) -> Result<(), E::Error> { - s.emit_u32(self.as_u32()) + default fn encode(&self, s: &mut E) { + s.emit_u32(self.as_u32()); } } @@ -203,7 +203,7 @@ rustc_index::newtype_index! { } impl Encodable for DefIndex { - default fn encode(&self, _: &mut E) -> Result<(), E::Error> { + default fn encode(&self, _: &mut E) { panic!("cannot encode `DefIndex` with `{}`", std::any::type_name::()); } } @@ -306,9 +306,9 @@ impl DefId { } impl Encodable for DefId { - default fn encode(&self, s: &mut E) -> Result<(), E::Error> { - self.krate.encode(s)?; - self.index.encode(s) + default fn encode(&self, s: &mut E) { + self.krate.encode(s); + self.index.encode(s); } } @@ -382,8 +382,8 @@ impl fmt::Debug for LocalDefId { } impl Encodable for LocalDefId { - fn encode(&self, s: &mut E) -> Result<(), E::Error> { - self.to_def_id().encode(s) + fn encode(&self, s: &mut E) { + self.to_def_id().encode(s); } } diff --git a/compiler/rustc_span/src/hygiene.rs b/compiler/rustc_span/src/hygiene.rs index 59f2badbabb..955db72157c 100644 --- a/compiler/rustc_span/src/hygiene.rs +++ b/compiler/rustc_span/src/hygiene.rs @@ -1189,12 +1189,12 @@ impl HygieneEncodeContext { } } - pub fn encode( + pub fn encode( &self, encoder: &mut T, - mut encode_ctxt: impl FnMut(&mut T, u32, &SyntaxContextData) -> Result<(), R>, - mut encode_expn: impl FnMut(&mut T, ExpnId, &ExpnData, ExpnHash) -> Result<(), R>, - ) -> Result<(), R> { + mut encode_ctxt: impl FnMut(&mut T, u32, &SyntaxContextData), + mut encode_expn: impl FnMut(&mut T, ExpnId, &ExpnData, ExpnHash), + ) { // When we serialize a `SyntaxContextData`, we may end up serializing // a `SyntaxContext` that we haven't seen before while !self.latest_ctxts.lock().is_empty() || !self.latest_expns.lock().is_empty() { @@ -1213,22 +1213,19 @@ impl HygieneEncodeContext { // order for_all_ctxts_in(latest_ctxts.into_iter(), |index, ctxt, data| { if self.serialized_ctxts.lock().insert(ctxt) { - encode_ctxt(encoder, index, data)?; + encode_ctxt(encoder, index, data); } - Ok(()) - })?; + }); let latest_expns = { std::mem::take(&mut *self.latest_expns.lock()) }; for_all_expns_in(latest_expns.into_iter(), |expn, data, hash| { if self.serialized_expns.lock().insert(expn) { - encode_expn(encoder, expn, data, hash)?; + encode_expn(encoder, expn, data, hash); } - Ok(()) - })?; + }); } debug!("encode_hygiene: Done serializing SyntaxContextData"); - Ok(()) } } @@ -1378,40 +1375,38 @@ pub fn decode_syntax_context SyntaxContext new_ctxt } -fn for_all_ctxts_in Result<(), E>>( +fn for_all_ctxts_in( ctxts: impl Iterator, mut f: F, -) -> Result<(), E> { +) { let all_data: Vec<_> = HygieneData::with(|data| { ctxts.map(|ctxt| (ctxt, data.syntax_context_data[ctxt.0 as usize].clone())).collect() }); for (ctxt, data) in all_data.into_iter() { - f(ctxt.0, ctxt, &data)?; + f(ctxt.0, ctxt, &data); } - Ok(()) } -fn for_all_expns_in( +fn for_all_expns_in( expns: impl Iterator, - mut f: impl FnMut(ExpnId, &ExpnData, ExpnHash) -> Result<(), E>, -) -> Result<(), E> { + mut f: impl FnMut(ExpnId, &ExpnData, ExpnHash), +) { let all_data: Vec<_> = HygieneData::with(|data| { expns.map(|expn| (expn, data.expn_data(expn).clone(), data.expn_hash(expn))).collect() }); for (expn, data, hash) in all_data.into_iter() { - f(expn, &data, hash)?; + f(expn, &data, hash); } - Ok(()) } impl Encodable for LocalExpnId { - fn encode(&self, e: &mut E) -> Result<(), E::Error> { - self.to_expn_id().encode(e) + fn encode(&self, e: &mut E) { + self.to_expn_id().encode(e); } } impl Encodable for ExpnId { - default fn encode(&self, _: &mut E) -> Result<(), E::Error> { + default fn encode(&self, _: &mut E) { panic!("cannot encode `ExpnId` with `{}`", std::any::type_name::()); } } @@ -1432,15 +1427,15 @@ pub fn raw_encode_syntax_context( ctxt: SyntaxContext, context: &HygieneEncodeContext, e: &mut E, -) -> Result<(), E::Error> { +) { if !context.serialized_ctxts.lock().contains(&ctxt) { context.latest_ctxts.lock().insert(ctxt); } - ctxt.0.encode(e) + ctxt.0.encode(e); } impl Encodable for SyntaxContext { - default fn encode(&self, _: &mut E) -> Result<(), E::Error> { + default fn encode(&self, _: &mut E) { panic!("cannot encode `SyntaxContext` with `{}`", std::any::type_name::()); } } diff --git a/compiler/rustc_span/src/lib.rs b/compiler/rustc_span/src/lib.rs index ae0228d6ea0..7f227217e3c 100644 --- a/compiler/rustc_span/src/lib.rs +++ b/compiler/rustc_span/src/lib.rs @@ -194,12 +194,10 @@ impl Hash for RealFileName { // This is functionally identical to #[derive(Encodable)], with the exception of // an added assert statement impl Encodable for RealFileName { - fn encode(&self, encoder: &mut S) -> Result<(), S::Error> { + fn encode(&self, encoder: &mut S) { match *self { RealFileName::LocalPath(ref local_path) => encoder.emit_enum_variant(0, |encoder| { - Ok({ - local_path.encode(encoder)?; - }) + local_path.encode(encoder); }), RealFileName::Remapped { ref local_path, ref virtual_name } => encoder @@ -207,9 +205,8 @@ impl Encodable for RealFileName { // For privacy and build reproducibility, we must not embed host-dependant path in artifacts // if they have been remapped by --remap-path-prefix assert!(local_path.is_none()); - local_path.encode(encoder)?; - virtual_name.encode(encoder)?; - Ok(()) + local_path.encode(encoder); + virtual_name.encode(encoder); }), } } @@ -946,10 +943,10 @@ impl Default for Span { } impl Encodable for Span { - default fn encode(&self, s: &mut E) -> Result<(), E::Error> { + default fn encode(&self, s: &mut E) { let span = self.data(); - span.lo.encode(s)?; - span.hi.encode(s) + span.lo.encode(s); + span.hi.encode(s); } } impl Decodable for Span { @@ -1297,17 +1294,17 @@ pub struct SourceFile { } impl Encodable for SourceFile { - fn encode(&self, s: &mut S) -> Result<(), S::Error> { - self.name.encode(s)?; - self.src_hash.encode(s)?; - self.start_pos.encode(s)?; - self.end_pos.encode(s)?; + fn encode(&self, s: &mut S) { + self.name.encode(s); + self.src_hash.encode(s); + self.start_pos.encode(s); + self.end_pos.encode(s); // We are always in `Lines` form by the time we reach here. assert!(self.lines.borrow().is_lines()); self.lines(|lines| { // Store the length. - s.emit_u32(lines.len() as u32)?; + s.emit_u32(lines.len() as u32); // Compute and store the difference list. if lines.len() != 0 { @@ -1329,10 +1326,10 @@ impl Encodable for SourceFile { }; // Encode the number of bytes used per diff. - s.emit_u8(bytes_per_diff as u8)?; + s.emit_u8(bytes_per_diff as u8); // Encode the first element. - lines[0].encode(s)?; + lines[0].encode(s); // Encode the difference list. let diff_iter = lines.array_windows().map(|&[fst, snd]| snd - fst); @@ -1359,16 +1356,15 @@ impl Encodable for SourceFile { } _ => unreachable!(), } - s.emit_raw_bytes(&raw_diffs)?; + s.emit_raw_bytes(&raw_diffs); } - Ok(()) - })?; + }); - self.multibyte_chars.encode(s)?; - self.non_narrow_chars.encode(s)?; - self.name_hash.encode(s)?; - self.normalized_pos.encode(s)?; - self.cnum.encode(s) + self.multibyte_chars.encode(s); + self.non_narrow_chars.encode(s); + self.name_hash.encode(s); + self.normalized_pos.encode(s); + self.cnum.encode(s); } } @@ -1916,8 +1912,8 @@ impl_pos! { } impl Encodable for BytePos { - fn encode(&self, s: &mut S) -> Result<(), S::Error> { - s.emit_u32(self.0) + fn encode(&self, s: &mut S) { + s.emit_u32(self.0); } } diff --git a/compiler/rustc_span/src/symbol.rs b/compiler/rustc_span/src/symbol.rs index bcaf53639cc..7b0fa65e808 100644 --- a/compiler/rustc_span/src/symbol.rs +++ b/compiler/rustc_span/src/symbol.rs @@ -1802,8 +1802,8 @@ impl fmt::Display for Symbol { } impl Encodable for Symbol { - fn encode(&self, s: &mut S) -> Result<(), S::Error> { - s.emit_str(self.as_str()) + fn encode(&self, s: &mut S) { + s.emit_str(self.as_str()); } } -- cgit 1.4.1-3-g733a5