bjoernager/rust
bjoernager/rust
1
Fork 0
Code Activity

Preserve SyntaxContext for invalid/dummy spans in crate metadata

Browse source 
Fixes #85197

We already preserved the `SyntaxContext` for invalid/dummy spans in the
incremental cache, but we weren't doing the same for crate metadata.
If an invalid (lo/hi from different files) span is written to the
incremental cache, we will decode it with a 'dummy' location, but keep
the original `SyntaxContext`. Since the crate metadata encoder was only
checking for `DUMMY_SP` (dummy location + root `SyntaxContext`),
the metadata encoder would treat it as a normal span, encoding the
`SyntaxContext`. As a result, the final span encoded to the metadata
would change across sessions, even if the crate itself was unchanged.

This PR updates our encoding of spans in the crate metadata to mirror
the encoding of spans into the incremental cache. We now always encode a
`SyntaxContext`, and encode location information for spans with a
non-dummy location.
This commit is contained in:
Aaron Hill 2021-05-12 00:10:41 -04:00
parent 70e52caed9
commit cdca3c81c1
No known key found for this signature in database
GPG key ID: B4087E510E98B164
7 changed files with 113 additions and 45 deletions
Download patch file Download diff file Expand all files Collapse all files

6
compiler/rustc_metadata/src/rmeta/decoder.rs
View file

@ -406,17 +406,17 @@ impl<'a, 'tcx> Decodable<DecodeContext<'a, 'tcx>> for ExpnId {
impl<'a, 'tcx> Decodable<DecodeContext<'a, 'tcx>> for Span {
fn decode(decoder: &mut DecodeContext<'a, 'tcx>) -> Result<Span, String> {
let ctxt = SyntaxContext::decode(decoder)?;
let tag = u8::decode(decoder)?;
if tag == TAG_INVALID_SPAN {
return Ok(DUMMY_SP);
if tag == TAG_PARTIAL_SPAN {
return Ok(DUMMY_SP.with_ctxt(ctxt));
}
debug_assert!(tag == TAG_VALID_SPAN_LOCAL || tag == TAG_VALID_SPAN_FOREIGN);
let lo = BytePos::decode(decoder)?;
let len = BytePos::decode(decoder)?;
let ctxt = SyntaxContext::decode(decoder)?;
let hi = lo + len;
let sess = if let Some(sess) = decoder.sess {

82
compiler/rustc_metadata/src/rmeta/encoder.rs
View file

@ -187,11 +187,48 @@ impl<'a, 'tcx> Encodable<EncodeContext<'a, 'tcx>> for ExpnId {
impl<'a, 'tcx> Encodable<EncodeContext<'a, 'tcx>> for Span {
fn encode(&self, s: &mut EncodeContext<'a, 'tcx>) -> opaque::EncodeResult {
if *self == rustc_span::DUMMY_SP {
return TAG_INVALID_SPAN.encode(s);
let span = self.data();
// Don't serialize any `SyntaxContext`s from a proc-macro crate,
// since we don't load proc-macro dependencies during serialization.
// This means that any hygiene information from macros used *within*
// a proc-macro crate (e.g. invoking a macro that expands to a proc-macro
// definition) will be lost.
//
// This can show up in two ways:
//
// 1. Any hygiene information associated with identifier of
// a proc macro (e.g. `#[proc_macro] pub fn $name`) will be lost.
// Since proc-macros can only be invoked from a different crate,
// real code should never need to care about this.
//
// 2. Using `Span::def_site` or `Span::mixed_site` will not
// include any hygiene information associated with the definition
// site. This means that a proc-macro cannot emit a `$crate`
// identifier which resolves to one of its dependencies,
// which also should never come up in practice.
//
// Additionally, this affects `Span::parent`, and any other
// span inspection APIs that would otherwise allow traversing
// the `SyntaxContexts` associated with a span.
//
// None of these user-visible effects should result in any
// cross-crate inconsistencies (getting one behavior in the same
// crate, and a different behavior in another crate) due to the
// limited surface that proc-macros can expose.
//
// IMPORTANT: If this is ever changed, be sure to update
// `rustc_span::hygiene::raw_encode_expn_id` to handle
// encoding `ExpnData` for proc-macro crates.
if s.is_proc_macro {
SyntaxContext::root().encode(s)?;
} else {
span.ctxt.encode(s)?;
}
let span = self.data();
if self.is_dummy() {
return TAG_PARTIAL_SPAN.encode(s);
}
// The Span infrastructure should make sure that this invariant holds:
debug_assert!(span.lo <= span.hi);
@ -206,7 +243,7 @@ impl<'a, 'tcx> Encodable<EncodeContext<'a, 'tcx>> for Span {
if !s.source_file_cache.0.contains(span.hi) {
// Unfortunately, macro expansion still sometimes generates Spans
// that malformed in this way.
return TAG_INVALID_SPAN.encode(s);
return TAG_PARTIAL_SPAN.encode(s);
}
let source_files = s.required_source_files.as_mut().expect("Already encoded SourceMap!");
@ -262,43 +299,6 @@ impl<'a, 'tcx> Encodable<EncodeContext<'a, 'tcx>> for Span {
let len = hi - lo;
len.encode(s)?;
// Don't serialize any `SyntaxContext`s from a proc-macro crate,
// since we don't load proc-macro dependencies during serialization.
// This means that any hygiene information from macros used *within*
// a proc-macro crate (e.g. invoking a macro that expands to a proc-macro
// definition) will be lost.
//
// This can show up in two ways:
//
// 1. Any hygiene information associated with identifier of
// a proc macro (e.g. `#[proc_macro] pub fn $name`) will be lost.
// Since proc-macros can only be invoked from a different crate,
// real code should never need to care about this.
//
// 2. Using `Span::def_site` or `Span::mixed_site` will not
// include any hygiene information associated with the definition
// site. This means that a proc-macro cannot emit a `$crate`
// identifier which resolves to one of its dependencies,
// which also should never come up in practice.
//
// Additionally, this affects `Span::parent`, and any other
// span inspection APIs that would otherwise allow traversing
// the `SyntaxContexts` associated with a span.
//
// None of these user-visible effects should result in any
// cross-crate inconsistencies (getting one behavior in the same
// crate, and a different behavior in another crate) due to the
// limited surface that proc-macros can expose.
//
// IMPORTANT: If this is ever changed, be sure to update
// `rustc_span::hygiene::raw_encode_expn_id` to handle
// encoding `ExpnData` for proc-macro crates.
if s.is_proc_macro {
SyntaxContext::root().encode(s)?;
} else {
span.ctxt.encode(s)?;
}
if tag == TAG_VALID_SPAN_FOREIGN {
// This needs to be two lines to avoid holding the `s.source_file_cache`
// while calling `cnum.encode(s)`

2
compiler/rustc_metadata/src/rmeta/mod.rs
View file

@ -451,4 +451,4 @@ struct GeneratorData<'tcx> {
// Tags used for encoding Spans:
const TAG_VALID_SPAN_LOCAL: u8 = 0;
const TAG_VALID_SPAN_FOREIGN: u8 = 1;
const TAG_INVALID_SPAN: u8 = 2;
const TAG_PARTIAL_SPAN: u8 = 2;