bjoernager/rust
bjoernager/rust
1
Fork 0
Code Activity

Use delayed error handling for Encodable and Encoder infallible.

Browse source 
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<Vec<u8>, !>`.
- 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`.
This commit is contained in:
Nicholas Nethercote 2022-06-07 13:30:45 +10:00
parent 582b9cbc45
commit 1acbe7573d
45 changed files with 611 additions and 682 deletions
Download patch file Download diff file Expand all files Collapse all files

16
compiler/rustc_span/src/def_id.rs
View file

@ -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<E: Encoder> Encodable<E> 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<E: Encoder> Encodable<E> 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::<E>());
}
}
@ -306,9 +306,9 @@ impl DefId {
}
impl<E: Encoder> Encodable<E> 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<E: Encoder> Encodable<E> 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);
}
}

47
compiler/rustc_span/src/hygiene.rs
View file

@ -1189,12 +1189,12 @@ impl HygieneEncodeContext {
}
}
pub fn encode<T, R>(
pub fn encode<T>(
&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<D: Decoder, F: FnOnce(&mut D, u32) -> SyntaxContext
new_ctxt
}
fn for_all_ctxts_in<E, F: FnMut(u32, SyntaxContext, &SyntaxContextData) -> Result<(), E>>(
fn for_all_ctxts_in<F: FnMut(u32, SyntaxContext, &SyntaxContextData)>(
ctxts: impl Iterator<Item = SyntaxContext>,
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<E>(
fn for_all_expns_in(
expns: impl Iterator<Item = ExpnId>,
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<E: Encoder> Encodable<E> 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<E: Encoder> Encodable<E> 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::<E>());
}
}
@ -1432,15 +1427,15 @@ pub fn raw_encode_syntax_context<E: Encoder>(
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<E: Encoder> Encodable<E> 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::<E>());
}
}

52
compiler/rustc_span/src/lib.rs
View file

@ -194,12 +194,10 @@ impl Hash for RealFileName {
// This is functionally identical to #[derive(Encodable)], with the exception of
// an added assert statement
impl<S: Encoder> Encodable<S> 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<S: Encoder> Encodable<S> 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<E: Encoder> Encodable<E> 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<D: Decoder> Decodable<D> for Span {
@ -1297,17 +1294,17 @@ pub struct SourceFile {
}
impl<S: Encoder> Encodable<S> 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<S: Encoder> Encodable<S> 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<S: Encoder> Encodable<S> 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<S: rustc_serialize::Encoder> Encodable<S> 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);
}
}

4
compiler/rustc_span/src/symbol.rs
View file

@ -1802,8 +1802,8 @@ impl fmt::Display for Symbol {
}
impl<S: Encoder> Encodable<S> 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());
}
}