Skip to content

Commit

Permalink
Auto merge of #72121 - Aaron1011:final-hygiene-rebase, r=petrochenkov
Browse files Browse the repository at this point in the history
Serialize span hygiene data

Fixes #68686
Fixes #70963

This PR serializies global hygiene data into both the incremental compilation cache and the crate metadata. This allows hygiene information to be preserved across compilation sessions (both incremental and cross-crate).

When serializing a `SyntaxContext`, we simply write out the raw id from the current compilation session. Whenever we deserialize a `SyntaxContext`, we 'remap' the id to a fresh id in our current compilation session, and load the associated `SyntaxContextData`.

As a result, some 'upstream' `SyntaxContextData` will end up getting duplicated in 'downstream' crates. This only happens when we actually need to use an 'upstream' `SyntaxContext`, which occurs when we deserialize a `Span` that requires it.

We serialize an `ExpnData` into the metadata of the crate which generated it. An `ExpnId` is serialized as a reference into the crate which 'owns' the corresponding `ExpnData`, which avoids duplication in downstream crates.

I've included a macros 2.0 test which requires hygiene serialization to compile successfully.

TODO:

- [x] <strike>Determine how many additional `DefId`s we end up creating for `ExpnId`s - this may be significant for `libcore`, which uses macros heavily. Alternatively, we could try to compute a `DefPathHash` without making a corresponding `DefId` - however, this might significantly complicate the implementation.</strike> (We no longer create `DefId`s)
- [x] Investigate the overhead of duplicating `SyntaxContextData` in crate metadata.
- [x] Investigate how `resolve_crate_root` behaves with deserialized hygiene data - the current logic may be wrong.
- [x] Add additional tests. The effects of this PR are usually only noticeable when working with headache-inducing macro expansions (e.g. macros expanding to macros), so there are lots of corner cases to test.
- [x] Determine what to do about this:

https://github.com/rust-lang/rust/blob/4774f9b523c942cb5c0236542b5bcac76f6b6b9a/src/librustc_resolve/build_reduced_graph.rs#L892

- [x] Determine if we need to do anything here - I think the fact that `src/test/ui/hygiene/cross_crate_hygiene.rs` passes means that this is working.

https://github.com/rust-lang/rust/blob/3d5d0f898c2f3998e50c2180c6202f193c3acdbc/src/librustc_resolve/imports.rs#L1389-L1392
  • Loading branch information
bors committed Jul 27, 2020
2 parents c709862 + f7235a8 commit fa36f96
Show file tree
Hide file tree
Showing 46 changed files with 1,166 additions and 358 deletions.
3 changes: 2 additions & 1 deletion src/librustc_ast_lowering/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@ use rustc_data_structures::thin_vec::ThinVec;
use rustc_errors::struct_span_err;
use rustc_hir as hir;
use rustc_hir::def::Res;
use rustc_span::source_map::{respan, DesugaringKind, ForLoopLoc, Span, Spanned};
use rustc_span::hygiene::ForLoopLoc;
use rustc_span::source_map::{respan, DesugaringKind, Span, Spanned};
use rustc_span::symbol::{sym, Ident, Symbol};
use rustc_target::asm;
use std::collections::hash_map::Entry;
Expand Down
4 changes: 3 additions & 1 deletion src/librustc_expand/base.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ use rustc_data_structures::sync::{self, Lrc};
use rustc_errors::{DiagnosticBuilder, ErrorReported};
use rustc_parse::{self, nt_to_tokenstream, parser, MACRO_ARGUMENTS};
use rustc_session::{parse::ParseSess, Limit};
use rustc_span::def_id::DefId;
use rustc_span::def_id::{DefId, LOCAL_CRATE};
use rustc_span::edition::Edition;
use rustc_span::hygiene::{AstPass, ExpnData, ExpnId, ExpnKind};
use rustc_span::source_map::SourceMap;
Expand Down Expand Up @@ -873,6 +873,8 @@ impl SyntaxExtension {
local_inner_macros: self.local_inner_macros,
edition: self.edition,
macro_def_id,
krate: LOCAL_CRATE,
orig_id: None,
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/librustc_expand/expand.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use crate::base::*;
use crate::config::StripUnconfigured;
use crate::configure;
use crate::hygiene::{ExpnData, ExpnId, ExpnKind, SyntaxContext};
use crate::hygiene::{ExpnData, ExpnKind, SyntaxContext};
use crate::mbe::macro_rules::annotate_err_with_kind;
use crate::module::{parse_external_mod, push_directory, Directory, DirectoryOwnership};
use crate::placeholders::{placeholder, PlaceholderExpander};
Expand All @@ -28,7 +28,7 @@ use rustc_session::parse::{feature_err, ParseSess};
use rustc_session::Limit;
use rustc_span::source_map::respan;
use rustc_span::symbol::{sym, Ident, Symbol};
use rustc_span::{FileName, Span, DUMMY_SP};
use rustc_span::{ExpnId, FileName, Span, DUMMY_SP};

use smallvec::{smallvec, SmallVec};
use std::io::ErrorKind;
Expand Down
36 changes: 21 additions & 15 deletions src/librustc_metadata/creader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -307,11 +307,16 @@ impl<'a> CrateLoader<'a> {
let private_dep =
self.sess.opts.externs.get(&name.as_str()).map(|e| e.is_private_dep).unwrap_or(false);

info!("register crate `{}` (private_dep = {})", crate_root.name(), private_dep);

// Claim this crate number and cache it
let cnum = self.cstore.alloc_new_crate_num();

info!(
"register crate `{}` (cnum = {}. private_dep = {})",
crate_root.name(),
cnum,
private_dep
);

// Maintain a reference to the top most crate.
// Stash paths for top-most crate locally if necessary.
let crate_paths;
Expand Down Expand Up @@ -339,22 +344,21 @@ impl<'a> CrateLoader<'a> {
None
};

self.cstore.set_crate_data(
let crate_metadata = CrateMetadata::new(
self.sess,
metadata,
crate_root,
raw_proc_macros,
cnum,
CrateMetadata::new(
self.sess,
metadata,
crate_root,
raw_proc_macros,
cnum,
cnum_map,
dep_kind,
source,
private_dep,
host_hash,
),
cnum_map,
dep_kind,
source,
private_dep,
host_hash,
);

self.cstore.set_crate_data(cnum, crate_metadata);

Ok(cnum)
}

Expand Down Expand Up @@ -569,6 +573,8 @@ impl<'a> CrateLoader<'a> {
let cnum = self.maybe_resolve_crate(dep.name, dep_kind, Some((root, &dep)))?;
crate_num_map.push(cnum);
}

debug!("resolve_crate_deps: cnum_map for {:?} is {:?}", krate, crate_num_map);
Ok(crate_num_map)
}

Expand Down
1 change: 1 addition & 0 deletions src/librustc_metadata/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
#![feature(proc_macro_internals)]
#![feature(min_specialization)]
#![feature(stmt_expr_attributes)]
#![feature(never_type)]
#![recursion_limit = "256"]

extern crate proc_macro;
Expand Down
78 changes: 76 additions & 2 deletions src/librustc_metadata/rmeta/decoder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,18 +32,21 @@ use rustc_middle::ty::{self, Ty, TyCtxt};
use rustc_middle::util::common::record_time;
use rustc_serialize::{opaque, Decodable, Decoder, SpecializedDecoder, UseSpecializedDecodable};
use rustc_session::Session;
use rustc_span::hygiene::ExpnDataDecodeMode;
use rustc_span::source_map::{respan, Spanned};
use rustc_span::symbol::{sym, Ident, Symbol};
use rustc_span::{self, hygiene::MacroKind, BytePos, Pos, Span, DUMMY_SP};
use rustc_span::{self, hygiene::MacroKind, BytePos, ExpnId, Pos, Span, SyntaxContext, DUMMY_SP};

use log::debug;
use proc_macro::bridge::client::ProcMacro;
use std::cell::Cell;
use std::io;
use std::mem;
use std::num::NonZeroUsize;
use std::path::Path;

pub use cstore_impl::{provide, provide_extern};
use rustc_span::hygiene::HygieneDecodeContext;

mod cstore_impl;

Expand Down Expand Up @@ -106,6 +109,13 @@ crate struct CrateMetadata {
/// The hash for the host proc macro. Used to support `-Z dual-proc-macro`.
host_hash: Option<Svh>,

/// Additional data used for decoding `HygieneData` (e.g. `SyntaxContext`
/// and `ExpnId`).
/// Note that we store a `HygieneDecodeContext` for each `CrateMetadat`. This is
/// because `SyntaxContext` ids are not globally unique, so we need
/// to track which ids we've decoded on a per-crate basis.
hygiene_context: HygieneDecodeContext,

// --- Data used only for improving diagnostics ---
/// Information about the `extern crate` item or path that caused this crate to be loaded.
/// If this is `None`, then the crate was injected (e.g., by the allocator).
Expand Down Expand Up @@ -411,6 +421,7 @@ impl<'a, 'tcx> SpecializedDecoder<Span> for DecodeContext<'a, 'tcx> {

let lo = BytePos::decode(self)?;
let len = BytePos::decode(self)?;
let ctxt = SyntaxContext::decode(self)?;
let hi = lo + len;

let sess = if let Some(sess) = self.sess {
Expand Down Expand Up @@ -524,7 +535,7 @@ impl<'a, 'tcx> SpecializedDecoder<Span> for DecodeContext<'a, 'tcx> {
let hi =
(hi + source_file.translated_source_file.start_pos) - source_file.original_start_pos;

Ok(Span::with_root_ctxt(lo, hi))
Ok(Span::new(lo, hi, ctxt))
}
}

Expand Down Expand Up @@ -1120,6 +1131,14 @@ impl<'a, 'tcx> CrateMetadataRef<'a> {
!self.is_proc_macro(id) && self.root.tables.mir.get(self, id).is_some()
}

fn module_expansion(&self, id: DefIndex, sess: &Session) -> ExpnId {
if let EntryKind::Mod(m) = self.kind(id) {
m.decode((self, sess)).expansion
} else {
panic!("Expected module, found {:?}", self.local_def_id(id))
}
}

fn get_optimized_mir(&self, tcx: TyCtxt<'tcx>, id: DefIndex) -> Body<'tcx> {
self.root
.tables
Expand Down Expand Up @@ -1652,6 +1671,7 @@ impl CrateMetadata {
private_dep,
host_hash,
extern_crate: Lock::new(None),
hygiene_context: Default::default(),
}
}

Expand Down Expand Up @@ -1784,3 +1804,57 @@ fn macro_kind(raw: &ProcMacro) -> MacroKind {
ProcMacro::Bang { .. } => MacroKind::Bang,
}
}

impl<'a, 'tcx> SpecializedDecoder<SyntaxContext> for DecodeContext<'a, 'tcx> {
fn specialized_decode(&mut self) -> Result<SyntaxContext, Self::Error> {
let cdata = self.cdata();
let sess = self.sess.unwrap();
let cname = cdata.root.name;
rustc_span::hygiene::decode_syntax_context(self, &cdata.hygiene_context, |_, id| {
debug!("SpecializedDecoder<SyntaxContext>: decoding {}", id);
Ok(cdata
.root
.syntax_contexts
.get(&cdata, id)
.unwrap_or_else(|| panic!("Missing SyntaxContext {:?} for crate {:?}", id, cname))
.decode((&cdata, sess)))
})
}
}

impl<'a, 'tcx> SpecializedDecoder<ExpnId> for DecodeContext<'a, 'tcx> {
fn specialized_decode(&mut self) -> Result<ExpnId, Self::Error> {
let local_cdata = self.cdata();
let sess = self.sess.unwrap();
let expn_cnum = Cell::new(None);
let get_ctxt = |cnum| {
expn_cnum.set(Some(cnum));
if cnum == LOCAL_CRATE {
&local_cdata.hygiene_context
} else {
&local_cdata.cstore.get_crate_data(cnum).cdata.hygiene_context
}
};

rustc_span::hygiene::decode_expn_id(
self,
ExpnDataDecodeMode::Metadata(get_ctxt),
|_this, index| {
let cnum = expn_cnum.get().unwrap();
// Lookup local `ExpnData`s in our own crate data. Foreign `ExpnData`s
// are stored in the owning crate, to avoid duplication.
let crate_data = if cnum == LOCAL_CRATE {
local_cdata
} else {
local_cdata.cstore.get_crate_data(cnum)
};
Ok(crate_data
.root
.expn_data
.get(&crate_data, index)
.unwrap()
.decode((&crate_data, sess)))
},
)
}
}
15 changes: 7 additions & 8 deletions src/librustc_metadata/rmeta/decoder/cstore_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,10 @@ use rustc_middle::ty::{self, TyCtxt};
use rustc_session::utils::NativeLibKind;
use rustc_session::{CrateDisambiguator, Session};
use rustc_span::source_map::{self, Span, Spanned};
use rustc_span::symbol::{Ident, Symbol};
use rustc_span::symbol::Symbol;

use rustc_data_structures::sync::Lrc;
use rustc_span::ExpnId;
use smallvec::SmallVec;
use std::any::Any;

Expand Down Expand Up @@ -417,13 +418,7 @@ impl CStore {
attr::mark_used(attr);
}

let ident = data
.def_key(id.index)
.disambiguated_data
.data
.get_opt_name()
.map(Ident::with_dummy_span) // FIXME: cross-crate hygiene
.expect("no name in load_macro");
let ident = data.item_ident(id.index, sess);

LoadedMacro::MacroDef(
ast::Item {
Expand Down Expand Up @@ -454,6 +449,10 @@ impl CStore {
pub fn item_generics_num_lifetimes(&self, def_id: DefId, sess: &Session) -> usize {
self.get_crate_data(def_id.krate).get_generics(def_id.index, sess).own_counts().lifetimes
}

pub fn module_expansion_untracked(&self, def_id: DefId, sess: &Session) -> ExpnId {
self.get_crate_data(def_id.krate).module_expansion(def_id.index, sess)
}
}

impl CrateStore for CStore {
Expand Down
Loading

0 comments on commit fa36f96

Please sign in to comment.