Skip to content

Commit

Permalink
Auto merge of #47046 - Manishearth:intra-doc-links, r=eddyb,Guillaume…
Browse files Browse the repository at this point in the history
…Gomez,QuietMisdreavus,Manishearth

Implement RFC 1946 - intra-rustdoc links

rust-lang/rfcs#1946 #43466

Note for reviewers: The plain line counts are a little inflated because of how the markdown link parsing was done. [Read the file diff with "whitespace only" changes removed](https://github.com/rust-lang/rust/pull/47046/files?w=1) to get a better view of what actually changed there.

This pulls the name/path resolution mechanisms out of the compiler and runs it on the markdown in a crate's docs, so that links can be made to `SomeStruct` directly rather than finding the folder path to `struct.SomeStruct.html`. Check the `src/test/rustdoc/intra-paths.rs` test in this PR for a demo. The change was... a little invasive, but unlocks a really powerful mechanism for writing documentation that doesn't care about where an item was written to on the hard disk.

Items included:

 - [x] Make work with the hoedown renderer
 - [x] Handle relative paths
 - [x] Parse out the "path ambiguities" qualifiers (`[crate foo]`, `[struct Foo]`, `[foo()]`, `[static FOO]`, `[foo!]`, etc)
 - [x] Resolve foreign macros
 - [x] Resolve local macros
 - [x] Handle the use of inner/outer attributes giving different resolution scopes (handling for non-modules pushed to different PR)

Items not included:

 - [ ] Make sure cross-crate inlining works (blocked on refactor described in #47046 (comment))
 - [ ] Implied Shortcut Reference Links (where just doing `[::std::iter::Iterator][]` without a reference anchor will resolve using the reference name rather than the link target) (requires modifying the markdown parser - blocked on Hoedown/Pulldown switch and pulldown-cmark/pulldown-cmark#121)
 - [ ] Handle enum variants and UFCS methods (Enum variants link to the enum page, associated methods don't link at all)
 - [ ] Emit more warnings/errors when things fail to resolve (linking to a value-namespaced item without a qualifier will emit an error, otherwise the link is just treated as a url, not a rust path)
 - [ ] Give better spans for resolution errors (currently the span for the first doc comment is used)
 - [ ] Check for inner doc comments on things that aren't modules

I'm making the PR, but it should be noted that most of the work was done by Misdreavus 😄

(Editor's note: This has become a lie, check that commit log, Manish did a ton of work after this PR was opened `>_>`)
  • Loading branch information
bors committed Jan 23, 2018
2 parents 47a8eb7 + 63811b6 commit 48a7ea9
Show file tree
Hide file tree
Showing 17 changed files with 995 additions and 264 deletions.
16 changes: 6 additions & 10 deletions src/librustc/hir/lowering.rs
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,11 @@ pub trait Resolver {
/// We must keep the set of definitions up to date as we add nodes that weren't in the AST.
/// This should only return `None` during testing.
fn definitions(&mut self) -> &mut Definitions;

/// Given suffix ["b","c","d"], creates a HIR path for `[::crate_root]::b::c::d` and resolves
/// it based on `is_value`.
fn resolve_str_path(&mut self, span: Span, crate_root: Option<&str>,
components: &[&str], is_value: bool) -> hir::Path;
}

#[derive(Clone, Copy, Debug)]
Expand Down Expand Up @@ -3625,16 +3630,7 @@ impl<'a> LoweringContext<'a> {
/// `fld.cx.use_std`, and `::core::b::c::d` otherwise.
/// The path is also resolved according to `is_value`.
fn std_path(&mut self, span: Span, components: &[&str], is_value: bool) -> hir::Path {
let mut path = hir::Path {
span,
def: Def::Err,
segments: iter::once(keywords::CrateRoot.name()).chain({
self.crate_root.into_iter().chain(components.iter().cloned()).map(Symbol::intern)
}).map(hir::PathSegment::from_name).collect(),
};

self.resolver.resolve_hir_path(&mut path, is_value);
path
self.resolver.resolve_str_path(span, self.crate_root, components, is_value)
}

fn signal_block_expr(&mut self,
Expand Down
83 changes: 60 additions & 23 deletions src/librustc_driver/driver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ use rustc::util::common::{ErrorReported, time};
use rustc_allocator as allocator;
use rustc_borrowck as borrowck;
use rustc_incremental;
use rustc_resolve::{MakeGlobMap, Resolver};
use rustc_resolve::{MakeGlobMap, Resolver, ResolverArenas};
use rustc_metadata::creader::CrateLoader;
use rustc_metadata::cstore::{self, CStore};
use rustc_trans_utils::trans_crate::TransCrate;
Expand Down Expand Up @@ -139,6 +139,7 @@ pub fn compile_input(trans: Box<TransCrate>,

let crate_name =
::rustc_trans_utils::link::find_crate_name(Some(sess), &krate.attrs, input);

let ExpansionResult { expanded_crate, defs, analysis, resolutions, mut hir_forest } = {
phase_2_configure_and_expand(
sess,
Expand Down Expand Up @@ -562,6 +563,12 @@ pub struct ExpansionResult {
pub hir_forest: hir_map::Forest,
}

pub struct InnerExpansionResult<'a> {
pub expanded_crate: ast::Crate,
pub resolver: Resolver<'a>,
pub hir_forest: hir_map::Forest,
}

/// Run the "early phases" of the compiler: initial `cfg` processing,
/// loading compiler plugins (including those from `addl_plugins`),
/// syntax expansion, secondary `cfg` expansion, synthesis of a test
Expand All @@ -578,6 +585,55 @@ pub fn phase_2_configure_and_expand<F>(sess: &Session,
make_glob_map: MakeGlobMap,
after_expand: F)
-> Result<ExpansionResult, CompileIncomplete>
where F: FnOnce(&ast::Crate) -> CompileResult {
// Currently, we ignore the name resolution data structures for the purposes of dependency
// tracking. Instead we will run name resolution and include its output in the hash of each
// item, much like we do for macro expansion. In other words, the hash reflects not just
// its contents but the results of name resolution on those contents. Hopefully we'll push
// this back at some point.
let mut crate_loader = CrateLoader::new(sess, &cstore, &crate_name);
let resolver_arenas = Resolver::arenas();
let result = phase_2_configure_and_expand_inner(sess, cstore, krate, registry, crate_name,
addl_plugins, make_glob_map, &resolver_arenas,
&mut crate_loader, after_expand);
match result {
Ok(InnerExpansionResult {expanded_crate, resolver, hir_forest}) => {
Ok(ExpansionResult {
expanded_crate,
defs: resolver.definitions,
hir_forest,
resolutions: Resolutions {
freevars: resolver.freevars,
export_map: resolver.export_map,
trait_map: resolver.trait_map,
maybe_unused_trait_imports: resolver.maybe_unused_trait_imports,
maybe_unused_extern_crates: resolver.maybe_unused_extern_crates,
},

analysis: ty::CrateAnalysis {
access_levels: Rc::new(AccessLevels::default()),
name: crate_name.to_string(),
glob_map: if resolver.make_glob_map { Some(resolver.glob_map) } else { None },
},
})
}
Err(x) => Err(x)
}
}

/// Same as phase_2_configure_and_expand, but doesn't let you keep the resolver
/// around
pub fn phase_2_configure_and_expand_inner<'a, F>(sess: &'a Session,
cstore: &'a CStore,
krate: ast::Crate,
registry: Option<Registry>,
crate_name: &str,
addl_plugins: Option<Vec<String>>,
make_glob_map: MakeGlobMap,
resolver_arenas: &'a ResolverArenas<'a>,
crate_loader: &'a mut CrateLoader,
after_expand: F)
-> Result<InnerExpansionResult<'a>, CompileIncomplete>
where F: FnOnce(&ast::Crate) -> CompileResult,
{
let time_passes = sess.time_passes();
Expand Down Expand Up @@ -666,19 +722,12 @@ pub fn phase_2_configure_and_expand<F>(sess: &Session,
return Err(CompileIncomplete::Stopped);
}

// Currently, we ignore the name resolution data structures for the purposes of dependency
// tracking. Instead we will run name resolution and include its output in the hash of each
// item, much like we do for macro expansion. In other words, the hash reflects not just
// its contents but the results of name resolution on those contents. Hopefully we'll push
// this back at some point.
let mut crate_loader = CrateLoader::new(sess, &cstore, crate_name);
let resolver_arenas = Resolver::arenas();
let mut resolver = Resolver::new(sess,
cstore,
&krate,
crate_name,
make_glob_map,
&mut crate_loader,
crate_loader,
&resolver_arenas);
resolver.whitelisted_legacy_custom_derives = whitelisted_legacy_custom_derives;
syntax_ext::register_builtins(&mut resolver, syntax_exts, sess.features.borrow().quote);
Expand Down Expand Up @@ -855,21 +904,9 @@ pub fn phase_2_configure_and_expand<F>(sess: &Session,
syntax::ext::hygiene::clear_markings();
}

Ok(ExpansionResult {
Ok(InnerExpansionResult {
expanded_crate: krate,
defs: resolver.definitions,
analysis: ty::CrateAnalysis {
access_levels: Rc::new(AccessLevels::default()),
name: crate_name.to_string(),
glob_map: if resolver.make_glob_map { Some(resolver.glob_map) } else { None },
},
resolutions: Resolutions {
freevars: resolver.freevars,
export_map: resolver.export_map,
trait_map: resolver.trait_map,
maybe_unused_trait_imports: resolver.maybe_unused_trait_imports,
maybe_unused_extern_crates: resolver.maybe_unused_extern_crates,
},
resolver,
hir_forest,
})
}
Expand Down
89 changes: 75 additions & 14 deletions src/librustc_resolve/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ use std::cell::{Cell, RefCell};
use std::cmp;
use std::collections::BTreeSet;
use std::fmt;
use std::iter;
use std::mem::replace;
use std::rc::Rc;

Expand Down Expand Up @@ -1320,6 +1321,7 @@ pub struct Resolver<'a> {
crate_loader: &'a mut CrateLoader,
macro_names: FxHashSet<Ident>,
global_macros: FxHashMap<Name, &'a NameBinding<'a>>,
pub all_macros: FxHashMap<Name, Def>,
lexical_macro_resolutions: Vec<(Ident, &'a Cell<LegacyScope<'a>>)>,
macro_map: FxHashMap<DefId, Rc<SyntaxExtension>>,
macro_defs: FxHashMap<Mark, DefId>,
Expand Down Expand Up @@ -1407,6 +1409,71 @@ impl<'a, 'b: 'a> ty::DefIdTree for &'a Resolver<'b> {

impl<'a> hir::lowering::Resolver for Resolver<'a> {
fn resolve_hir_path(&mut self, path: &mut hir::Path, is_value: bool) {
self.resolve_hir_path_cb(path, is_value,
|resolver, span, error| resolve_error(resolver, span, error))
}

fn resolve_str_path(&mut self, span: Span, crate_root: Option<&str>,
components: &[&str], is_value: bool) -> hir::Path {
let mut path = hir::Path {
span,
def: Def::Err,
segments: iter::once(keywords::CrateRoot.name()).chain({
crate_root.into_iter().chain(components.iter().cloned()).map(Symbol::intern)
}).map(hir::PathSegment::from_name).collect(),
};

self.resolve_hir_path(&mut path, is_value);
path
}

fn get_resolution(&mut self, id: NodeId) -> Option<PathResolution> {
self.def_map.get(&id).cloned()
}

fn definitions(&mut self) -> &mut Definitions {
&mut self.definitions
}
}

impl<'a> Resolver<'a> {
/// Rustdoc uses this to resolve things in a recoverable way. ResolutionError<'a>
/// isn't something that can be returned because it can't be made to live that long,
/// and also it's a private type. Fortunately rustdoc doesn't need to know the error,
/// just that an error occured.
pub fn resolve_str_path_error(&mut self, span: Span, path_str: &str, is_value: bool)
-> Result<hir::Path, ()> {
use std::iter;
let mut errored = false;

let mut path = if path_str.starts_with("::") {
hir::Path {
span,
def: Def::Err,
segments: iter::once(keywords::CrateRoot.name()).chain({
path_str.split("::").skip(1).map(Symbol::intern)
}).map(hir::PathSegment::from_name).collect(),
}
} else {
hir::Path {
span,
def: Def::Err,
segments: path_str.split("::").map(Symbol::intern)
.map(hir::PathSegment::from_name).collect(),
}
};
self.resolve_hir_path_cb(&mut path, is_value, |_, _, _| errored = true);
if errored || path.def == Def::Err {
Err(())
} else {
Ok(path)
}
}

/// resolve_hir_path, but takes a callback in case there was an error
fn resolve_hir_path_cb<F>(&mut self, path: &mut hir::Path, is_value: bool, error_callback: F)
where F: for<'c, 'b> FnOnce(&'c mut Resolver, Span, ResolutionError<'b>)
{
let namespace = if is_value { ValueNS } else { TypeNS };
let hir::Path { ref segments, span, ref mut def } = *path;
let path: Vec<SpannedIdent> = segments.iter()
Expand All @@ -1418,24 +1485,16 @@ impl<'a> hir::lowering::Resolver for Resolver<'a> {
*def = path_res.base_def(),
PathResult::NonModule(..) => match self.resolve_path(&path, None, true, span) {
PathResult::Failed(span, msg, _) => {
resolve_error(self, span, ResolutionError::FailedToResolve(&msg));
error_callback(self, span, ResolutionError::FailedToResolve(&msg));
}
_ => {}
},
PathResult::Indeterminate => unreachable!(),
PathResult::Failed(span, msg, _) => {
resolve_error(self, span, ResolutionError::FailedToResolve(&msg));
error_callback(self, span, ResolutionError::FailedToResolve(&msg));
}
}
}

fn get_resolution(&mut self, id: NodeId) -> Option<PathResolution> {
self.def_map.get(&id).cloned()
}

fn definitions(&mut self) -> &mut Definitions {
&mut self.definitions
}
}

impl<'a> Resolver<'a> {
Expand Down Expand Up @@ -1538,6 +1597,7 @@ impl<'a> Resolver<'a> {
crate_loader,
macro_names: FxHashSet(),
global_macros: FxHashMap(),
all_macros: FxHashMap(),
lexical_macro_resolutions: Vec::new(),
macro_map: FxHashMap(),
macro_exports: Vec::new(),
Expand Down Expand Up @@ -1833,8 +1893,8 @@ impl<'a> Resolver<'a> {
// generate a fake "implementation scope" containing all the
// implementations thus found, for compatibility with old resolve pass.

fn with_scope<F>(&mut self, id: NodeId, f: F)
where F: FnOnce(&mut Resolver)
pub fn with_scope<F, T>(&mut self, id: NodeId, f: F) -> T
where F: FnOnce(&mut Resolver) -> T
{
let id = self.definitions.local_def_id(id);
let module = self.module_map.get(&id).cloned(); // clones a reference
Expand All @@ -1845,13 +1905,14 @@ impl<'a> Resolver<'a> {
self.ribs[TypeNS].push(Rib::new(ModuleRibKind(module)));

self.finalize_current_module_macro_resolutions();
f(self);
let ret = f(self);

self.current_module = orig_module;
self.ribs[ValueNS].pop();
self.ribs[TypeNS].pop();
ret
} else {
f(self);
f(self)
}
}

Expand Down
5 changes: 3 additions & 2 deletions src/librustc_resolve/macros.rs
Original file line number Diff line number Diff line change
Expand Up @@ -409,7 +409,7 @@ impl<'a> Resolver<'a> {
def
}

fn resolve_macro_to_def_inner(&mut self, scope: Mark, path: &ast::Path,
pub fn resolve_macro_to_def_inner(&mut self, scope: Mark, path: &ast::Path,
kind: MacroKind, force: bool)
-> Result<Def, Determinacy> {
let ast::Path { ref segments, span } = *path;
Expand Down Expand Up @@ -755,8 +755,9 @@ impl<'a> Resolver<'a> {
*legacy_scope = LegacyScope::Binding(self.arenas.alloc_legacy_binding(LegacyBinding {
parent: Cell::new(*legacy_scope), ident: ident, def_id: def_id, span: item.span,
}));
let def = Def::Macro(def_id, MacroKind::Bang);
self.all_macros.insert(ident.name, def);
if attr::contains_name(&item.attrs, "macro_export") {
let def = Def::Macro(def_id, MacroKind::Bang);
self.macro_exports.push(Export {
ident: ident.modern(),
def: def,
Expand Down
6 changes: 5 additions & 1 deletion src/librustdoc/clean/inline.rs
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,11 @@ pub fn record_extern_fqn(cx: &DocContext, did: DefId, kind: clean::TypeKind) {
None
}
});
let fqn = once(crate_name).chain(relative).collect();
let fqn = if let clean::TypeKind::Macro = kind {
vec![crate_name, relative.last().unwrap()]
} else {
once(crate_name).chain(relative).collect()
};
cx.renderinfo.borrow_mut().external_paths.insert(did, (fqn, kind));
}

Expand Down
Loading

0 comments on commit 48a7ea9

Please sign in to comment.