From 473fcfd49a1bdb3981e7354165f98af04f847df1 Mon Sep 17 00:00:00 2001 From: QuietMisdreavus Date: Thu, 21 Dec 2017 14:54:16 -0600 Subject: [PATCH 01/44] new function to pull the links from a chunk of markdown --- src/librustdoc/html/markdown.rs | 417 +++++++++++++++++++------------- 1 file changed, 250 insertions(+), 167 deletions(-) diff --git a/src/librustdoc/html/markdown.rs b/src/librustdoc/html/markdown.rs index e66add20376fa..498c8911dc036 100644 --- a/src/librustdoc/html/markdown.rs +++ b/src/librustdoc/html/markdown.rs @@ -527,6 +527,7 @@ struct MyOpaque { *const hoedown_buffer, *const hoedown_renderer_data, libc::size_t), toc_builder: Option, + links: Option>, } extern { @@ -555,201 +556,203 @@ impl hoedown_buffer { } } -pub fn render(w: &mut fmt::Formatter, - s: &str, - print_toc: bool, - html_flags: libc::c_uint) -> fmt::Result { - extern fn block(ob: *mut hoedown_buffer, orig_text: *const hoedown_buffer, - lang: *const hoedown_buffer, data: *const hoedown_renderer_data, - line: libc::size_t) { - unsafe { - if orig_text.is_null() { return } - - let opaque = (*data).opaque as *mut hoedown_html_renderer_state; - let my_opaque: &MyOpaque = &*((*opaque).opaque as *const MyOpaque); - let text = (*orig_text).as_bytes(); - let origtext = str::from_utf8(text).unwrap(); - let origtext = origtext.trim_left(); - debug!("docblock: ==============\n{:?}\n=======", text); - let mut compile_fail = false; - let mut ignore = false; - - let rendered = if lang.is_null() || origtext.is_empty() { - false +extern fn hoedown_block(ob: *mut hoedown_buffer, orig_text: *const hoedown_buffer, + lang: *const hoedown_buffer, data: *const hoedown_renderer_data, + line: libc::size_t) { + unsafe { + if orig_text.is_null() { return } + + let opaque = (*data).opaque as *mut hoedown_html_renderer_state; + let my_opaque: &MyOpaque = &*((*opaque).opaque as *const MyOpaque); + let text = (*orig_text).as_bytes(); + let origtext = str::from_utf8(text).unwrap(); + let origtext = origtext.trim_left(); + debug!("docblock: ==============\n{:?}\n=======", text); + let mut compile_fail = false; + let mut ignore = false; + + let rendered = if lang.is_null() || origtext.is_empty() { + false + } else { + let rlang = (*lang).as_bytes(); + let rlang = str::from_utf8(rlang).unwrap(); + let parse_result = LangString::parse(rlang); + compile_fail = parse_result.compile_fail; + ignore = parse_result.ignore; + if !parse_result.rust { + (my_opaque.dfltblk)(ob, orig_text, lang, + opaque as *const hoedown_renderer_data, + line); + true } else { - let rlang = (*lang).as_bytes(); - let rlang = str::from_utf8(rlang).unwrap(); - let parse_result = LangString::parse(rlang); - compile_fail = parse_result.compile_fail; - ignore = parse_result.ignore; - if !parse_result.rust { - (my_opaque.dfltblk)(ob, orig_text, lang, - opaque as *const hoedown_renderer_data, - line); - true + false + } + }; + + let lines = origtext.lines().filter_map(|l| map_line(l).for_html()); + let text = lines.collect::>().join("\n"); + if rendered { return } + PLAYGROUND.with(|play| { + // insert newline to clearly separate it from the + // previous block so we can shorten the html output + let mut s = String::from("\n"); + let playground_button = play.borrow().as_ref().and_then(|&(ref krate, ref url)| { + if url.is_empty() { + return None; + } + let test = origtext.lines() + .map(|l| map_line(l).for_code()) + .collect::>().join("\n"); + let krate = krate.as_ref().map(|s| &**s); + let (test, _) = test::make_test(&test, krate, false, + &Default::default()); + let channel = if test.contains("#![feature(") { + "&version=nightly" } else { - false + "" + }; + // These characters don't need to be escaped in a URI. + // FIXME: use a library function for percent encoding. + fn dont_escape(c: u8) -> bool { + (b'a' <= c && c <= b'z') || + (b'A' <= c && c <= b'Z') || + (b'0' <= c && c <= b'9') || + c == b'-' || c == b'_' || c == b'.' || + c == b'~' || c == b'!' || c == b'\'' || + c == b'(' || c == b')' || c == b'*' } - }; - - let lines = origtext.lines().filter_map(|l| map_line(l).for_html()); - let text = lines.collect::>().join("\n"); - if rendered { return } - PLAYGROUND.with(|play| { - // insert newline to clearly separate it from the - // previous block so we can shorten the html output - let mut s = String::from("\n"); - let playground_button = play.borrow().as_ref().and_then(|&(ref krate, ref url)| { - if url.is_empty() { - return None; - } - let test = origtext.lines() - .map(|l| map_line(l).for_code()) - .collect::>().join("\n"); - let krate = krate.as_ref().map(|s| &**s); - let (test, _) = test::make_test(&test, krate, false, - &Default::default()); - let channel = if test.contains("#![feature(") { - "&version=nightly" + let mut test_escaped = String::new(); + for b in test.bytes() { + if dont_escape(b) { + test_escaped.push(char::from(b)); } else { - "" - }; - // These characters don't need to be escaped in a URI. - // FIXME: use a library function for percent encoding. - fn dont_escape(c: u8) -> bool { - (b'a' <= c && c <= b'z') || - (b'A' <= c && c <= b'Z') || - (b'0' <= c && c <= b'9') || - c == b'-' || c == b'_' || c == b'.' || - c == b'~' || c == b'!' || c == b'\'' || - c == b'(' || c == b')' || c == b'*' - } - let mut test_escaped = String::new(); - for b in test.bytes() { - if dont_escape(b) { - test_escaped.push(char::from(b)); - } else { - write!(test_escaped, "%{:02X}", b).unwrap(); - } + write!(test_escaped, "%{:02X}", b).unwrap(); } - Some(format!( - r#"Run"#, - url, test_escaped, channel - )) - }); - let tooltip = if ignore { - Some(("This example is not tested", "ignore")) - } else if compile_fail { - Some(("This example deliberately fails to compile", "compile_fail")) - } else { - None - }; - s.push_str(&highlight::render_with_highlighting( - &text, - Some(&format!("rust-example-rendered{}", - if ignore { " ignore" } - else if compile_fail { " compile_fail" } - else { "" })), - None, - playground_button.as_ref().map(String::as_str), - tooltip)); - hoedown_buffer_put(ob, s.as_ptr(), s.len()); - }) - } + } + Some(format!( + r#"Run"#, + url, test_escaped, channel + )) + }); + let tooltip = if ignore { + Some(("This example is not tested", "ignore")) + } else if compile_fail { + Some(("This example deliberately fails to compile", "compile_fail")) + } else { + None + }; + s.push_str(&highlight::render_with_highlighting( + &text, + Some(&format!("rust-example-rendered{}", + if ignore { " ignore" } + else if compile_fail { " compile_fail" } + else { "" })), + None, + playground_button.as_ref().map(String::as_str), + tooltip)); + hoedown_buffer_put(ob, s.as_ptr(), s.len()); + }) } +} - extern fn header(ob: *mut hoedown_buffer, text: *const hoedown_buffer, - level: libc::c_int, data: *const hoedown_renderer_data, - _: libc::size_t) { - // hoedown does this, we may as well too - unsafe { hoedown_buffer_put(ob, "\n".as_ptr(), 1); } +extern fn hoedown_header(ob: *mut hoedown_buffer, text: *const hoedown_buffer, + level: libc::c_int, data: *const hoedown_renderer_data, + _: libc::size_t) { + // hoedown does this, we may as well too + unsafe { hoedown_buffer_put(ob, "\n".as_ptr(), 1); } - // Extract the text provided - let s = if text.is_null() { - "".to_owned() - } else { - let s = unsafe { (*text).as_bytes() }; - str::from_utf8(&s).unwrap().to_owned() - }; + // Extract the text provided + let s = if text.is_null() { + "".to_owned() + } else { + let s = unsafe { (*text).as_bytes() }; + str::from_utf8(&s).unwrap().to_owned() + }; - // Discard '', '' tags and some escaped characters, - // transform the contents of the header into a hyphenated string - // without non-alphanumeric characters other than '-' and '_'. - // - // This is a terrible hack working around how hoedown gives us rendered - // html for text rather than the raw text. - let mut id = s.clone(); - let repl_sub = vec!["", "", "", "", - "", "", - "<", ">", "&", "'", """]; - for sub in repl_sub { - id = id.replace(sub, ""); - } - let id = id.chars().filter_map(|c| { - if c.is_alphanumeric() || c == '-' || c == '_' { - if c.is_ascii() { - Some(c.to_ascii_lowercase()) - } else { - Some(c) - } - } else if c.is_whitespace() && c.is_ascii() { - Some('-') + // Discard '', '' tags and some escaped characters, + // transform the contents of the header into a hyphenated string + // without non-alphanumeric characters other than '-' and '_'. + // + // This is a terrible hack working around how hoedown gives us rendered + // html for text rather than the raw text. + let mut id = s.clone(); + let repl_sub = vec!["", "", "", "", + "", "", + "<", ">", "&", "'", """]; + for sub in repl_sub { + id = id.replace(sub, ""); + } + let id = id.chars().filter_map(|c| { + if c.is_alphanumeric() || c == '-' || c == '_' { + if c.is_ascii() { + Some(c.to_ascii_lowercase()) } else { - None + Some(c) } - }).collect::(); + } else if c.is_whitespace() && c.is_ascii() { + Some('-') + } else { + None + } + }).collect::(); - let opaque = unsafe { (*data).opaque as *mut hoedown_html_renderer_state }; - let opaque = unsafe { &mut *((*opaque).opaque as *mut MyOpaque) }; + let opaque = unsafe { (*data).opaque as *mut hoedown_html_renderer_state }; + let opaque = unsafe { &mut *((*opaque).opaque as *mut MyOpaque) }; - let id = derive_id(id); + let id = derive_id(id); - let sec = opaque.toc_builder.as_mut().map_or("".to_owned(), |builder| { - format!("{} ", builder.push(level as u32, s.clone(), id.clone())) - }); + let sec = opaque.toc_builder.as_mut().map_or("".to_owned(), |builder| { + format!("{} ", builder.push(level as u32, s.clone(), id.clone())) + }); - // Render the HTML - let text = format!("\ - {sec}{}", - s, lvl = level, id = id, sec = sec); + // Render the HTML + let text = format!("\ + {sec}{}", + s, lvl = level, id = id, sec = sec); - unsafe { hoedown_buffer_put(ob, text.as_ptr(), text.len()); } - } + unsafe { hoedown_buffer_put(ob, text.as_ptr(), text.len()); } +} - extern fn codespan( - ob: *mut hoedown_buffer, - text: *const hoedown_buffer, - _: *const hoedown_renderer_data, - _: libc::size_t - ) -> libc::c_int { - let content = if text.is_null() { - "".to_owned() - } else { - let bytes = unsafe { (*text).as_bytes() }; - let s = str::from_utf8(bytes).unwrap(); - collapse_whitespace(s) - }; +extern fn hoedown_codespan( + ob: *mut hoedown_buffer, + text: *const hoedown_buffer, + _: *const hoedown_renderer_data, + _: libc::size_t +) -> libc::c_int { + let content = if text.is_null() { + "".to_owned() + } else { + let bytes = unsafe { (*text).as_bytes() }; + let s = str::from_utf8(bytes).unwrap(); + collapse_whitespace(s) + }; - let content = format!("{}", Escape(&content)); - unsafe { - hoedown_buffer_put(ob, content.as_ptr(), content.len()); - } - // Return anything except 0, which would mean "also print the code span verbatim". - 1 + let content = format!("{}", Escape(&content)); + unsafe { + hoedown_buffer_put(ob, content.as_ptr(), content.len()); } + // Return anything except 0, which would mean "also print the code span verbatim". + 1 +} + +pub fn render(w: &mut fmt::Formatter, + s: &str, + print_toc: bool, + html_flags: libc::c_uint) -> fmt::Result { unsafe { let ob = hoedown_buffer_new(DEF_OUNIT); let renderer = hoedown_html_renderer_new(html_flags, 0); let mut opaque = MyOpaque { dfltblk: (*renderer).blockcode.unwrap(), - toc_builder: if print_toc {Some(TocBuilder::new())} else {None} + toc_builder: if print_toc {Some(TocBuilder::new())} else {None}, + links: None, }; (*((*renderer).opaque as *mut hoedown_html_renderer_state)).opaque = &mut opaque as *mut _ as *mut libc::c_void; - (*renderer).blockcode = Some(block); - (*renderer).header = Some(header); - (*renderer).codespan = Some(codespan); + (*renderer).blockcode = Some(hoedown_block); + (*renderer).header = Some(hoedown_header); + (*renderer).codespan = Some(hoedown_codespan); let document = hoedown_document_new(renderer, HOEDOWN_EXTENSIONS, 16); hoedown_document_render(document, ob, s.as_ptr(), @@ -1140,6 +1143,86 @@ pub fn plain_summary_line(md: &str) -> String { s } +pub fn markdown_links(md: &str, render_type: RenderType) -> Vec { + if md.is_empty() { + return vec![]; + } + + match render_type { + RenderType::Hoedown => { + extern fn hoedown_link( + _ob: *mut hoedown_buffer, + _content: *const hoedown_buffer, + link: *const hoedown_buffer, + _title: *const hoedown_buffer, + data: *const hoedown_renderer_data, + _line: libc::size_t + ) -> libc::c_int { + if link.is_null() { + return 0; + } + + let opaque = unsafe { (*data).opaque as *mut hoedown_html_renderer_state }; + let opaque = unsafe { &mut *((*opaque).opaque as *mut MyOpaque) }; + + if let Some(ref mut links) = opaque.links { + let s = unsafe { (*link).as_bytes() }; + let s = str::from_utf8(&s).unwrap().to_owned(); + + links.push(s); + } + + //returning 0 here means "emit the span verbatim", but we're not using the output + //anyway so we don't really care + 0 + } + + unsafe { + let ob = hoedown_buffer_new(DEF_OUNIT); + let renderer = hoedown_html_renderer_new(0, 0); + let mut opaque = MyOpaque { + dfltblk: (*renderer).blockcode.unwrap(), + toc_builder: None, + links: Some(vec![]), + }; + (*((*renderer).opaque as *mut hoedown_html_renderer_state)).opaque + = &mut opaque as *mut _ as *mut libc::c_void; + (*renderer).blockcode = Some(hoedown_block); + (*renderer).header = Some(hoedown_header); + (*renderer).codespan = Some(hoedown_codespan); + (*renderer).link = Some(hoedown_link); + + let document = hoedown_document_new(renderer, HOEDOWN_EXTENSIONS, 16); + hoedown_document_render(document, ob, md.as_ptr(), + md.len() as libc::size_t); + hoedown_document_free(document); + + hoedown_html_renderer_free(renderer); + + opaque.links.unwrap() + } + } + RenderType::Pulldown => { + let mut opts = Options::empty(); + opts.insert(OPTION_ENABLE_TABLES); + opts.insert(OPTION_ENABLE_FOOTNOTES); + + let p = Parser::new_ext(md, opts); + + let iter = Footnotes::new(CodeBlocks::new(HeadingLinks::new(p, None))); + let mut links = vec![]; + + for ev in iter { + if let Event::Start(Tag::Link(dest, _)) = ev { + links.push(dest.into_owned()); + } + } + + links + } + } +} + #[cfg(test)] mod tests { use super::{LangString, Markdown, MarkdownHtml}; From 5db40f775415a9c006a14ebe65b88bcee31976ea Mon Sep 17 00:00:00 2001 From: QuietMisdreavus Date: Thu, 21 Dec 2017 15:15:07 -0600 Subject: [PATCH 02/44] add RenderType to DocContext --- src/librustdoc/core.rs | 7 ++++++- src/librustdoc/lib.rs | 7 ++++++- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/src/librustdoc/core.rs b/src/librustdoc/core.rs index ef7d5b5ff84af..6cbbe1fdf44fa 100644 --- a/src/librustdoc/core.rs +++ b/src/librustdoc/core.rs @@ -35,6 +35,7 @@ use std::path::PathBuf; use visit_ast::RustdocVisitor; use clean; use clean::Clean; +use html::markdown::RenderType; use html::render::RenderInfo; pub use rustc::session::config::Input; @@ -54,6 +55,8 @@ pub struct DocContext<'a, 'tcx: 'a> { pub renderinfo: RefCell, /// Later on moved through `clean::Crate` into `html::render::CACHE_KEY` pub external_traits: RefCell>, + /// Which markdown renderer to use when extracting links. + pub render_type: RenderType, // The current set of type and lifetime substitutions, // for expanding type aliases at the HIR level: @@ -104,7 +107,8 @@ pub fn run_core(search_paths: SearchPaths, triple: Option, maybe_sysroot: Option, allow_warnings: bool, - force_unstable_if_unmarked: bool) -> (clean::Crate, RenderInfo) + force_unstable_if_unmarked: bool, + render_type: RenderType) -> (clean::Crate, RenderInfo) { // Parse, resolve, and typecheck the given crate. @@ -207,6 +211,7 @@ pub fn run_core(search_paths: SearchPaths, access_levels: RefCell::new(access_levels), external_traits: Default::default(), renderinfo: Default::default(), + render_type, ty_substs: Default::default(), lt_substs: Default::default(), }; diff --git a/src/librustdoc/lib.rs b/src/librustdoc/lib.rs index 2e2dba7681cc3..01c2a5620da25 100644 --- a/src/librustdoc/lib.rs +++ b/src/librustdoc/lib.rs @@ -503,6 +503,11 @@ where R: 'static + Send, F: 'static + Send + FnOnce(Output) -> R { let crate_name = matches.opt_str("crate-name"); let crate_version = matches.opt_str("crate-version"); let plugin_path = matches.opt_str("plugin-path"); + let render_type = if matches.opt_present("enable-commonmark") { + RenderType::Pulldown + } else { + RenderType::Hoedown + }; info!("starting to run rustc"); let display_warnings = matches.opt_present("display-warnings"); @@ -517,7 +522,7 @@ where R: 'static + Send, F: 'static + Send + FnOnce(Output) -> R { let (mut krate, renderinfo) = core::run_core(paths, cfgs, externs, Input::File(cratefile), triple, maybe_sysroot, - display_warnings, force_unstable_if_unmarked); + display_warnings, force_unstable_if_unmarked, render_type); info!("finished with rustc"); From d9c1a17eecf7539368e541958f95562a81fd600a Mon Sep 17 00:00:00 2001 From: QuietMisdreavus Date: Thu, 21 Dec 2017 15:15:48 -0600 Subject: [PATCH 03/44] give render_text a generic return type --- src/librustdoc/html/render.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/librustdoc/html/render.rs b/src/librustdoc/html/render.rs index cfa09ea30a8be..f335e073d5691 100644 --- a/src/librustdoc/html/render.rs +++ b/src/librustdoc/html/render.rs @@ -421,7 +421,7 @@ thread_local!(pub static CURRENT_LOCATION_KEY: RefCell> = thread_local!(pub static USED_ID_MAP: RefCell> = RefCell::new(init_ids())); -pub fn render_text String>(mut render: F) -> (String, String) { +pub fn render_text T>(mut render: F) -> (T, T) { // Save the state of USED_ID_MAP so it only gets updated once even // though we're rendering twice. let orig_used_id_map = USED_ID_MAP.with(|map| map.borrow().clone()); From 76f831647ade4a5e82244bd4d19da308478cc83d Mon Sep 17 00:00:00 2001 From: QuietMisdreavus Date: Thu, 21 Dec 2017 15:16:29 -0600 Subject: [PATCH 04/44] add a rustc_resolve::Resolver to DocContext --- src/librustdoc/clean/mod.rs | 2 +- src/librustdoc/core.rs | 27 ++++++++++++++++++++++++--- src/librustdoc/visit_ast.rs | 8 ++++---- src/librustdoc/visit_lib.rs | 8 ++++---- 4 files changed, 33 insertions(+), 12 deletions(-) diff --git a/src/librustdoc/clean/mod.rs b/src/librustdoc/clean/mod.rs index cc75664cacbcc..6b58decd0b5d8 100644 --- a/src/librustdoc/clean/mod.rs +++ b/src/librustdoc/clean/mod.rs @@ -124,7 +124,7 @@ pub struct Crate { pub masked_crates: FxHashSet, } -impl<'a, 'tcx> Clean for visit_ast::RustdocVisitor<'a, 'tcx> { +impl<'a, 'tcx, 'rcx> Clean for visit_ast::RustdocVisitor<'a, 'tcx, 'rcx> { fn clean(&self, cx: &DocContext) -> Crate { use ::visit_lib::LibEmbargoVisitor; diff --git a/src/librustdoc/core.rs b/src/librustdoc/core.rs index 6cbbe1fdf44fa..b094567b15ea5 100644 --- a/src/librustdoc/core.rs +++ b/src/librustdoc/core.rs @@ -20,6 +20,7 @@ use rustc::lint; use rustc::util::nodemap::FxHashMap; use rustc_trans; use rustc_resolve as resolve; +use rustc_metadata::creader::CrateLoader; use rustc_metadata::cstore::CStore; use syntax::codemap; @@ -43,8 +44,9 @@ pub use rustc::session::search_paths::SearchPaths; pub type ExternalPaths = FxHashMap, clean::TypeKind)>; -pub struct DocContext<'a, 'tcx: 'a> { +pub struct DocContext<'a, 'tcx: 'a, 'rcx> { pub tcx: TyCtxt<'a, 'tcx, 'tcx>, + pub resolver: resolve::Resolver<'rcx>, pub populated_all_crate_impls: Cell, // Note that external items for which `doc(hidden)` applies to are shown as // non-reachable while local items aren't. This is because we're reusing @@ -67,7 +69,7 @@ pub struct DocContext<'a, 'tcx: 'a> { pub lt_substs: RefCell>, } -impl<'a, 'tcx> DocContext<'a, 'tcx> { +impl<'a, 'tcx, 'rcx> DocContext<'a, 'tcx, 'rcx> { pub fn sess(&self) -> &session::Session { &self.tcx.sess } @@ -160,7 +162,13 @@ pub fn run_core(search_paths: SearchPaths, let name = ::rustc_trans_utils::link::find_crate_name(Some(&sess), &krate.attrs, &input); - let driver::ExpansionResult { defs, analysis, resolutions, mut hir_forest, .. } = { + let driver::ExpansionResult { + expanded_crate, + defs, + analysis, + resolutions, + mut hir_forest + } = { let result = driver::phase_2_configure_and_expand(&sess, &cstore, krate, @@ -173,6 +181,8 @@ pub fn run_core(search_paths: SearchPaths, }; let arenas = AllArenas::new(); + let mut crate_loader = CrateLoader::new(&sess, &cstore, &name); + let resolver_arenas = resolve::Resolver::arenas(); let hir_map = hir_map::map_crate(&sess, &*cstore, &mut hir_forest, &defs); let output_filenames = driver::build_output_filenames(&input, &None, @@ -205,8 +215,19 @@ pub fn run_core(search_paths: SearchPaths, .collect() }; + // Set up a Resolver so that the doc cleaning can look up paths in the docs + let mut resolver = resolve::Resolver::new(&sess, + &*cstore, + &expanded_crate, + &name, + resolve::MakeGlobMap::No, + &mut crate_loader, + &resolver_arenas); + resolver.resolve_crate(&expanded_crate); + let ctxt = DocContext { tcx, + resolver, populated_all_crate_impls: Cell::new(false), access_levels: RefCell::new(access_levels), external_traits: Default::default(), diff --git a/src/librustdoc/visit_ast.rs b/src/librustdoc/visit_ast.rs index 1cb52d735bb18..23a2208292a36 100644 --- a/src/librustdoc/visit_ast.rs +++ b/src/librustdoc/visit_ast.rs @@ -40,11 +40,11 @@ use doctree::*; // also, is there some reason that this doesn't use the 'visit' // framework from syntax? -pub struct RustdocVisitor<'a, 'tcx: 'a> { +pub struct RustdocVisitor<'a, 'tcx: 'a, 'rcx: 'a> { cstore: &'tcx CrateStore, pub module: Module, pub attrs: hir::HirVec, - pub cx: &'a core::DocContext<'a, 'tcx>, + pub cx: &'a core::DocContext<'a, 'tcx, 'rcx>, view_item_stack: FxHashSet, inlining: bool, /// Is the current module and all of its parents public? @@ -52,9 +52,9 @@ pub struct RustdocVisitor<'a, 'tcx: 'a> { reexported_macros: FxHashSet, } -impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> { +impl<'a, 'tcx, 'rcx> RustdocVisitor<'a, 'tcx, 'rcx> { pub fn new(cstore: &'tcx CrateStore, - cx: &'a core::DocContext<'a, 'tcx>) -> RustdocVisitor<'a, 'tcx> { + cx: &'a core::DocContext<'a, 'tcx, 'rcx>) -> RustdocVisitor<'a, 'tcx, 'rcx> { // If the root is re-exported, terminate all recursion. let mut stack = FxHashSet(); stack.insert(ast::CRATE_NODE_ID); diff --git a/src/librustdoc/visit_lib.rs b/src/librustdoc/visit_lib.rs index 2fd47fa0a6d0a..7da0a7bfe6ea8 100644 --- a/src/librustdoc/visit_lib.rs +++ b/src/librustdoc/visit_lib.rs @@ -22,8 +22,8 @@ use clean::{AttributesExt, NestedAttributesExt}; /// Similar to `librustc_privacy::EmbargoVisitor`, but also takes /// specific rustdoc annotations into account (i.e. `doc(hidden)`) -pub struct LibEmbargoVisitor<'a, 'b: 'a, 'tcx: 'b> { - cx: &'a ::core::DocContext<'b, 'tcx>, +pub struct LibEmbargoVisitor<'a, 'b: 'a, 'tcx: 'b, 'rcx: 'a> { + cx: &'a ::core::DocContext<'b, 'tcx, 'rcx>, // Accessibility levels for reachable nodes access_levels: RefMut<'a, AccessLevels>, // Previous accessibility level, None means unreachable @@ -32,8 +32,8 @@ pub struct LibEmbargoVisitor<'a, 'b: 'a, 'tcx: 'b> { visited_mods: FxHashSet, } -impl<'a, 'b, 'tcx> LibEmbargoVisitor<'a, 'b, 'tcx> { - pub fn new(cx: &'a ::core::DocContext<'b, 'tcx>) -> LibEmbargoVisitor<'a, 'b, 'tcx> { +impl<'a, 'b, 'tcx, 'rcx> LibEmbargoVisitor<'a, 'b, 'tcx, 'rcx> { + pub fn new(cx: &'a ::core::DocContext<'b, 'tcx, 'rcx>) -> LibEmbargoVisitor<'a, 'b, 'tcx, 'rcx> { LibEmbargoVisitor { cx, access_levels: cx.access_levels.borrow_mut(), From c3d0d5a4bb905fbefaaa8034ce5bef690923a8fb Mon Sep 17 00:00:00 2001 From: QuietMisdreavus Date: Fri, 22 Dec 2017 13:12:54 -0600 Subject: [PATCH 05/44] resolve paths when cleaning docs --- src/librustdoc/clean/mod.rs | 43 ++++++++++++++++++++++++++++++--- src/librustdoc/core.rs | 4 +-- src/librustdoc/html/markdown.rs | 3 +++ 3 files changed, 45 insertions(+), 5 deletions(-) diff --git a/src/librustdoc/clean/mod.rs b/src/librustdoc/clean/mod.rs index 6b58decd0b5d8..3f7750cacc2cf 100644 --- a/src/librustdoc/clean/mod.rs +++ b/src/librustdoc/clean/mod.rs @@ -23,8 +23,9 @@ use syntax::abi::Abi; use syntax::ast; use syntax::attr; use syntax::codemap::Spanned; +use syntax::feature_gate::UnstableFeatures; use syntax::ptr::P; -use syntax::symbol::keywords; +use syntax::symbol::{keywords, Symbol}; use syntax_pos::{self, DUMMY_SP, Pos, FileName}; use rustc::middle::const_val::ConstVal; @@ -33,6 +34,7 @@ use rustc::middle::resolve_lifetime as rl; use rustc::middle::lang_items; use rustc::hir::def::{Def, CtorKind}; use rustc::hir::def_id::{CrateNum, DefId, CRATE_DEF_INDEX, LOCAL_CRATE}; +use rustc::hir::lowering::Resolver; use rustc::ty::subst::Substs; use rustc::ty::{self, Ty, AdtKind}; use rustc::middle::stability; @@ -43,7 +45,7 @@ use rustc::hir; use rustc_const_math::ConstInt; use std::default::Default; -use std::{mem, slice, vec}; +use std::{mem, slice, vec, iter}; use std::iter::FromIterator; use std::rc::Rc; use std::sync::Arc; @@ -53,6 +55,7 @@ use core::DocContext; use doctree; use visit_ast; use html::item_type::ItemType; +use html::markdown::markdown_links; pub mod inline; pub mod cfg; @@ -633,6 +636,7 @@ pub struct Attributes { pub other_attrs: Vec, pub cfg: Option>, pub span: Option, + pub links: Vec<(String, DefId)>, } impl Attributes { @@ -762,11 +766,13 @@ impl Attributes { Some(attr.clone()) }) }).collect(); + Attributes { doc_strings, other_attrs, cfg: if cfg == Cfg::True { None } else { Some(Rc::new(cfg)) }, span: sp, + links: vec![], } } @@ -795,7 +801,38 @@ impl AttributesExt for Attributes { impl Clean for [ast::Attribute] { fn clean(&self, cx: &DocContext) -> Attributes { - Attributes::from_ast(cx.sess().diagnostic(), self) + let mut attrs = Attributes::from_ast(cx.sess().diagnostic(), self); + + if UnstableFeatures::from_environment().is_nightly_build() { + let dox = attrs.collapsed_doc_value().unwrap_or_else(String::new); + for link in markdown_links(&dox, cx.render_type) { + if !link.starts_with("::") { + // FIXME (misdreavus): can only support absolute paths because of limitations + // in Resolver. this may, with a lot of effort, figure out how to resolve paths + // within scopes, but the one use of `resolve_hir_path` i found in the HIR + // lowering code itself used an absolute path. we're brushing up against some + // structural limitations in the compiler already, but this may be a design one + // as well >_> + continue; + } + + let mut path = hir::Path { + span: DUMMY_SP, + def: Def::Err, + segments: iter::once(keywords::CrateRoot.name()).chain({ + link.split("::").skip(1).map(Symbol::intern) + }).map(hir::PathSegment::from_name).collect(), + }; + + cx.resolver.borrow_mut().resolve_hir_path(&mut path, false); + + if path.def != Def::Err { + attrs.links.push((link, path.def.def_id())); + } + } + } + + attrs } } diff --git a/src/librustdoc/core.rs b/src/librustdoc/core.rs index b094567b15ea5..008285e965336 100644 --- a/src/librustdoc/core.rs +++ b/src/librustdoc/core.rs @@ -46,7 +46,7 @@ pub type ExternalPaths = FxHashMap, clean::TypeKind)>; pub struct DocContext<'a, 'tcx: 'a, 'rcx> { pub tcx: TyCtxt<'a, 'tcx, 'tcx>, - pub resolver: resolve::Resolver<'rcx>, + pub resolver: RefCell>, pub populated_all_crate_impls: Cell, // Note that external items for which `doc(hidden)` applies to are shown as // non-reachable while local items aren't. This is because we're reusing @@ -227,7 +227,7 @@ pub fn run_core(search_paths: SearchPaths, let ctxt = DocContext { tcx, - resolver, + resolver: RefCell::new(resolver), populated_all_crate_impls: Cell::new(false), access_levels: RefCell::new(access_levels), external_traits: Default::default(), diff --git a/src/librustdoc/html/markdown.rs b/src/librustdoc/html/markdown.rs index 498c8911dc036..9bb35da246c89 100644 --- a/src/librustdoc/html/markdown.rs +++ b/src/librustdoc/html/markdown.rs @@ -1169,6 +1169,8 @@ pub fn markdown_links(md: &str, render_type: RenderType) -> Vec { let s = unsafe { (*link).as_bytes() }; let s = str::from_utf8(&s).unwrap().to_owned(); + debug!("found link: {}", s); + links.push(s); } @@ -1214,6 +1216,7 @@ pub fn markdown_links(md: &str, render_type: RenderType) -> Vec { for ev in iter { if let Event::Start(Tag::Link(dest, _)) = ev { + debug!("found link: {}", dest); links.push(dest.into_owned()); } } From 31ca2322a06ee5dbf91e2ea21f4354593fd8ed52 Mon Sep 17 00:00:00 2001 From: QuietMisdreavus Date: Sun, 24 Dec 2017 22:28:34 -0600 Subject: [PATCH 06/44] abort documenting on resolution errors --- src/librustdoc/clean/mod.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/librustdoc/clean/mod.rs b/src/librustdoc/clean/mod.rs index 3f7750cacc2cf..caa0dccba827b 100644 --- a/src/librustdoc/clean/mod.rs +++ b/src/librustdoc/clean/mod.rs @@ -830,6 +830,8 @@ impl Clean for [ast::Attribute] { attrs.links.push((link, path.def.def_id())); } } + + cx.sess().abort_if_errors(); } attrs From f7a8a97b6997fc6a8a5a22a9d1c45d93869a0cb5 Mon Sep 17 00:00:00 2001 From: Manish Goregaokar Date: Mon, 25 Dec 2017 11:37:55 +0530 Subject: [PATCH 07/44] DRY std_path --- src/librustc/hir/lowering.rs | 27 +++++++++++++++++---------- src/librustdoc/clean/mod.rs | 19 +++++++++---------- 2 files changed, 26 insertions(+), 20 deletions(-) diff --git a/src/librustc/hir/lowering.rs b/src/librustc/hir/lowering.rs index 32b55a05124ac..b7a7fcd9872e0 100644 --- a/src/librustc/hir/lowering.rs +++ b/src/librustc/hir/lowering.rs @@ -151,6 +151,22 @@ 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"], returns path `::cratename::b::c::d` when + /// The path is also resolved according to `is_value`. + fn std_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 + } } #[derive(Clone, Copy, Debug)] @@ -3625,16 +3641,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.std_path(span, self.crate_root, components, is_value) } fn signal_block_expr(&mut self, diff --git a/src/librustdoc/clean/mod.rs b/src/librustdoc/clean/mod.rs index caa0dccba827b..816b8caf23eec 100644 --- a/src/librustdoc/clean/mod.rs +++ b/src/librustdoc/clean/mod.rs @@ -25,7 +25,7 @@ use syntax::attr; use syntax::codemap::Spanned; use syntax::feature_gate::UnstableFeatures; use syntax::ptr::P; -use syntax::symbol::{keywords, Symbol}; +use syntax::symbol::keywords; use syntax_pos::{self, DUMMY_SP, Pos, FileName}; use rustc::middle::const_val::ConstVal; @@ -45,7 +45,7 @@ use rustc::hir; use rustc_const_math::ConstInt; use std::default::Default; -use std::{mem, slice, vec, iter}; +use std::{mem, slice, vec}; use std::iter::FromIterator; use std::rc::Rc; use std::sync::Arc; @@ -816,16 +816,15 @@ impl Clean for [ast::Attribute] { continue; } - let mut path = hir::Path { - span: DUMMY_SP, - def: Def::Err, - segments: iter::once(keywords::CrateRoot.name()).chain({ - link.split("::").skip(1).map(Symbol::intern) - }).map(hir::PathSegment::from_name).collect(), + let path = { + // This allocation could be avoided if std_path could take an iterator; + // but it can't because that would break object safety. This can still be + // fixed. + let components = link.split("::").skip(1).collect::>(); + println!("{:?}", components); + cx.resolver.borrow_mut().std_path(DUMMY_SP, None, &components, false) }; - cx.resolver.borrow_mut().resolve_hir_path(&mut path, false); - if path.def != Def::Err { attrs.links.push((link, path.def.def_id())); } From d18b344afba6ab301bef297c161265e1aa6be79e Mon Sep 17 00:00:00 2001 From: Manish Goregaokar Date: Mon, 25 Dec 2017 13:49:25 +0530 Subject: [PATCH 08/44] Move resolve arenas/crate loader outside of the core of phase_2_configure_and_expand --- src/librustc_driver/driver.rs | 27 ++++++++++++++++----------- src/librustdoc/core.rs | 1 + 2 files changed, 17 insertions(+), 11 deletions(-) diff --git a/src/librustc_driver/driver.rs b/src/librustc_driver/driver.rs index 468a08b1fd9f3..90ab8a1951f12 100644 --- a/src/librustc_driver/driver.rs +++ b/src/librustc_driver/driver.rs @@ -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; @@ -139,6 +139,14 @@ pub fn compile_input(trans: Box, let crate_name = ::rustc_trans_utils::link::find_crate_name(Some(sess), &krate.attrs, input); + + // 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 ExpansionResult { expanded_crate, defs, analysis, resolutions, mut hir_forest } = { phase_2_configure_and_expand( sess, @@ -148,6 +156,8 @@ pub fn compile_input(trans: Box, &crate_name, addl_plugins, control.make_glob_map, + &resolver_arenas, + &mut crate_loader, |expanded_crate| { let mut state = CompileState::state_after_expand( input, sess, outdir, output, &cstore, expanded_crate, &crate_name, @@ -569,13 +579,15 @@ pub struct ExpansionResult { /// standard library and prelude, and name resolution. /// /// Returns `None` if we're aborting after handling -W help. -pub fn phase_2_configure_and_expand(sess: &Session, - cstore: &CStore, +pub fn phase_2_configure_and_expand<'a, F>(sess: &'a Session, + cstore: &'a CStore, krate: ast::Crate, registry: Option, crate_name: &str, addl_plugins: Option>, make_glob_map: MakeGlobMap, + resolver_arenas: &'a ResolverArenas<'a>, + crate_loader: &'a mut CrateLoader, after_expand: F) -> Result where F: FnOnce(&ast::Crate) -> CompileResult, @@ -666,19 +678,12 @@ pub fn phase_2_configure_and_expand(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); diff --git a/src/librustdoc/core.rs b/src/librustdoc/core.rs index 008285e965336..b849c433ec396 100644 --- a/src/librustdoc/core.rs +++ b/src/librustdoc/core.rs @@ -223,6 +223,7 @@ pub fn run_core(search_paths: SearchPaths, resolve::MakeGlobMap::No, &mut crate_loader, &resolver_arenas); + resolver.resolve_imports(); resolver.resolve_crate(&expanded_crate); let ctxt = DocContext { From fe0c10019d7ee96909cc42cc265ef999a6b5dd70 Mon Sep 17 00:00:00 2001 From: Manish Goregaokar Date: Mon, 25 Dec 2017 13:55:21 +0530 Subject: [PATCH 09/44] Split out creation of the resolver arena in phase_2_configure_and_expand --- src/librustc_driver/driver.rs | 79 ++++++++++++++++++++++++----------- 1 file changed, 55 insertions(+), 24 deletions(-) diff --git a/src/librustc_driver/driver.rs b/src/librustc_driver/driver.rs index 90ab8a1951f12..31e7b3617990f 100644 --- a/src/librustc_driver/driver.rs +++ b/src/librustc_driver/driver.rs @@ -140,13 +140,6 @@ pub fn compile_input(trans: Box, let crate_name = ::rustc_trans_utils::link::find_crate_name(Some(sess), &krate.attrs, input); - // 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 ExpansionResult { expanded_crate, defs, analysis, resolutions, mut hir_forest } = { phase_2_configure_and_expand( sess, @@ -156,8 +149,6 @@ pub fn compile_input(trans: Box, &crate_name, addl_plugins, control.make_glob_map, - &resolver_arenas, - &mut crate_loader, |expanded_crate| { let mut state = CompileState::state_after_expand( input, sess, outdir, output, &cstore, expanded_crate, &crate_name, @@ -572,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 @@ -580,6 +577,52 @@ pub struct ExpansionResult { /// /// Returns `None` if we're aborting after handling -W help. pub fn phase_2_configure_and_expand<'a, F>(sess: &'a Session, + cstore: &'a CStore, + krate: ast::Crate, + registry: Option, + crate_name: &str, + addl_plugins: Option>, + make_glob_map: MakeGlobMap, + after_expand: F) + -> Result + 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, @@ -589,7 +632,7 @@ pub fn phase_2_configure_and_expand<'a, F>(sess: &'a Session, resolver_arenas: &'a ResolverArenas<'a>, crate_loader: &'a mut CrateLoader, after_expand: F) - -> Result + -> Result, CompileIncomplete> where F: FnOnce(&ast::Crate) -> CompileResult, { let time_passes = sess.time_passes(); @@ -860,21 +903,9 @@ pub fn phase_2_configure_and_expand<'a, F>(sess: &'a 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, }) } From dae2e22e814931ea26e571c7d6e72aa1b05a871d Mon Sep 17 00:00:00 2001 From: Manish Goregaokar Date: Mon, 25 Dec 2017 14:34:56 +0530 Subject: [PATCH 10/44] Make correct resolver available in rustdoc --- src/librustdoc/clean/mod.rs | 3 +- src/librustdoc/core.rs | 71 ++++++++++++++++++++----------------- src/librustdoc/lib.rs | 2 +- src/librustdoc/visit_ast.rs | 12 +++---- src/librustdoc/visit_lib.rs | 2 +- 5 files changed, 48 insertions(+), 42 deletions(-) diff --git a/src/librustdoc/clean/mod.rs b/src/librustdoc/clean/mod.rs index 816b8caf23eec..b890a980d4315 100644 --- a/src/librustdoc/clean/mod.rs +++ b/src/librustdoc/clean/mod.rs @@ -127,7 +127,7 @@ pub struct Crate { pub masked_crates: FxHashSet, } -impl<'a, 'tcx, 'rcx> Clean for visit_ast::RustdocVisitor<'a, 'tcx, 'rcx> { +impl<'a, 'b, 'tcx, 'rcx> Clean for visit_ast::RustdocVisitor<'a, 'b, 'tcx, 'rcx> { fn clean(&self, cx: &DocContext) -> Crate { use ::visit_lib::LibEmbargoVisitor; @@ -821,7 +821,6 @@ impl Clean for [ast::Attribute] { // but it can't because that would break object safety. This can still be // fixed. let components = link.split("::").skip(1).collect::>(); - println!("{:?}", components); cx.resolver.borrow_mut().std_path(DUMMY_SP, None, &components, false) }; diff --git a/src/librustdoc/core.rs b/src/librustdoc/core.rs index b849c433ec396..6fc21f0541b1b 100644 --- a/src/librustdoc/core.rs +++ b/src/librustdoc/core.rs @@ -44,9 +44,9 @@ pub use rustc::session::search_paths::SearchPaths; pub type ExternalPaths = FxHashMap, clean::TypeKind)>; -pub struct DocContext<'a, 'tcx: 'a, 'rcx> { +pub struct DocContext<'a, 'tcx: 'a, 'rcx: 'a> { pub tcx: TyCtxt<'a, 'tcx, 'tcx>, - pub resolver: RefCell>, + pub resolver: &'a RefCell>, pub populated_all_crate_impls: Cell, // Note that external items for which `doc(hidden)` applies to are shown as // non-reachable while local items aren't. This is because we're reusing @@ -162,27 +162,43 @@ pub fn run_core(search_paths: SearchPaths, let name = ::rustc_trans_utils::link::find_crate_name(Some(&sess), &krate.attrs, &input); - let driver::ExpansionResult { - expanded_crate, - defs, - analysis, - resolutions, - mut hir_forest - } = { - let result = driver::phase_2_configure_and_expand(&sess, - &cstore, - krate, - None, - &name, - None, - resolve::MakeGlobMap::No, - |_| Ok(())); - abort_on_err(result, &sess) + let mut crate_loader = CrateLoader::new(&sess, &cstore, &name); + + let resolver_arenas = resolve::Resolver::arenas(); + let result = driver::phase_2_configure_and_expand_inner(&sess, + &cstore, + krate, + None, + &name, + None, + resolve::MakeGlobMap::No, + &resolver_arenas, + &mut crate_loader, + |_| Ok(())); + let driver::InnerExpansionResult { + mut hir_forest, + resolver, + .. + } = abort_on_err(result, &sess); + + // We need to hold on to the complete resolver, so we clone everything + // for the analysis passes to use. Suboptimal, but necessary in the + // current architecture. + let defs = resolver.definitions.clone(); + let resolutions = ty::Resolutions { + freevars: resolver.freevars.clone(), + export_map: resolver.export_map.clone(), + trait_map: resolver.trait_map.clone(), + maybe_unused_trait_imports: resolver.maybe_unused_trait_imports.clone(), + maybe_unused_extern_crates: resolver.maybe_unused_extern_crates.clone(), + }; + let analysis = ty::CrateAnalysis { + access_levels: Rc::new(AccessLevels::default()), + name: name.to_string(), + glob_map: if resolver.make_glob_map { Some(resolver.glob_map.clone()) } else { None }, }; let arenas = AllArenas::new(); - let mut crate_loader = CrateLoader::new(&sess, &cstore, &name); - let resolver_arenas = resolve::Resolver::arenas(); let hir_map = hir_map::map_crate(&sess, &*cstore, &mut hir_forest, &defs); let output_filenames = driver::build_output_filenames(&input, &None, @@ -190,6 +206,8 @@ pub fn run_core(search_paths: SearchPaths, &[], &sess); + let resolver = RefCell::new(resolver); + abort_on_err(driver::phase_3_run_analysis_passes(&*trans, control, &sess, @@ -215,20 +233,9 @@ pub fn run_core(search_paths: SearchPaths, .collect() }; - // Set up a Resolver so that the doc cleaning can look up paths in the docs - let mut resolver = resolve::Resolver::new(&sess, - &*cstore, - &expanded_crate, - &name, - resolve::MakeGlobMap::No, - &mut crate_loader, - &resolver_arenas); - resolver.resolve_imports(); - resolver.resolve_crate(&expanded_crate); - let ctxt = DocContext { tcx, - resolver: RefCell::new(resolver), + resolver: &resolver, populated_all_crate_impls: Cell::new(false), access_levels: RefCell::new(access_levels), external_traits: Default::default(), diff --git a/src/librustdoc/lib.rs b/src/librustdoc/lib.rs index 01c2a5620da25..5206c3667b006 100644 --- a/src/librustdoc/lib.rs +++ b/src/librustdoc/lib.rs @@ -12,7 +12,7 @@ html_favicon_url = "https://doc.rust-lang.org/favicon.ico", html_root_url = "https://doc.rust-lang.org/nightly/", html_playground_url = "https://play.rust-lang.org/")] -#![deny(warnings)] + #![feature(ascii_ctype)] #![feature(rustc_private)] diff --git a/src/librustdoc/visit_ast.rs b/src/librustdoc/visit_ast.rs index 23a2208292a36..d2f7da29b33ab 100644 --- a/src/librustdoc/visit_ast.rs +++ b/src/librustdoc/visit_ast.rs @@ -40,11 +40,11 @@ use doctree::*; // also, is there some reason that this doesn't use the 'visit' // framework from syntax? -pub struct RustdocVisitor<'a, 'tcx: 'a, 'rcx: 'a> { - cstore: &'tcx CrateStore, +pub struct RustdocVisitor<'a, 'b: 'a, 'tcx: 'b, 'rcx: 'b> { + cstore: &'a CrateStore, pub module: Module, pub attrs: hir::HirVec, - pub cx: &'a core::DocContext<'a, 'tcx, 'rcx>, + pub cx: &'a core::DocContext<'b, 'tcx, 'rcx>, view_item_stack: FxHashSet, inlining: bool, /// Is the current module and all of its parents public? @@ -52,9 +52,9 @@ pub struct RustdocVisitor<'a, 'tcx: 'a, 'rcx: 'a> { reexported_macros: FxHashSet, } -impl<'a, 'tcx, 'rcx> RustdocVisitor<'a, 'tcx, 'rcx> { - pub fn new(cstore: &'tcx CrateStore, - cx: &'a core::DocContext<'a, 'tcx, 'rcx>) -> RustdocVisitor<'a, 'tcx, 'rcx> { +impl<'a, 'b, 'tcx, 'rcx> RustdocVisitor<'a, 'b, 'tcx, 'rcx> { + pub fn new(cstore: &'a CrateStore, + cx: &'a core::DocContext<'b, 'tcx, 'rcx>) -> RustdocVisitor<'a, 'b, 'tcx, 'rcx> { // If the root is re-exported, terminate all recursion. let mut stack = FxHashSet(); stack.insert(ast::CRATE_NODE_ID); diff --git a/src/librustdoc/visit_lib.rs b/src/librustdoc/visit_lib.rs index 7da0a7bfe6ea8..55f3fdefd1b41 100644 --- a/src/librustdoc/visit_lib.rs +++ b/src/librustdoc/visit_lib.rs @@ -22,7 +22,7 @@ use clean::{AttributesExt, NestedAttributesExt}; /// Similar to `librustc_privacy::EmbargoVisitor`, but also takes /// specific rustdoc annotations into account (i.e. `doc(hidden)`) -pub struct LibEmbargoVisitor<'a, 'b: 'a, 'tcx: 'b, 'rcx: 'a> { +pub struct LibEmbargoVisitor<'a, 'b: 'a, 'tcx: 'b, 'rcx: 'b> { cx: &'a ::core::DocContext<'b, 'tcx, 'rcx>, // Accessibility levels for reachable nodes access_levels: RefMut<'a, AccessLevels>, From e8dd5df69b67b30ec92e0198f1ade6476916d463 Mon Sep 17 00:00:00 2001 From: Manish Goregaokar Date: Thu, 28 Dec 2017 16:29:20 +0530 Subject: [PATCH 11/44] Add LinkReplacer pass for pulldown --- src/librustdoc/clean/mod.rs | 19 +++++++++++++ src/librustdoc/externalfiles.rs | 4 +-- src/librustdoc/html/markdown.rs | 47 ++++++++++++++++++++++++++++----- src/librustdoc/html/render.rs | 13 ++++----- src/librustdoc/markdown.rs | 2 +- 5 files changed, 69 insertions(+), 16 deletions(-) diff --git a/src/librustdoc/clean/mod.rs b/src/librustdoc/clean/mod.rs index b890a980d4315..c91be9ad7b8f4 100644 --- a/src/librustdoc/clean/mod.rs +++ b/src/librustdoc/clean/mod.rs @@ -308,6 +308,11 @@ impl Item { pub fn collapsed_doc_value(&self) -> Option { self.attrs.collapsed_doc_value() } + + pub fn links(&self) -> Vec<(String, String)> { + self.attrs.links() + } + pub fn is_crate(&self) -> bool { match self.inner { StrippedItem(box ModuleItem(Module { is_crate: true, ..})) | @@ -791,6 +796,20 @@ impl Attributes { None } } + + /// Get links as a vector + /// + /// Cache must be populated before call + pub fn links(&self) -> Vec<(String, String)> { + use html::format::href; + self.links.iter().filter_map(|&(ref s, did)| { + if let Some((href, ..)) = href(did) { + Some((s.clone(), href)) + } else { + None + } + }).collect() + } } impl AttributesExt for Attributes { diff --git a/src/librustdoc/externalfiles.rs b/src/librustdoc/externalfiles.rs index f8320330ad265..f7d07af04ea98 100644 --- a/src/librustdoc/externalfiles.rs +++ b/src/librustdoc/externalfiles.rs @@ -37,7 +37,7 @@ impl ExternalHtml { ) .and_then(|(ih, bc)| load_external_files(md_before_content) - .map(|m_bc| (ih, format!("{}{}", bc, Markdown(&m_bc, render)))) + .map(|m_bc| (ih, format!("{}{}", bc, Markdown(&m_bc, &[], render)))) ) .and_then(|(ih, bc)| load_external_files(after_content) @@ -45,7 +45,7 @@ impl ExternalHtml { ) .and_then(|(ih, bc, ac)| load_external_files(md_after_content) - .map(|m_ac| (ih, bc, format!("{}{}", ac, Markdown(&m_ac, render)))) + .map(|m_ac| (ih, bc, format!("{}{}", ac, Markdown(&m_ac, &[], render)))) ) .map(|(ih, bc, ac)| ExternalHtml { diff --git a/src/librustdoc/html/markdown.rs b/src/librustdoc/html/markdown.rs index 9bb35da246c89..9e875a7c28059 100644 --- a/src/librustdoc/html/markdown.rs +++ b/src/librustdoc/html/markdown.rs @@ -56,15 +56,16 @@ pub enum RenderType { /// A unit struct which has the `fmt::Display` trait implemented. When /// formatted, this struct will emit the HTML corresponding to the rendered /// version of the contained markdown string. -// The second parameter is whether we need a shorter version or not. -pub struct Markdown<'a>(pub &'a str, pub RenderType); +/// The second parameter is a list of link replacements +// The third parameter is whether we need a shorter version or not. +pub struct Markdown<'a>(pub &'a str, pub &'a [(String, String)], pub RenderType); /// A unit struct like `Markdown`, that renders the markdown with a /// table of contents. pub struct MarkdownWithToc<'a>(pub &'a str, pub RenderType); /// A unit struct like `Markdown`, that renders the markdown escaping HTML tags. pub struct MarkdownHtml<'a>(pub &'a str, pub RenderType); /// A unit struct like `Markdown`, that renders only the first paragraph. -pub struct MarkdownSummaryLine<'a>(pub &'a str); +pub struct MarkdownSummaryLine<'a>(pub &'a str, pub &'a [(String, String)]); /// Controls whether a line will be hidden or shown in HTML output. /// @@ -247,6 +248,38 @@ impl<'a, I: Iterator>> Iterator for CodeBlocks<'a, I> { } } +/// Make headings links with anchor ids and build up TOC. +struct LinkReplacer<'a, 'b, I: Iterator>> { + inner: I, + links: &'b [(String, String)] +} + +impl<'a, 'b, I: Iterator>> LinkReplacer<'a, 'b, I> { + fn new(iter: I, links: &'b [(String, String)]) -> Self { + LinkReplacer { + inner: iter, + links + } + } +} + +impl<'a, 'b, I: Iterator>> Iterator for LinkReplacer<'a, 'b, I> { + type Item = Event<'a>; + + fn next(&mut self) -> Option { + let event = self.inner.next(); + if let Some(Event::Start(Tag::Link(dest, text))) = event { + if let Some(&(_, ref replace)) = self.links.into_iter().find(|link| &*link.0 == &*dest) { + Some(Event::Start(Tag::Link(replace.to_owned().into(), text))) + } else { + Some(Event::Start(Tag::Link(dest, text))) + } + } else { + event + } + } +} + /// Make headings links with anchor ids and build up TOC. struct HeadingLinks<'a, 'b, I: Iterator>> { inner: I, @@ -996,7 +1029,7 @@ impl LangString { impl<'a> fmt::Display for Markdown<'a> { fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result { - let Markdown(md, render_type) = *self; + let Markdown(md, links, render_type) = *self; // This is actually common enough to special-case if md.is_empty() { return Ok(()) } @@ -1012,7 +1045,7 @@ impl<'a> fmt::Display for Markdown<'a> { let mut s = String::with_capacity(md.len() * 3 / 2); html::push_html(&mut s, - Footnotes::new(CodeBlocks::new(HeadingLinks::new(p, None)))); + Footnotes::new(CodeBlocks::new(LinkReplacer::new(HeadingLinks::new(p, None), links)))); fmt.write_str(&s) } @@ -1079,7 +1112,7 @@ impl<'a> fmt::Display for MarkdownHtml<'a> { impl<'a> fmt::Display for MarkdownSummaryLine<'a> { fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result { - let MarkdownSummaryLine(md) = *self; + let MarkdownSummaryLine(md, links) = *self; // This is actually common enough to special-case if md.is_empty() { return Ok(()) } @@ -1087,7 +1120,7 @@ impl<'a> fmt::Display for MarkdownSummaryLine<'a> { let mut s = String::new(); - html::push_html(&mut s, SummaryLine::new(p)); + html::push_html(&mut s, LinkReplacer::new(SummaryLine::new(p), links)); fmt.write_str(&s) } diff --git a/src/librustdoc/html/render.rs b/src/librustdoc/html/render.rs index f335e073d5691..52200f5afd79a 100644 --- a/src/librustdoc/html/render.rs +++ b/src/librustdoc/html/render.rs @@ -1861,12 +1861,13 @@ fn document(w: &mut fmt::Formatter, cx: &Context, item: &clean::Item) -> fmt::Re /// rendering between Pulldown and Hoedown. fn render_markdown(w: &mut fmt::Formatter, md_text: &str, + links: Vec<(String, String)>, span: Span, render_type: RenderType, prefix: &str, scx: &SharedContext) -> fmt::Result { - let (hoedown_output, pulldown_output) = render_text(|ty| format!("{}", Markdown(md_text, ty))); + let (hoedown_output, pulldown_output) = render_text(|ty| format!("{}", Markdown(md_text, &links, ty))); let mut differences = html_diff::get_differences(&pulldown_output, &hoedown_output); differences.retain(|s| { match *s { @@ -1898,7 +1899,7 @@ fn document_short(w: &mut fmt::Formatter, item: &clean::Item, link: AssocItemLin } else { format!("{}", &plain_summary_line(Some(s))) }; - render_markdown(w, &markdown, item.source.clone(), cx.render_type, prefix, &cx.shared)?; + render_markdown(w, &markdown, item.links(), item.source.clone(), cx.render_type, prefix, &cx.shared)?; } else if !prefix.is_empty() { write!(w, "
{}
", prefix)?; } @@ -1924,7 +1925,7 @@ fn document_full(w: &mut fmt::Formatter, item: &clean::Item, cx: &Context, prefix: &str) -> fmt::Result { if let Some(s) = cx.shared.maybe_collapsed_doc_value(item) { debug!("Doc block: =====\n{}\n=====", s); - render_markdown(w, &*s, item.source.clone(), cx.render_type, prefix, &cx.shared)?; + render_markdown(w, &*s, item.links(), item.source.clone(), cx.render_type, prefix, &cx.shared)?; } else if !prefix.is_empty() { write!(w, "
{}
", prefix)?; } @@ -2146,10 +2147,10 @@ fn item_module(w: &mut fmt::Formatter, cx: &Context, stab_docs = stab_docs, docs = if cx.render_type == RenderType::Hoedown { format!("{}", - shorter(Some(&Markdown(doc_value, + shorter(Some(&Markdown(doc_value, &item.links(), RenderType::Hoedown).to_string()))) } else { - format!("{}", MarkdownSummaryLine(doc_value)) + format!("{}", MarkdownSummaryLine(doc_value, &item.links())) }, class = myitem.type_(), stab = myitem.stability_class().unwrap_or("".to_string()), @@ -3338,7 +3339,7 @@ fn render_impl(w: &mut fmt::Formatter, cx: &Context, i: &Impl, link: AssocItemLi write!(w, "")?; write!(w, "\n")?; if let Some(ref dox) = cx.shared.maybe_collapsed_doc_value(&i.impl_item) { - write!(w, "
{}
", Markdown(&*dox, cx.render_type))?; + write!(w, "
{}
", Markdown(&*dox, &i.impl_item.links(), cx.render_type))?; } } diff --git a/src/librustdoc/markdown.rs b/src/librustdoc/markdown.rs index af93505293cef..9af2ebf0661da 100644 --- a/src/librustdoc/markdown.rs +++ b/src/librustdoc/markdown.rs @@ -104,7 +104,7 @@ pub fn render(input: &Path, mut output: PathBuf, matches: &getopts::Matches, } else { // Save the state of USED_ID_MAP so it only gets updated once even // though we're rendering twice. - render_text(|ty| format!("{}", Markdown(text, ty))) + render_text(|ty| format!("{}", Markdown(text, &[], ty))) }; let mut differences = html_diff::get_differences(&pulldown_output, &hoedown_output); From 611866f3cfb129202bc229659c1e8f4f313b330e Mon Sep 17 00:00:00 2001 From: QuietMisdreavus Date: Thu, 28 Dec 2017 10:52:40 -0600 Subject: [PATCH 12/44] cleanup --- src/librustc/hir/lowering.rs | 8 ++++---- src/librustc_driver/driver.rs | 9 +++++---- src/librustdoc/clean/mod.rs | 6 +++--- src/librustdoc/html/markdown.rs | 10 ++++++++-- src/librustdoc/html/render.rs | 22 ++++++++++++++++++---- src/librustdoc/lib.rs | 2 +- src/librustdoc/visit_ast.rs | 8 ++++---- src/librustdoc/visit_lib.rs | 8 ++++---- 8 files changed, 47 insertions(+), 26 deletions(-) diff --git a/src/librustc/hir/lowering.rs b/src/librustc/hir/lowering.rs index b7a7fcd9872e0..f2154f885fa4a 100644 --- a/src/librustc/hir/lowering.rs +++ b/src/librustc/hir/lowering.rs @@ -152,9 +152,9 @@ pub trait Resolver { /// This should only return `None` during testing. fn definitions(&mut self) -> &mut Definitions; - /// Given suffix ["b","c","d"], returns path `::cratename::b::c::d` when - /// The path is also resolved according to `is_value`. - fn std_path(&mut self, span: Span, crate_root: Option<&str>, + /// 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 { let mut path = hir::Path { span, @@ -3641,7 +3641,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 { - self.resolver.std_path(span, self.crate_root, components, is_value) + self.resolver.resolve_str_path(span, self.crate_root, components, is_value) } fn signal_block_expr(&mut self, diff --git a/src/librustc_driver/driver.rs b/src/librustc_driver/driver.rs index 31e7b3617990f..b2897bd454839 100644 --- a/src/librustc_driver/driver.rs +++ b/src/librustc_driver/driver.rs @@ -576,8 +576,8 @@ pub struct InnerExpansionResult<'a> { /// standard library and prelude, and name resolution. /// /// Returns `None` if we're aborting after handling -W help. -pub fn phase_2_configure_and_expand<'a, F>(sess: &'a Session, - cstore: &'a CStore, +pub fn phase_2_configure_and_expand(sess: &Session, + cstore: &CStore, krate: ast::Crate, registry: Option, crate_name: &str, @@ -593,8 +593,9 @@ pub fn phase_2_configure_and_expand<'a, F>(sess: &'a Session, // 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); + 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 { diff --git a/src/librustdoc/clean/mod.rs b/src/librustdoc/clean/mod.rs index c91be9ad7b8f4..ed07233184740 100644 --- a/src/librustdoc/clean/mod.rs +++ b/src/librustdoc/clean/mod.rs @@ -127,7 +127,7 @@ pub struct Crate { pub masked_crates: FxHashSet, } -impl<'a, 'b, 'tcx, 'rcx> Clean for visit_ast::RustdocVisitor<'a, 'b, 'tcx, 'rcx> { +impl<'a, 'tcx, 'rcx> Clean for visit_ast::RustdocVisitor<'a, 'tcx, 'rcx> { fn clean(&self, cx: &DocContext) -> Crate { use ::visit_lib::LibEmbargoVisitor; @@ -836,11 +836,11 @@ impl Clean for [ast::Attribute] { } let path = { - // This allocation could be avoided if std_path could take an iterator; + // This allocation could be avoided if resolve_str_path could take an iterator; // but it can't because that would break object safety. This can still be // fixed. let components = link.split("::").skip(1).collect::>(); - cx.resolver.borrow_mut().std_path(DUMMY_SP, None, &components, false) + cx.resolver.borrow_mut().resolve_str_path(DUMMY_SP, None, &components, false) }; if path.def != Def::Err { diff --git a/src/librustdoc/html/markdown.rs b/src/librustdoc/html/markdown.rs index 9e875a7c28059..86657aa000b0d 100644 --- a/src/librustdoc/html/markdown.rs +++ b/src/librustdoc/html/markdown.rs @@ -269,7 +269,8 @@ impl<'a, 'b, I: Iterator>> Iterator for LinkReplacer<'a, 'b, I> fn next(&mut self) -> Option { let event = self.inner.next(); if let Some(Event::Start(Tag::Link(dest, text))) = event { - if let Some(&(_, ref replace)) = self.links.into_iter().find(|link| &*link.0 == &*dest) { + if let Some(&(_, ref replace)) = self.links.into_iter().find(|link| &*link.0 == &*dest) + { Some(Event::Start(Tag::Link(replace.to_owned().into(), text))) } else { Some(Event::Start(Tag::Link(dest, text))) @@ -1045,7 +1046,11 @@ impl<'a> fmt::Display for Markdown<'a> { let mut s = String::with_capacity(md.len() * 3 / 2); html::push_html(&mut s, - Footnotes::new(CodeBlocks::new(LinkReplacer::new(HeadingLinks::new(p, None), links)))); + Footnotes::new( + CodeBlocks::new( + LinkReplacer::new( + HeadingLinks::new(p, None), + links)))); fmt.write_str(&s) } @@ -1233,6 +1238,7 @@ pub fn markdown_links(md: &str, render_type: RenderType) -> Vec { hoedown_document_free(document); hoedown_html_renderer_free(renderer); + hoedown_buffer_free(ob); opaque.links.unwrap() } diff --git a/src/librustdoc/html/render.rs b/src/librustdoc/html/render.rs index 52200f5afd79a..5dea05cedbeaf 100644 --- a/src/librustdoc/html/render.rs +++ b/src/librustdoc/html/render.rs @@ -1867,7 +1867,8 @@ fn render_markdown(w: &mut fmt::Formatter, prefix: &str, scx: &SharedContext) -> fmt::Result { - let (hoedown_output, pulldown_output) = render_text(|ty| format!("{}", Markdown(md_text, &links, ty))); + let (hoedown_output, pulldown_output) = + render_text(|ty| format!("{}", Markdown(md_text, &links, ty))); let mut differences = html_diff::get_differences(&pulldown_output, &hoedown_output); differences.retain(|s| { match *s { @@ -1899,7 +1900,13 @@ fn document_short(w: &mut fmt::Formatter, item: &clean::Item, link: AssocItemLin } else { format!("{}", &plain_summary_line(Some(s))) }; - render_markdown(w, &markdown, item.links(), item.source.clone(), cx.render_type, prefix, &cx.shared)?; + render_markdown(w, + &markdown, + item.links(), + item.source.clone(), + cx.render_type, + prefix, + &cx.shared)?; } else if !prefix.is_empty() { write!(w, "
{}
", prefix)?; } @@ -1925,7 +1932,13 @@ fn document_full(w: &mut fmt::Formatter, item: &clean::Item, cx: &Context, prefix: &str) -> fmt::Result { if let Some(s) = cx.shared.maybe_collapsed_doc_value(item) { debug!("Doc block: =====\n{}\n=====", s); - render_markdown(w, &*s, item.links(), item.source.clone(), cx.render_type, prefix, &cx.shared)?; + render_markdown(w, + &*s, + item.links(), + item.source.clone(), + cx.render_type, + prefix, + &cx.shared)?; } else if !prefix.is_empty() { write!(w, "
{}
", prefix)?; } @@ -3339,7 +3352,8 @@ fn render_impl(w: &mut fmt::Formatter, cx: &Context, i: &Impl, link: AssocItemLi write!(w, "")?; write!(w, "\n")?; if let Some(ref dox) = cx.shared.maybe_collapsed_doc_value(&i.impl_item) { - write!(w, "
{}
", Markdown(&*dox, &i.impl_item.links(), cx.render_type))?; + write!(w, "
{}
", + Markdown(&*dox, &i.impl_item.links(), cx.render_type))?; } } diff --git a/src/librustdoc/lib.rs b/src/librustdoc/lib.rs index 5206c3667b006..01c2a5620da25 100644 --- a/src/librustdoc/lib.rs +++ b/src/librustdoc/lib.rs @@ -12,7 +12,7 @@ html_favicon_url = "https://doc.rust-lang.org/favicon.ico", html_root_url = "https://doc.rust-lang.org/nightly/", html_playground_url = "https://play.rust-lang.org/")] - +#![deny(warnings)] #![feature(ascii_ctype)] #![feature(rustc_private)] diff --git a/src/librustdoc/visit_ast.rs b/src/librustdoc/visit_ast.rs index d2f7da29b33ab..7b208465369fc 100644 --- a/src/librustdoc/visit_ast.rs +++ b/src/librustdoc/visit_ast.rs @@ -40,11 +40,11 @@ use doctree::*; // also, is there some reason that this doesn't use the 'visit' // framework from syntax? -pub struct RustdocVisitor<'a, 'b: 'a, 'tcx: 'b, 'rcx: 'b> { +pub struct RustdocVisitor<'a, 'tcx: 'a, 'rcx: 'a> { cstore: &'a CrateStore, pub module: Module, pub attrs: hir::HirVec, - pub cx: &'a core::DocContext<'b, 'tcx, 'rcx>, + pub cx: &'a core::DocContext<'a, 'tcx, 'rcx>, view_item_stack: FxHashSet, inlining: bool, /// Is the current module and all of its parents public? @@ -52,9 +52,9 @@ pub struct RustdocVisitor<'a, 'b: 'a, 'tcx: 'b, 'rcx: 'b> { reexported_macros: FxHashSet, } -impl<'a, 'b, 'tcx, 'rcx> RustdocVisitor<'a, 'b, 'tcx, 'rcx> { +impl<'a, 'tcx, 'rcx> RustdocVisitor<'a, 'tcx, 'rcx> { pub fn new(cstore: &'a CrateStore, - cx: &'a core::DocContext<'b, 'tcx, 'rcx>) -> RustdocVisitor<'a, 'b, 'tcx, 'rcx> { + cx: &'a core::DocContext<'a, 'tcx, 'rcx>) -> RustdocVisitor<'a, 'tcx, 'rcx> { // If the root is re-exported, terminate all recursion. let mut stack = FxHashSet(); stack.insert(ast::CRATE_NODE_ID); diff --git a/src/librustdoc/visit_lib.rs b/src/librustdoc/visit_lib.rs index 55f3fdefd1b41..15a8b58d0f6b9 100644 --- a/src/librustdoc/visit_lib.rs +++ b/src/librustdoc/visit_lib.rs @@ -22,8 +22,8 @@ use clean::{AttributesExt, NestedAttributesExt}; /// Similar to `librustc_privacy::EmbargoVisitor`, but also takes /// specific rustdoc annotations into account (i.e. `doc(hidden)`) -pub struct LibEmbargoVisitor<'a, 'b: 'a, 'tcx: 'b, 'rcx: 'b> { - cx: &'a ::core::DocContext<'b, 'tcx, 'rcx>, +pub struct LibEmbargoVisitor<'a, 'tcx: 'a, 'rcx: 'a> { + cx: &'a ::core::DocContext<'a, 'tcx, 'rcx>, // Accessibility levels for reachable nodes access_levels: RefMut<'a, AccessLevels>, // Previous accessibility level, None means unreachable @@ -32,8 +32,8 @@ pub struct LibEmbargoVisitor<'a, 'b: 'a, 'tcx: 'b, 'rcx: 'b> { visited_mods: FxHashSet, } -impl<'a, 'b, 'tcx, 'rcx> LibEmbargoVisitor<'a, 'b, 'tcx, 'rcx> { - pub fn new(cx: &'a ::core::DocContext<'b, 'tcx, 'rcx>) -> LibEmbargoVisitor<'a, 'b, 'tcx, 'rcx> { +impl<'a, 'tcx, 'rcx> LibEmbargoVisitor<'a, 'tcx, 'rcx> { + pub fn new(cx: &'a ::core::DocContext<'a, 'tcx, 'rcx>) -> LibEmbargoVisitor<'a, 'tcx, 'rcx> { LibEmbargoVisitor { cx, access_levels: cx.access_levels.borrow_mut(), From 9d5b1ae763c0d34e31c8fe866be57f570bd0869f Mon Sep 17 00:00:00 2001 From: QuietMisdreavus Date: Thu, 28 Dec 2017 11:41:00 -0600 Subject: [PATCH 13/44] add intra-links support to hoedown --- src/librustdoc/html/markdown.rs | 74 +++++++++++++++++++++++++++++---- 1 file changed, 66 insertions(+), 8 deletions(-) diff --git a/src/librustdoc/html/markdown.rs b/src/librustdoc/html/markdown.rs index 86657aa000b0d..6fd8eb4de5fc1 100644 --- a/src/librustdoc/html/markdown.rs +++ b/src/librustdoc/html/markdown.rs @@ -561,7 +561,8 @@ struct MyOpaque { *const hoedown_buffer, *const hoedown_renderer_data, libc::size_t), toc_builder: Option, - links: Option>, + links_out: Option>, + links_replace: Vec<(String, String)>, } extern { @@ -771,8 +772,62 @@ extern fn hoedown_codespan( pub fn render(w: &mut fmt::Formatter, s: &str, + links: &[(String, String)], print_toc: bool, html_flags: libc::c_uint) -> fmt::Result { + extern fn hoedown_link( + ob: *mut hoedown_buffer, + content: *const hoedown_buffer, + link: *const hoedown_buffer, + title: *const hoedown_buffer, + data: *const hoedown_renderer_data, + _line: libc::size_t + ) -> libc::c_int { + if link.is_null() { + return 0; + } + + let opaque = unsafe { (*data).opaque as *mut hoedown_html_renderer_state }; + let opaque = unsafe { &mut *((*opaque).opaque as *mut MyOpaque) }; + + let link = { + let s = unsafe { (*link).as_bytes() }; + str::from_utf8(s).unwrap().to_owned() + }; + + let link = if let Some(&(_, ref new_target)) = opaque.links_replace + .iter() + .find(|t| &*t.0 == &*link) { + new_target.to_owned() + } else { + return 0; + }; + + let content = unsafe { + content.as_ref().map(|c| { + let s = c.as_bytes(); + str::from_utf8(s).unwrap().to_owned() + }) + }; + + let title = unsafe { + title.as_ref().map(|t| { + let s = t.as_bytes(); + str::from_utf8(s).unwrap().to_owned() + }) + }; + + let link_out = format!("{content}", + link = link, + title = title.map_or(String::new(), + |t| format!(" title=\"{}\"", t)), + content = content.unwrap_or(String::new())); + + unsafe { hoedown_buffer_put(ob, link_out.as_ptr(), link_out.len()); } + + //return "anything but 0" to show we've written the link in + 1 + } unsafe { let ob = hoedown_buffer_new(DEF_OUNIT); @@ -780,13 +835,15 @@ pub fn render(w: &mut fmt::Formatter, let mut opaque = MyOpaque { dfltblk: (*renderer).blockcode.unwrap(), toc_builder: if print_toc {Some(TocBuilder::new())} else {None}, - links: None, + links_out: None, + links_replace: links.to_vec(), }; (*((*renderer).opaque as *mut hoedown_html_renderer_state)).opaque = &mut opaque as *mut _ as *mut libc::c_void; (*renderer).blockcode = Some(hoedown_block); (*renderer).header = Some(hoedown_header); (*renderer).codespan = Some(hoedown_codespan); + (*renderer).link = Some(hoedown_link); let document = hoedown_document_new(renderer, HOEDOWN_EXTENSIONS, 16); hoedown_document_render(document, ob, s.as_ptr(), @@ -1035,7 +1092,7 @@ impl<'a> fmt::Display for Markdown<'a> { // This is actually common enough to special-case if md.is_empty() { return Ok(()) } if render_type == RenderType::Hoedown { - render(fmt, md, false, 0) + render(fmt, md, links, false, 0) } else { let mut opts = Options::empty(); opts.insert(OPTION_ENABLE_TABLES); @@ -1062,7 +1119,7 @@ impl<'a> fmt::Display for MarkdownWithToc<'a> { let MarkdownWithToc(md, render_type) = *self; if render_type == RenderType::Hoedown { - render(fmt, md, true, 0) + render(fmt, md, &[], true, 0) } else { let mut opts = Options::empty(); opts.insert(OPTION_ENABLE_TABLES); @@ -1091,7 +1148,7 @@ impl<'a> fmt::Display for MarkdownHtml<'a> { // This is actually common enough to special-case if md.is_empty() { return Ok(()) } if render_type == RenderType::Hoedown { - render(fmt, md, false, HOEDOWN_HTML_ESCAPE) + render(fmt, md, &[], false, HOEDOWN_HTML_ESCAPE) } else { let mut opts = Options::empty(); opts.insert(OPTION_ENABLE_TABLES); @@ -1203,7 +1260,7 @@ pub fn markdown_links(md: &str, render_type: RenderType) -> Vec { let opaque = unsafe { (*data).opaque as *mut hoedown_html_renderer_state }; let opaque = unsafe { &mut *((*opaque).opaque as *mut MyOpaque) }; - if let Some(ref mut links) = opaque.links { + if let Some(ref mut links) = opaque.links_out { let s = unsafe { (*link).as_bytes() }; let s = str::from_utf8(&s).unwrap().to_owned(); @@ -1223,7 +1280,8 @@ pub fn markdown_links(md: &str, render_type: RenderType) -> Vec { let mut opaque = MyOpaque { dfltblk: (*renderer).blockcode.unwrap(), toc_builder: None, - links: Some(vec![]), + links_out: Some(vec![]), + links_replace: vec![], }; (*((*renderer).opaque as *mut hoedown_html_renderer_state)).opaque = &mut opaque as *mut _ as *mut libc::c_void; @@ -1240,7 +1298,7 @@ pub fn markdown_links(md: &str, render_type: RenderType) -> Vec { hoedown_html_renderer_free(renderer); hoedown_buffer_free(ob); - opaque.links.unwrap() + opaque.links_out.unwrap() } } RenderType::Pulldown => { From ef4587b270226a34fb178ca43c60484ca98371a0 Mon Sep 17 00:00:00 2001 From: QuietMisdreavus Date: Thu, 28 Dec 2017 13:35:10 -0600 Subject: [PATCH 14/44] fix error_index_generator --- src/tools/error_index_generator/main.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tools/error_index_generator/main.rs b/src/tools/error_index_generator/main.rs index aedae366c4119..8454e71fa3f9b 100644 --- a/src/tools/error_index_generator/main.rs +++ b/src/tools/error_index_generator/main.rs @@ -100,7 +100,7 @@ impl Formatter for HTMLFormatter { // Description rendered as markdown. match info.description { - Some(ref desc) => write!(output, "{}", Markdown(desc, RenderType::Hoedown))?, + Some(ref desc) => write!(output, "{}", Markdown(desc, &[], RenderType::Hoedown))?, None => write!(output, "

No description.

\n")?, } From c4a4d3a031301248f8769960bc1c0eb43cb779ec Mon Sep 17 00:00:00 2001 From: QuietMisdreavus Date: Thu, 28 Dec 2017 16:46:13 -0600 Subject: [PATCH 15/44] parse path ambiguity markers --- src/librustdoc/clean/mod.rs | 46 +++++++++++++++++++++++++++---------- 1 file changed, 34 insertions(+), 12 deletions(-) diff --git a/src/librustdoc/clean/mod.rs b/src/librustdoc/clean/mod.rs index ed07233184740..2c2cad1dc401d 100644 --- a/src/librustdoc/clean/mod.rs +++ b/src/librustdoc/clean/mod.rs @@ -825,22 +825,44 @@ impl Clean for [ast::Attribute] { if UnstableFeatures::from_environment().is_nightly_build() { let dox = attrs.collapsed_doc_value().unwrap_or_else(String::new); for link in markdown_links(&dox, cx.render_type) { - if !link.starts_with("::") { - // FIXME (misdreavus): can only support absolute paths because of limitations - // in Resolver. this may, with a lot of effort, figure out how to resolve paths - // within scopes, but the one use of `resolve_hir_path` i found in the HIR - // lowering code itself used an absolute path. we're brushing up against some - // structural limitations in the compiler already, but this may be a design one - // as well >_> - continue; - } - let path = { + let is_value: bool; + let path_str = if let Some(prefix) = + ["struct", "enum", "type", "trait", "union"].iter() + .find(|p| link.starts_with(**p)) { + is_value = false; + link.trim_left_matches(prefix).trim() + } else if let Some(prefix) = + ["const", "static"].iter() + .find(|p| link.starts_with(**p)) { + is_value = true; + link.trim_left_matches(prefix).trim() + } else if link.ends_with("()") { + is_value = true; + link.trim_right_matches("()").trim() + } else if link.ends_with("!") { + // FIXME (misdreavus): macros are resolved with different machinery + continue; + } else { + is_value = false; + link.trim() + }; + + if !path_str.starts_with("::") { + // FIXME (misdreavus): can only support absolute paths because of limitations + // in Resolver. this may, with a lot of effort, figure out how to resolve paths + // within scopes, but the one use of `resolve_hir_path` i found in the HIR + // lowering code itself used an absolute path. we're brushing up against some + // structural limitations in the compiler already, but this may be a design one + // as well >_> + continue; + } + // This allocation could be avoided if resolve_str_path could take an iterator; // but it can't because that would break object safety. This can still be // fixed. - let components = link.split("::").skip(1).collect::>(); - cx.resolver.borrow_mut().resolve_str_path(DUMMY_SP, None, &components, false) + let components = path_str.split("::").skip(1).collect::>(); + cx.resolver.borrow_mut().resolve_str_path(DUMMY_SP, None, &components, is_value) }; if path.def != Def::Err { From 30fca0919c8bcb7995982d4141b3a6f2e651c5a6 Mon Sep 17 00:00:00 2001 From: QuietMisdreavus Date: Thu, 28 Dec 2017 16:48:30 -0600 Subject: [PATCH 16/44] add basic test for rustdoc intra links --- src/test/rustdoc/intra-links.rs | 39 +++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) create mode 100644 src/test/rustdoc/intra-links.rs diff --git a/src/test/rustdoc/intra-links.rs b/src/test/rustdoc/intra-links.rs new file mode 100644 index 0000000000000..f1165c9a6e4cb --- /dev/null +++ b/src/test/rustdoc/intra-links.rs @@ -0,0 +1,39 @@ +// Copyright 2017 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// @has intra_links/index.html +// @has - '//a/@href' '../intra_links/struct.ThisType.html' +// @has - '//a/@href' '../intra_links/enum.ThisEnum.html' +// @has - '//a/@href' '../intra_links/trait.ThisTrait.html' +// @has - '//a/@href' '../intra_links/type.ThisAlias.html' +// @has - '//a/@href' '../intra_links/union.ThisUnion.html' +// @has - '//a/@href' '../intra_links/fn.this_function.html' +// @has - '//a/@href' '../intra_links/constant.THIS_CONST.html' +// @has - '//a/@href' '../intra_links/static.THIS_STATIC.html' +//! In this crate we would like to link to: +//! +//! * [`ThisType`](struct ::ThisType) +//! * [`ThisEnum`](enum ::ThisEnum) +//! * [`ThisTrait`](trait ::ThisTrait) +//! * [`ThisAlias`](type ::ThisAlias) +//! * [`ThisUnion`](union ::ThisUnion) +//! * [`this_function`](::this_function()) +//! * [`THIS_CONST`](const ::THIS_CONST) +//! * [`THIS_STATIC`](static ::THIS_STATIC) + +pub struct ThisType; +pub enum ThisEnum { ThisVariant, } +pub trait ThisTrait {} +pub type ThisAlias = Result<(), ()>; +pub union ThisUnion { this_field: usize, } + +pub fn this_function() {} +pub const THIS_CONST: usize = 5usize; +pub static THIS_STATIC: usize = 5usize; From f951d7438937df9938d49be4c53c45cdfaaf867e Mon Sep 17 00:00:00 2001 From: Manish Goregaokar Date: Fri, 29 Dec 2017 12:05:42 +0530 Subject: [PATCH 17/44] Don't return early and discard the link in hoedown mode --- src/librustdoc/html/markdown.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/librustdoc/html/markdown.rs b/src/librustdoc/html/markdown.rs index 6fd8eb4de5fc1..c76a47ed25b10 100644 --- a/src/librustdoc/html/markdown.rs +++ b/src/librustdoc/html/markdown.rs @@ -800,7 +800,7 @@ pub fn render(w: &mut fmt::Formatter, .find(|t| &*t.0 == &*link) { new_target.to_owned() } else { - return 0; + link }; let content = unsafe { From 140e77f71d227779a1520aabe10c74d2b7b80d5f Mon Sep 17 00:00:00 2001 From: Manish Goregaokar Date: Fri, 29 Dec 2017 13:36:32 +0530 Subject: [PATCH 18/44] Make resolve_hir_path and resolve_str_path fallible --- src/librustc/hir/lowering.rs | 13 +------ src/librustc_resolve/lib.rs | 70 ++++++++++++++++++++++++++++++------ 2 files changed, 61 insertions(+), 22 deletions(-) diff --git a/src/librustc/hir/lowering.rs b/src/librustc/hir/lowering.rs index f2154f885fa4a..a87f2747a57f5 100644 --- a/src/librustc/hir/lowering.rs +++ b/src/librustc/hir/lowering.rs @@ -155,18 +155,7 @@ pub trait Resolver { /// 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 { - 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 - } + components: &[&str], is_value: bool) -> hir::Path; } #[derive(Clone, Copy, Debug)] diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs index d9ae776a4d7b2..fdbed25179987 100644 --- a/src/librustc_resolve/lib.rs +++ b/src/librustc_resolve/lib.rs @@ -1407,6 +1407,64 @@ 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 { + self.resolve_str_path_cb(span, crate_root, components, is_value, + |resolver, span, error| resolve_error(resolver, span, error)) + } + + fn get_resolution(&mut self, id: NodeId) -> Option { + self.def_map.get(&id).cloned() + } + + fn definitions(&mut self) -> &mut Definitions { + &mut self.definitions + } +} + +impl<'a> Resolver<'a> { + /// resolve_str_path, but takes a callback in case there was an error + fn resolve_str_path_cb(&mut self, span: Span, crate_root: Option<&str>, + components: &[&str], is_value: bool, error_callback: F) -> hir::Path + where F: for<'b, 'c> FnOnce(&'c mut Resolver, Span, ResolutionError<'b>) + { + use std::iter; + 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_cb(&mut path, is_value, error_callback); + path + } + + /// 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, crate_root: Option<&str>, + components: &[&str], is_value: bool) -> Result { + let mut errored = false; + let path = self.resolve_str_path_cb(span, crate_root, components, 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(&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 = segments.iter() @@ -1418,24 +1476,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 { - self.def_map.get(&id).cloned() - } - - fn definitions(&mut self) -> &mut Definitions { - &mut self.definitions - } } impl<'a> Resolver<'a> { From d6dcc47f0d1abb50049a5d1503ce9a95b56efaed Mon Sep 17 00:00:00 2001 From: Manish Goregaokar Date: Fri, 29 Dec 2017 13:52:30 +0530 Subject: [PATCH 19/44] Handle errors for intra doc link path lookup --- src/librustdoc/clean/mod.rs | 41 ++++++++++++++++++++++++++++--------- 1 file changed, 31 insertions(+), 10 deletions(-) diff --git a/src/librustdoc/clean/mod.rs b/src/librustdoc/clean/mod.rs index 2c2cad1dc401d..206de77b00efe 100644 --- a/src/librustdoc/clean/mod.rs +++ b/src/librustdoc/clean/mod.rs @@ -34,7 +34,6 @@ use rustc::middle::resolve_lifetime as rl; use rustc::middle::lang_items; use rustc::hir::def::{Def, CtorKind}; use rustc::hir::def_id::{CrateNum, DefId, CRATE_DEF_INDEX, LOCAL_CRATE}; -use rustc::hir::lowering::Resolver; use rustc::ty::subst::Substs; use rustc::ty::{self, Ty, AdtKind}; use rustc::middle::stability; @@ -826,25 +825,25 @@ impl Clean for [ast::Attribute] { let dox = attrs.collapsed_doc_value().unwrap_or_else(String::new); for link in markdown_links(&dox, cx.render_type) { let path = { - let is_value: bool; + let is_value; let path_str = if let Some(prefix) = ["struct", "enum", "type", "trait", "union"].iter() .find(|p| link.starts_with(**p)) { - is_value = false; + is_value = Some(false); link.trim_left_matches(prefix).trim() } else if let Some(prefix) = ["const", "static"].iter() .find(|p| link.starts_with(**p)) { - is_value = true; + is_value = Some(true); link.trim_left_matches(prefix).trim() } else if link.ends_with("()") { - is_value = true; + is_value = Some(true); link.trim_right_matches("()").trim() } else if link.ends_with("!") { // FIXME (misdreavus): macros are resolved with different machinery continue; } else { - is_value = false; + is_value = None; link.trim() }; @@ -862,12 +861,34 @@ impl Clean for [ast::Attribute] { // but it can't because that would break object safety. This can still be // fixed. let components = path_str.split("::").skip(1).collect::>(); - cx.resolver.borrow_mut().resolve_str_path(DUMMY_SP, None, &components, is_value) + let resolve = |is_val| cx.resolver.borrow_mut().resolve_str_path_error(DUMMY_SP, None, &components, is_val); + + if let Some(is_value) = is_value { + if let Ok(path) = resolve(is_value) { + path + } else { + // this could just be a normal link or a broken link + // we could potentially check if something is + // "intra-doc-link-like" and warn in that case + continue; + } + } else { + // try both! + // It is imperative we search for not-a-value first + // Otherwise we will find struct ctors for when we are looking + // for structs, etc, and the link won't work. + if let Ok(path) = resolve(false) { + path + } else if let Ok(path) = resolve(true) { + path + } else { + // this could just be a normal link + continue; + } + } }; - if path.def != Def::Err { - attrs.links.push((link, path.def.def_id())); - } + attrs.links.push((link, path.def.def_id())); } cx.sess().abort_if_errors(); From 8166b59c74f423fbd9b5f1f21f63b9c1f972c9cc Mon Sep 17 00:00:00 2001 From: Manish Goregaokar Date: Mon, 1 Jan 2018 11:56:07 +0530 Subject: [PATCH 20/44] Use correct item for links in modules --- src/librustdoc/html/render.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/librustdoc/html/render.rs b/src/librustdoc/html/render.rs index 5dea05cedbeaf..832c0cc17d119 100644 --- a/src/librustdoc/html/render.rs +++ b/src/librustdoc/html/render.rs @@ -2160,10 +2160,10 @@ fn item_module(w: &mut fmt::Formatter, cx: &Context, stab_docs = stab_docs, docs = if cx.render_type == RenderType::Hoedown { format!("{}", - shorter(Some(&Markdown(doc_value, &item.links(), + shorter(Some(&Markdown(doc_value, &myitem.links(), RenderType::Hoedown).to_string()))) } else { - format!("{}", MarkdownSummaryLine(doc_value, &item.links())) + format!("{}", MarkdownSummaryLine(doc_value, &myitem.links())) }, class = myitem.type_(), stab = myitem.stability_class().unwrap_or("".to_string()), From 4f10f676d9e39940bf54ebfc7017f1505837532f Mon Sep 17 00:00:00 2001 From: Manish Goregaokar Date: Mon, 1 Jan 2018 13:01:19 +0530 Subject: [PATCH 21/44] Handle relative paths --- src/librustc_resolve/lib.rs | 64 +++++++++++++++++++++---------------- src/librustdoc/clean/mod.rs | 36 ++++++++++++--------- src/librustdoc/core.rs | 4 +++ 3 files changed, 61 insertions(+), 43 deletions(-) diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs index fdbed25179987..07c940612b16c 100644 --- a/src/librustc_resolve/lib.rs +++ b/src/librustc_resolve/lib.rs @@ -1413,25 +1413,6 @@ impl<'a> hir::lowering::Resolver for Resolver<'a> { fn resolve_str_path(&mut self, span: Span, crate_root: Option<&str>, components: &[&str], is_value: bool) -> hir::Path { - self.resolve_str_path_cb(span, crate_root, components, is_value, - |resolver, span, error| resolve_error(resolver, span, error)) - } - - fn get_resolution(&mut self, id: NodeId) -> Option { - self.def_map.get(&id).cloned() - } - - fn definitions(&mut self) -> &mut Definitions { - &mut self.definitions - } -} - -impl<'a> Resolver<'a> { - /// resolve_str_path, but takes a callback in case there was an error - fn resolve_str_path_cb(&mut self, span: Span, crate_root: Option<&str>, - components: &[&str], is_value: bool, error_callback: F) -> hir::Path - where F: for<'b, 'c> FnOnce(&'c mut Resolver, Span, ResolutionError<'b>) - { use std::iter; let mut path = hir::Path { span, @@ -1441,19 +1422,45 @@ impl<'a> Resolver<'a> { }).map(hir::PathSegment::from_name).collect(), }; - self.resolve_hir_path_cb(&mut path, is_value, error_callback); + self.resolve_hir_path(&mut path, is_value); path } + fn get_resolution(&mut self, id: NodeId) -> Option { + 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, crate_root: Option<&str>, - components: &[&str], is_value: bool) -> Result { + pub fn resolve_str_path_error(&mut self, span: Span, path_str: &str, is_value: bool) -> Result { + use std::iter; let mut errored = false; - let path = self.resolve_str_path_cb(span, crate_root, components, is_value, - |_, _, _| errored = true); + + 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 { @@ -1883,8 +1890,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(&mut self, id: NodeId, f: F) - where F: FnOnce(&mut Resolver) + pub fn with_scope(&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 @@ -1895,13 +1902,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) } } diff --git a/src/librustdoc/clean/mod.rs b/src/librustdoc/clean/mod.rs index 206de77b00efe..7daf8a7ae6d13 100644 --- a/src/librustdoc/clean/mod.rs +++ b/src/librustdoc/clean/mod.rs @@ -472,6 +472,11 @@ impl Clean for doctree::Module { "".to_string() }; + // maintain a stack of mod ids + // we could also pass this down through clean() + // but that might complicate things. + cx.mod_ids.borrow_mut().push(self.id); + let mut items: Vec = vec![]; items.extend(self.extern_crates.iter().map(|x| x.clean(cx))); items.extend(self.imports.iter().flat_map(|x| x.clean(cx))); @@ -488,6 +493,8 @@ impl Clean for doctree::Module { items.extend(self.impls.iter().flat_map(|x| x.clean(cx))); items.extend(self.macros.iter().map(|x| x.clean(cx))); + cx.mod_ids.borrow_mut().pop(); + // determine if we should display the inner contents or // the outer `mod` item for the source code. let whence = { @@ -847,21 +854,20 @@ impl Clean for [ast::Attribute] { link.trim() }; - if !path_str.starts_with("::") { - // FIXME (misdreavus): can only support absolute paths because of limitations - // in Resolver. this may, with a lot of effort, figure out how to resolve paths - // within scopes, but the one use of `resolve_hir_path` i found in the HIR - // lowering code itself used an absolute path. we're brushing up against some - // structural limitations in the compiler already, but this may be a design one - // as well >_> - continue; - } - - // This allocation could be avoided if resolve_str_path could take an iterator; - // but it can't because that would break object safety. This can still be - // fixed. - let components = path_str.split("::").skip(1).collect::>(); - let resolve = |is_val| cx.resolver.borrow_mut().resolve_str_path_error(DUMMY_SP, None, &components, is_val); + let resolve = |is_val| { + // In case we're in a module, try to resolve the relative + // path + if let Some(id) = cx.mod_ids.borrow().last() { + cx.resolver.borrow_mut() + .with_scope(*id, |resolver| { + resolver.resolve_str_path_error(DUMMY_SP, &path_str, is_val) + }) + } else { + // FIXME(Manishearth) this branch doesn't seem to ever be hit, really + cx.resolver.borrow_mut() + .resolve_str_path_error(DUMMY_SP, &path_str, is_val) + } + }; if let Some(is_value) = is_value { if let Ok(path) = resolve(is_value) { diff --git a/src/librustdoc/core.rs b/src/librustdoc/core.rs index 6fc21f0541b1b..5fe4794389ff5 100644 --- a/src/librustdoc/core.rs +++ b/src/librustdoc/core.rs @@ -23,6 +23,7 @@ use rustc_resolve as resolve; use rustc_metadata::creader::CrateLoader; use rustc_metadata::cstore::CStore; +use syntax::ast::NodeId; use syntax::codemap; use syntax::feature_gate::UnstableFeatures; use errors; @@ -47,6 +48,8 @@ pub type ExternalPaths = FxHashMap, clean::TypeKind)>; pub struct DocContext<'a, 'tcx: 'a, 'rcx: 'a> { pub tcx: TyCtxt<'a, 'tcx, 'tcx>, pub resolver: &'a RefCell>, + /// The stack of module NodeIds up till this point + pub mod_ids: RefCell>, pub populated_all_crate_impls: Cell, // Note that external items for which `doc(hidden)` applies to are shown as // non-reachable while local items aren't. This is because we're reusing @@ -243,6 +246,7 @@ pub fn run_core(search_paths: SearchPaths, render_type, ty_substs: Default::default(), lt_substs: Default::default(), + mod_ids: Default::default(), }; debug!("crate: {:?}", tcx.hir.krate()); From 191e5b0b78eae71ed9d59d03f2292501dd6d488a Mon Sep 17 00:00:00 2001 From: Manish Goregaokar Date: Mon, 1 Jan 2018 14:22:28 +0530 Subject: [PATCH 22/44] Exit early for non-linky things --- src/librustdoc/clean/mod.rs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/librustdoc/clean/mod.rs b/src/librustdoc/clean/mod.rs index 7daf8a7ae6d13..dcf12ed8180e8 100644 --- a/src/librustdoc/clean/mod.rs +++ b/src/librustdoc/clean/mod.rs @@ -854,6 +854,13 @@ impl Clean for [ast::Attribute] { link.trim() }; + // avoid resolving things (i.e. regular links) which aren't like paths + // FIXME(Manishearth) given that most links have slashes in them might be worth + // doing a check for slashes first + if path_str.contains(|ch: char| !(ch.is_alphanumeric() || ch == ':' || ch == '_')) { + continue; + } + let resolve = |is_val| { // In case we're in a module, try to resolve the relative // path From c0af89723d5fee5c4bd7b54f7d7885b4faac00d8 Mon Sep 17 00:00:00 2001 From: Manish Goregaokar Date: Wed, 3 Jan 2018 11:33:04 +0530 Subject: [PATCH 23/44] Fix tidy --- src/librustc_resolve/lib.rs | 3 ++- src/librustdoc/clean/mod.rs | 11 +++++++---- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs index 07c940612b16c..6b7ad83946019 100644 --- a/src/librustc_resolve/lib.rs +++ b/src/librustc_resolve/lib.rs @@ -1440,7 +1440,8 @@ impl<'a> Resolver<'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 { + pub fn resolve_str_path_error(&mut self, span: Span, path_str: &str, is_value: bool) + -> Result { use std::iter; let mut errored = false; diff --git a/src/librustdoc/clean/mod.rs b/src/librustdoc/clean/mod.rs index dcf12ed8180e8..59a797b55d062 100644 --- a/src/librustdoc/clean/mod.rs +++ b/src/librustdoc/clean/mod.rs @@ -834,8 +834,9 @@ impl Clean for [ast::Attribute] { let path = { let is_value; let path_str = if let Some(prefix) = - ["struct", "enum", "type", "trait", "union"].iter() - .find(|p| link.starts_with(**p)) { + ["struct", "enum", "type", + "trait", "union"].iter() + .find(|p| link.starts_with(**p)) { is_value = Some(false); link.trim_left_matches(prefix).trim() } else if let Some(prefix) = @@ -857,7 +858,8 @@ impl Clean for [ast::Attribute] { // avoid resolving things (i.e. regular links) which aren't like paths // FIXME(Manishearth) given that most links have slashes in them might be worth // doing a check for slashes first - if path_str.contains(|ch: char| !(ch.is_alphanumeric() || ch == ':' || ch == '_')) { + if path_str.contains(|ch: char| !(ch.is_alphanumeric() || + ch == ':' || ch == '_')) { continue; } @@ -867,7 +869,8 @@ impl Clean for [ast::Attribute] { if let Some(id) = cx.mod_ids.borrow().last() { cx.resolver.borrow_mut() .with_scope(*id, |resolver| { - resolver.resolve_str_path_error(DUMMY_SP, &path_str, is_val) + resolver.resolve_str_path_error(DUMMY_SP, + &path_str, is_val) }) } else { // FIXME(Manishearth) this branch doesn't seem to ever be hit, really From 383d169e15361d47a29087e598c5be470e06c3cc Mon Sep 17 00:00:00 2001 From: Manish Goregaokar Date: Wed, 3 Jan 2018 14:11:54 +0530 Subject: [PATCH 24/44] Fix unit tests --- src/librustdoc/html/markdown.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/librustdoc/html/markdown.rs b/src/librustdoc/html/markdown.rs index c76a47ed25b10..c8de48ea42b30 100644 --- a/src/librustdoc/html/markdown.rs +++ b/src/librustdoc/html/markdown.rs @@ -1374,14 +1374,14 @@ mod tests { #[test] fn issue_17736() { let markdown = "# title"; - format!("{}", Markdown(markdown, RenderType::Pulldown)); + format!("{}", Markdown(markdown, &[], RenderType::Pulldown)); reset_ids(true); } #[test] fn test_header() { fn t(input: &str, expect: &str) { - let output = format!("{}", Markdown(input, RenderType::Pulldown)); + let output = format!("{}", Markdown(input, &[], RenderType::Pulldown)); assert_eq!(output, expect, "original: {}", input); reset_ids(true); } @@ -1403,7 +1403,7 @@ mod tests { #[test] fn test_header_ids_multiple_blocks() { fn t(input: &str, expect: &str) { - let output = format!("{}", Markdown(input, RenderType::Pulldown)); + let output = format!("{}", Markdown(input, &[], RenderType::Pulldown)); assert_eq!(output, expect, "original: {}", input); } From d6dd902616448e9b86285f5056b32ac4d49e98ac Mon Sep 17 00:00:00 2001 From: Manish Goregaokar Date: Sat, 6 Jan 2018 15:45:44 +0530 Subject: [PATCH 25/44] Register definitions --- src/librustdoc/clean/mod.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/librustdoc/clean/mod.rs b/src/librustdoc/clean/mod.rs index 59a797b55d062..fb5fee9908ad9 100644 --- a/src/librustdoc/clean/mod.rs +++ b/src/librustdoc/clean/mod.rs @@ -904,6 +904,8 @@ impl Clean for [ast::Attribute] { } }; + register_def(cx, def); + attrs.links.push((link, path.def.def_id())); } From 7ac48d793bc06d2646244e070c25cc129e539d17 Mon Sep 17 00:00:00 2001 From: Manish Goregaokar Date: Sat, 6 Jan 2018 14:01:54 +0530 Subject: [PATCH 26/44] Resolve foreign macros --- src/librustc_resolve/macros.rs | 2 +- src/librustdoc/clean/inline.rs | 6 +- src/librustdoc/clean/mod.rs | 107 ++++++++++++++++++++++--------- src/librustdoc/html/item_type.rs | 1 + src/librustdoc/html/render.rs | 2 +- 5 files changed, 84 insertions(+), 34 deletions(-) diff --git a/src/librustc_resolve/macros.rs b/src/librustc_resolve/macros.rs index ceb39aea108c8..e06d7dce4dbef 100644 --- a/src/librustc_resolve/macros.rs +++ b/src/librustc_resolve/macros.rs @@ -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 { let ast::Path { ref segments, span } = *path; diff --git a/src/librustdoc/clean/inline.rs b/src/librustdoc/clean/inline.rs index b8c34d78d305e..e4e3cc2acd5ef 100644 --- a/src/librustdoc/clean/inline.rs +++ b/src/librustdoc/clean/inline.rs @@ -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)); } diff --git a/src/librustdoc/clean/mod.rs b/src/librustdoc/clean/mod.rs index fb5fee9908ad9..74d1fd0d14e14 100644 --- a/src/librustdoc/clean/mod.rs +++ b/src/librustdoc/clean/mod.rs @@ -824,6 +824,16 @@ impl AttributesExt for Attributes { } } +enum PathKind { + /// can be either value or type, not a macro + Unknown, + /// macro + Macro, + /// values, functions, consts, statics, everything in the value namespace + Value, + /// types, traits, everything in the type namespace + Type +} impl Clean for [ast::Attribute] { fn clean(&self, cx: &DocContext) -> Attributes { let mut attrs = Attributes::from_ast(cx.sess().diagnostic(), self); @@ -831,27 +841,26 @@ impl Clean for [ast::Attribute] { if UnstableFeatures::from_environment().is_nightly_build() { let dox = attrs.collapsed_doc_value().unwrap_or_else(String::new); for link in markdown_links(&dox, cx.render_type) { - let path = { - let is_value; + let def = { + let mut kind = PathKind::Unknown; let path_str = if let Some(prefix) = ["struct", "enum", "type", "trait", "union"].iter() .find(|p| link.starts_with(**p)) { - is_value = Some(false); + kind = PathKind::Type; link.trim_left_matches(prefix).trim() } else if let Some(prefix) = ["const", "static"].iter() .find(|p| link.starts_with(**p)) { - is_value = Some(true); + kind = PathKind::Value; link.trim_left_matches(prefix).trim() } else if link.ends_with("()") { - is_value = Some(true); + kind = PathKind::Value; link.trim_right_matches("()").trim() - } else if link.ends_with("!") { - // FIXME (misdreavus): macros are resolved with different machinery - continue; + } else if link.ends_with('!') { + kind = PathKind::Macro; + link.trim_right_matches('!').trim() } else { - is_value = None; link.trim() }; @@ -879,34 +888,68 @@ impl Clean for [ast::Attribute] { } }; - if let Some(is_value) = is_value { - if let Ok(path) = resolve(is_value) { - path - } else { - // this could just be a normal link or a broken link - // we could potentially check if something is - // "intra-doc-link-like" and warn in that case - continue; + match kind { + PathKind::Value => { + if let Ok(path) = resolve(true) { + path.def + } else { + // this could just be a normal link or a broken link + // we could potentially check if something is + // "intra-doc-link-like" and warn in that case + continue; + } } - } else { - // try both! - // It is imperative we search for not-a-value first - // Otherwise we will find struct ctors for when we are looking - // for structs, etc, and the link won't work. - if let Ok(path) = resolve(false) { - path - } else if let Ok(path) = resolve(true) { - path - } else { - // this could just be a normal link - continue; + PathKind::Type => { + if let Ok(path) = resolve(false) { + path.def + } else { + // this could just be a normal link + continue; + } + } + PathKind::Unknown => { + // try both! + // It is imperative we search for not-a-value first + // Otherwise we will find struct ctors for when we are looking + // for structs, etc, and the link won't work. + if let Ok(path) = resolve(false) { + path.def + } else if let Ok(path) = resolve(true) { + path.def + } else { + // this could just be a normal link + continue; + } + } + PathKind::Macro => { + use syntax::ext::base::MacroKind; + use syntax::ext::hygiene::Mark; + let segment = ast::PathSegment { + identifier: ast::Ident::from_str(path_str), + span: DUMMY_SP, + parameters: None, + }; + let path = ast::Path { + span: DUMMY_SP, + segments: vec![segment], + }; + + let mut resolver = cx.resolver.borrow_mut(); + let mark = Mark::root(); + let res = resolver + .resolve_macro_to_def_inner(mark, &path, MacroKind::Bang, false); + if let Ok(def) = res { + def + } else { + continue; + } } } }; - register_def(cx, def); - attrs.links.push((link, path.def.def_id())); + register_def(cx, def); + attrs.links.push((link, def.def_id())); } cx.sess().abort_if_errors(); @@ -1970,6 +2013,7 @@ pub enum TypeKind { Variant, Typedef, Foreign, + Macro, } pub trait GetDefId { @@ -3271,6 +3315,7 @@ fn register_def(cx: &DocContext, def: Def) -> DefId { Def::TyForeign(i) => (i, TypeKind::Foreign), Def::Static(i, _) => (i, TypeKind::Static), Def::Variant(i) => (cx.tcx.parent_def_id(i).unwrap(), TypeKind::Enum), + Def::Macro(i, _) => (i, TypeKind::Macro), Def::SelfTy(Some(def_id), _) => (def_id, TypeKind::Trait), Def::SelfTy(_, Some(impl_def_id)) => { return impl_def_id diff --git a/src/librustdoc/html/item_type.rs b/src/librustdoc/html/item_type.rs index 81087cd412e2c..e9c6488c49c6c 100644 --- a/src/librustdoc/html/item_type.rs +++ b/src/librustdoc/html/item_type.rs @@ -102,6 +102,7 @@ impl From for ItemType { clean::TypeKind::Variant => ItemType::Variant, clean::TypeKind::Typedef => ItemType::Typedef, clean::TypeKind::Foreign => ItemType::ForeignType, + clean::TypeKind::Macro => ItemType::Macro, } } } diff --git a/src/librustdoc/html/render.rs b/src/librustdoc/html/render.rs index 832c0cc17d119..b58a59f12173c 100644 --- a/src/librustdoc/html/render.rs +++ b/src/librustdoc/html/render.rs @@ -1284,7 +1284,7 @@ impl DocFolder for Cache { clean::FunctionItem(..) | clean::ModuleItem(..) | clean::ForeignFunctionItem(..) | clean::ForeignStaticItem(..) | clean::ConstantItem(..) | clean::StaticItem(..) | - clean::UnionItem(..) | clean::ForeignTypeItem + clean::UnionItem(..) | clean::ForeignTypeItem | clean::MacroItem(..) if !self.stripped_mod => { // Re-exported items mean that the same id can show up twice // in the rustdoc ast that we're looking at. We know, From 00ce770e348a4d00985b8872fc4874480115f621 Mon Sep 17 00:00:00 2001 From: Manish Goregaokar Date: Sat, 6 Jan 2018 17:23:33 +0530 Subject: [PATCH 27/44] Store a list of local macros on the resolver; use for resolving intra-doc macro links --- src/librustc_resolve/lib.rs | 2 ++ src/librustc_resolve/macros.rs | 3 ++- src/librustdoc/clean/mod.rs | 2 ++ 3 files changed, 6 insertions(+), 1 deletion(-) diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs index 6b7ad83946019..c55e23e917a1c 100644 --- a/src/librustc_resolve/lib.rs +++ b/src/librustc_resolve/lib.rs @@ -1320,6 +1320,7 @@ pub struct Resolver<'a> { crate_loader: &'a mut CrateLoader, macro_names: FxHashSet, global_macros: FxHashMap>, + pub all_macros: FxHashMap, lexical_macro_resolutions: Vec<(Ident, &'a Cell>)>, macro_map: FxHashMap>, macro_defs: FxHashMap, @@ -1596,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(), diff --git a/src/librustc_resolve/macros.rs b/src/librustc_resolve/macros.rs index e06d7dce4dbef..080ef3252a633 100644 --- a/src/librustc_resolve/macros.rs +++ b/src/librustc_resolve/macros.rs @@ -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, diff --git a/src/librustdoc/clean/mod.rs b/src/librustdoc/clean/mod.rs index 74d1fd0d14e14..5031dddfdf38f 100644 --- a/src/librustdoc/clean/mod.rs +++ b/src/librustdoc/clean/mod.rs @@ -940,6 +940,8 @@ impl Clean for [ast::Attribute] { .resolve_macro_to_def_inner(mark, &path, MacroKind::Bang, false); if let Ok(def) = res { def + } else if let Some(def) = resolver.all_macros.get(&path_str.into()) { + *def } else { continue; } From a3d71d7405d7a7f2ca846a57597841502a2272ef Mon Sep 17 00:00:00 2001 From: QuietMisdreavus Date: Sun, 7 Jan 2018 17:09:16 -0600 Subject: [PATCH 28/44] add a macro to the intra-links test --- src/test/rustdoc/intra-links.rs | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/src/test/rustdoc/intra-links.rs b/src/test/rustdoc/intra-links.rs index f1165c9a6e4cb..5039d7f30cb6e 100644 --- a/src/test/rustdoc/intra-links.rs +++ b/src/test/rustdoc/intra-links.rs @@ -17,16 +17,23 @@ // @has - '//a/@href' '../intra_links/fn.this_function.html' // @has - '//a/@href' '../intra_links/constant.THIS_CONST.html' // @has - '//a/@href' '../intra_links/static.THIS_STATIC.html' +// @has - '//a/@href' '../intra_links/macro.this_macro.html' //! In this crate we would like to link to: //! -//! * [`ThisType`](struct ::ThisType) -//! * [`ThisEnum`](enum ::ThisEnum) -//! * [`ThisTrait`](trait ::ThisTrait) -//! * [`ThisAlias`](type ::ThisAlias) -//! * [`ThisUnion`](union ::ThisUnion) -//! * [`this_function`](::this_function()) -//! * [`THIS_CONST`](const ::THIS_CONST) -//! * [`THIS_STATIC`](static ::THIS_STATIC) +//! * [`ThisType`](ThisType) +//! * [`ThisEnum`](ThisEnum) +//! * [`ThisTrait`](ThisTrait) +//! * [`ThisAlias`](ThisAlias) +//! * [`ThisUnion`](ThisUnion) +//! * [`this_function`](this_function) +//! * [`THIS_CONST`](THIS_CONST) +//! * [`THIS_STATIC`](THIS_STATIC) +//! * [`this_macro`](this_macro!) + +#[macro_export] +macro_rules! this_macro { + () => {}; +} pub struct ThisType; pub enum ThisEnum { ThisVariant, } From 4a20fb44c807f57070841618121ccc601bfffae8 Mon Sep 17 00:00:00 2001 From: QuietMisdreavus Date: Mon, 8 Jan 2018 09:23:12 -0600 Subject: [PATCH 29/44] use @ instead of space for link ambiguity markers since spaces aren't allowed in link targets in commonmark, a new symbol is needed to separate the marker from the rest of the path. hence, @ --- src/librustdoc/clean/mod.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/librustdoc/clean/mod.rs b/src/librustdoc/clean/mod.rs index 5031dddfdf38f..d4260c3dfb58c 100644 --- a/src/librustdoc/clean/mod.rs +++ b/src/librustdoc/clean/mod.rs @@ -848,21 +848,21 @@ impl Clean for [ast::Attribute] { "trait", "union"].iter() .find(|p| link.starts_with(**p)) { kind = PathKind::Type; - link.trim_left_matches(prefix).trim() + link.trim_left_matches(prefix).trim_left_matches('@') } else if let Some(prefix) = ["const", "static"].iter() .find(|p| link.starts_with(**p)) { kind = PathKind::Value; - link.trim_left_matches(prefix).trim() + link.trim_left_matches(prefix).trim_left_matches('@') } else if link.ends_with("()") { kind = PathKind::Value; - link.trim_right_matches("()").trim() + link.trim_right_matches("()") } else if link.ends_with('!') { kind = PathKind::Macro; - link.trim_right_matches('!').trim() + link.trim_right_matches('!') } else { - link.trim() - }; + &link[..] + }.trim(); // avoid resolving things (i.e. regular links) which aren't like paths // FIXME(Manishearth) given that most links have slashes in them might be worth From eca3c558817ad6f2be485515d0131c6bbd80340e Mon Sep 17 00:00:00 2001 From: QuietMisdreavus Date: Mon, 8 Jan 2018 09:33:27 -0600 Subject: [PATCH 30/44] add ambiguity markers to the intra-links test --- src/test/rustdoc/intra-links.rs | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/src/test/rustdoc/intra-links.rs b/src/test/rustdoc/intra-links.rs index 5039d7f30cb6e..cc9d288ba9141 100644 --- a/src/test/rustdoc/intra-links.rs +++ b/src/test/rustdoc/intra-links.rs @@ -18,6 +18,8 @@ // @has - '//a/@href' '../intra_links/constant.THIS_CONST.html' // @has - '//a/@href' '../intra_links/static.THIS_STATIC.html' // @has - '//a/@href' '../intra_links/macro.this_macro.html' +// @has - '//a/@href' '../intra_links/trait.SoAmbiguous.html' +// @has - '//a/@href' '../intra_links/fn.SoAmbiguous.html' //! In this crate we would like to link to: //! //! * [`ThisType`](ThisType) @@ -29,6 +31,13 @@ //! * [`THIS_CONST`](THIS_CONST) //! * [`THIS_STATIC`](THIS_STATIC) //! * [`this_macro`](this_macro!) +//! +//! In addition, there's some specifics we want to look at. There's [a trait called +//! SoAmbiguous][ambig-trait], but there's also [a function called SoAmbiguous][ambig-fn] too! +//! Whatever shall we do? +//! +//! [ambig-trait]: trait@SoAmbiguous +//! [ambig-fn]: SoAmbiguous() #[macro_export] macro_rules! this_macro { @@ -44,3 +53,8 @@ pub union ThisUnion { this_field: usize, } pub fn this_function() {} pub const THIS_CONST: usize = 5usize; pub static THIS_STATIC: usize = 5usize; + +pub trait SoAmbiguous {} + +#[allow(bad_style)] +pub fn SoAmbiguous() {} From 1a62b17f7d68dfa1ec2f25e02e5cd3dc0095ea94 Mon Sep 17 00:00:00 2001 From: QuietMisdreavus Date: Mon, 8 Jan 2018 10:07:16 -0600 Subject: [PATCH 31/44] clean module docs while its module ID is still on the stack --- src/librustdoc/clean/mod.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/librustdoc/clean/mod.rs b/src/librustdoc/clean/mod.rs index d4260c3dfb58c..cd78209df3055 100644 --- a/src/librustdoc/clean/mod.rs +++ b/src/librustdoc/clean/mod.rs @@ -476,6 +476,7 @@ impl Clean for doctree::Module { // we could also pass this down through clean() // but that might complicate things. cx.mod_ids.borrow_mut().push(self.id); + let attrs = self.attrs.clean(cx); let mut items: Vec = vec![]; items.extend(self.extern_crates.iter().map(|x| x.clean(cx))); @@ -512,7 +513,7 @@ impl Clean for doctree::Module { Item { name: Some(name), - attrs: self.attrs.clean(cx), + attrs, source: whence.clean(cx), visibility: self.vis.clean(cx), stability: self.stab.clean(cx), From b31bb097f519f03a5925da376d4ef2fbc8f30c62 Mon Sep 17 00:00:00 2001 From: QuietMisdreavus Date: Mon, 8 Jan 2018 11:10:50 -0600 Subject: [PATCH 32/44] resolve module docs based on inner/outer attributes --- src/librustdoc/clean/mod.rs | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/src/librustdoc/clean/mod.rs b/src/librustdoc/clean/mod.rs index cd78209df3055..e1dbcb27864d8 100644 --- a/src/librustdoc/clean/mod.rs +++ b/src/librustdoc/clean/mod.rs @@ -20,7 +20,7 @@ pub use self::FunctionRetTy::*; pub use self::Visibility::*; use syntax::abi::Abi; -use syntax::ast; +use syntax::ast::{self, AttrStyle}; use syntax::attr; use syntax::codemap::Spanned; use syntax::feature_gate::UnstableFeatures; @@ -472,11 +472,22 @@ impl Clean for doctree::Module { "".to_string() }; - // maintain a stack of mod ids - // we could also pass this down through clean() - // but that might complicate things. - cx.mod_ids.borrow_mut().push(self.id); - let attrs = self.attrs.clean(cx); + // maintain a stack of mod ids, for doc comment path resolution + // but we also need to resolve the module's own docs based on whether its docs were written + // inside or outside the module, so check for that + let attrs = if self.attrs.iter() + .filter(|a| a.check_name("doc")) + .next() + .map_or(true, |a| a.style == AttrStyle::Inner) { + // inner doc comment, use the module's own scope for resolution + cx.mod_ids.borrow_mut().push(self.id); + self.attrs.clean(cx) + } else { + // outer doc comment, use its parent's scope + let attrs = self.attrs.clean(cx); + cx.mod_ids.borrow_mut().push(self.id); + attrs + }; let mut items: Vec = vec![]; items.extend(self.extern_crates.iter().map(|x| x.clean(cx))); From afe3e27085752efd1d9affe1f9c95e67de4dcf5d Mon Sep 17 00:00:00 2001 From: QuietMisdreavus Date: Mon, 8 Jan 2018 13:20:19 -0600 Subject: [PATCH 33/44] value-namespace items require a marker, so emit an error --- src/librustdoc/clean/mod.rs | 10 ++++++++-- src/test/rustdoc/intra-links.rs | 6 +++--- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/src/librustdoc/clean/mod.rs b/src/librustdoc/clean/mod.rs index e1dbcb27864d8..5e8d6301acb54 100644 --- a/src/librustdoc/clean/mod.rs +++ b/src/librustdoc/clean/mod.rs @@ -926,8 +926,14 @@ impl Clean for [ast::Attribute] { // for structs, etc, and the link won't work. if let Ok(path) = resolve(false) { path.def - } else if let Ok(path) = resolve(true) { - path.def + } else if let Ok(_path) = resolve(true) { + let sp = attrs.doc_strings.first().map_or(DUMMY_SP, |a| a.span()); + cx.sess().struct_span_err(sp, &format!("could not resolve `{}`", + path_str)) + .help(&format!("try `{0}()`, `static@{0}`, or `const@{0}`", + path_str)) + .emit(); + continue; } else { // this could just be a normal link continue; diff --git a/src/test/rustdoc/intra-links.rs b/src/test/rustdoc/intra-links.rs index cc9d288ba9141..aa6f553875441 100644 --- a/src/test/rustdoc/intra-links.rs +++ b/src/test/rustdoc/intra-links.rs @@ -27,9 +27,9 @@ //! * [`ThisTrait`](ThisTrait) //! * [`ThisAlias`](ThisAlias) //! * [`ThisUnion`](ThisUnion) -//! * [`this_function`](this_function) -//! * [`THIS_CONST`](THIS_CONST) -//! * [`THIS_STATIC`](THIS_STATIC) +//! * [`this_function`](this_function()) +//! * [`THIS_CONST`](const@THIS_CONST) +//! * [`THIS_STATIC`](static@THIS_STATIC) //! * [`this_macro`](this_macro!) //! //! In addition, there's some specifics we want to look at. There's [a trait called From 28805fd53e0fcff3e552e1fb90f8ab20bab68c5b Mon Sep 17 00:00:00 2001 From: Manish Goregaokar Date: Tue, 9 Jan 2018 10:50:52 +0530 Subject: [PATCH 34/44] Better error message --- src/librustdoc/clean/mod.rs | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/src/librustdoc/clean/mod.rs b/src/librustdoc/clean/mod.rs index 5e8d6301acb54..4bc7b81b3f6ea 100644 --- a/src/librustdoc/clean/mod.rs +++ b/src/librustdoc/clean/mod.rs @@ -926,12 +926,19 @@ impl Clean for [ast::Attribute] { // for structs, etc, and the link won't work. if let Ok(path) = resolve(false) { path.def - } else if let Ok(_path) = resolve(true) { + } else if let Ok(path) = resolve(true) { + let kind = match path.def { + Def::Variant(..) | Def::VariantCtor(..) => ("variant", format!("{}()", path_str)), + Def::Fn(..) => ("function", format!("{}()", path_str)), + Def::Method(..) => ("method", format!("{}()", path_str)), + Def::Const(..) => ("const", format!("const@{}", path_str)), + Def::Static(..) => ("static", format!("static@{}", path_str)), + _ => ("value", format!("static@{}", path_str)), + }; let sp = attrs.doc_strings.first().map_or(DUMMY_SP, |a| a.span()); - cx.sess().struct_span_err(sp, &format!("could not resolve `{}`", - path_str)) - .help(&format!("try `{0}()`, `static@{0}`, or `const@{0}`", - path_str)) + cx.sess().struct_span_err(sp, &format!("could not resolve `{}` as a type, it is a {}", + path_str, kind.0)) + .help(&format!("try `{}`", kind.1)) .emit(); continue; } else { From d44910ceeb03b053a8ff641395c4aff505caaaa6 Mon Sep 17 00:00:00 2001 From: Manish Goregaokar Date: Tue, 9 Jan 2018 11:55:28 +0530 Subject: [PATCH 35/44] Use the registered def id (makes enum variants link to the enum page instead of not at all) --- src/librustdoc/clean/mod.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/librustdoc/clean/mod.rs b/src/librustdoc/clean/mod.rs index 4bc7b81b3f6ea..97b60e931bc68 100644 --- a/src/librustdoc/clean/mod.rs +++ b/src/librustdoc/clean/mod.rs @@ -975,8 +975,8 @@ impl Clean for [ast::Attribute] { }; - register_def(cx, def); - attrs.links.push((link, def.def_id())); + let id = register_def(cx, def); + attrs.links.push((link, id)); } cx.sess().abort_if_errors(); From 6a1a449220d01dbee0f26b5f1b50a012e6b1bd30 Mon Sep 17 00:00:00 2001 From: Manish Goregaokar Date: Wed, 10 Jan 2018 11:06:51 +0530 Subject: [PATCH 36/44] Error only in the case of overlap --- src/librustdoc/clean/mod.rs | 69 ++++++++++++++++++++++++++----------- 1 file changed, 49 insertions(+), 20 deletions(-) diff --git a/src/librustdoc/clean/mod.rs b/src/librustdoc/clean/mod.rs index 97b60e931bc68..5a09197fd4194 100644 --- a/src/librustdoc/clean/mod.rs +++ b/src/librustdoc/clean/mod.rs @@ -856,16 +856,16 @@ impl Clean for [ast::Attribute] { let def = { let mut kind = PathKind::Unknown; let path_str = if let Some(prefix) = - ["struct", "enum", "type", - "trait", "union"].iter() + ["struct@", "enum@", "type@", + "trait@", "union@"].iter() .find(|p| link.starts_with(**p)) { kind = PathKind::Type; - link.trim_left_matches(prefix).trim_left_matches('@') + link.trim_left_matches(prefix) } else if let Some(prefix) = - ["const", "static"].iter() + ["const@", "static@"].iter() .find(|p| link.starts_with(**p)) { kind = PathKind::Value; - link.trim_left_matches(prefix).trim_left_matches('@') + link.trim_left_matches(prefix) } else if link.ends_with("()") { kind = PathKind::Value; link.trim_right_matches("()") @@ -923,24 +923,53 @@ impl Clean for [ast::Attribute] { // try both! // It is imperative we search for not-a-value first // Otherwise we will find struct ctors for when we are looking - // for structs, etc, and the link won't work. + // for structs, and the link won't work. if let Ok(path) = resolve(false) { + // if there is something in both namespaces + if let Ok(value_path) = resolve(true) { + let kind = match value_path.def { + // structs and mods exist in both namespaces. skip them + Def::StructCtor(..) | Def::Mod(..) => None, + Def::Variant(..) | Def::VariantCtor(..) + => Some(("variant", format!("{}()", path_str))), + Def::Fn(..) + => Some(("function", format!("{}()", path_str))), + Def::Method(..) + => Some(("method", format!("{}()", path_str))), + Def::Const(..) + => Some(("const", format!("const@{}", path_str))), + Def::Static(..) + => Some(("static", format!("static@{}", path_str))), + _ => Some(("value", format!("static@{}", path_str))), + }; + if let Some((value_kind, disambig)) = kind { + let (type_kind, article) = match path.def { + // we can still have non-tuple structs + Def::Struct(..) => ("struct", "a"), + Def::Enum(..) => ("enum", "an"), + Def::Trait(..) => ("trait", "a"), + Def::Union(..) => ("union", "a"), + _ => ("type", "a"), + }; + let sp = attrs.doc_strings.first() + .map_or(DUMMY_SP, |a| a.span()); + cx.sess() + .struct_span_err(sp, + &format!("`{}` is both {} {} and a {}", + path_str, article, type_kind, + value_kind)) + .help(&format!("try `{0}` if you want to select the {1}, \ + or `{2}@{3}` if you want to \ + select the {2}", + disambig, value_kind, type_kind, + path_str)) + .emit(); + continue; + } + } path.def } else if let Ok(path) = resolve(true) { - let kind = match path.def { - Def::Variant(..) | Def::VariantCtor(..) => ("variant", format!("{}()", path_str)), - Def::Fn(..) => ("function", format!("{}()", path_str)), - Def::Method(..) => ("method", format!("{}()", path_str)), - Def::Const(..) => ("const", format!("const@{}", path_str)), - Def::Static(..) => ("static", format!("static@{}", path_str)), - _ => ("value", format!("static@{}", path_str)), - }; - let sp = attrs.doc_strings.first().map_or(DUMMY_SP, |a| a.span()); - cx.sess().struct_span_err(sp, &format!("could not resolve `{}` as a type, it is a {}", - path_str, kind.0)) - .help(&format!("try `{}`", kind.1)) - .emit(); - continue; + path.def } else { // this could just be a normal link continue; From 869dd91d443b10e11d20d93beb9c06cb0fd7ec42 Mon Sep 17 00:00:00 2001 From: Manish Goregaokar Date: Fri, 19 Jan 2018 16:45:48 +0530 Subject: [PATCH 37/44] Allow function@, value@, macro@ --- src/librustdoc/clean/mod.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/librustdoc/clean/mod.rs b/src/librustdoc/clean/mod.rs index 5a09197fd4194..1734071f849bc 100644 --- a/src/librustdoc/clean/mod.rs +++ b/src/librustdoc/clean/mod.rs @@ -862,13 +862,17 @@ impl Clean for [ast::Attribute] { kind = PathKind::Type; link.trim_left_matches(prefix) } else if let Some(prefix) = - ["const@", "static@"].iter() - .find(|p| link.starts_with(**p)) { + ["const@", "static@", + "value@", "function@"].iter() + .find(|p| link.starts_with(**p)) { kind = PathKind::Value; link.trim_left_matches(prefix) } else if link.ends_with("()") { kind = PathKind::Value; link.trim_right_matches("()") + } else if link.starts_with("macro@") { + kind = PathKind::Macro; + link.trim_left_matches("macro@") } else if link.ends_with('!') { kind = PathKind::Macro; link.trim_right_matches('!') From 5762fa4b5a22276626414d94b38b0e7886396089 Mon Sep 17 00:00:00 2001 From: Manish Goregaokar Date: Fri, 19 Jan 2018 17:19:01 +0530 Subject: [PATCH 38/44] Allow macros to be resolved with ambiguous idents too --- src/librustdoc/clean/mod.rs | 50 ++++++++++++++++++++++--------------- 1 file changed, 30 insertions(+), 20 deletions(-) diff --git a/src/librustdoc/clean/mod.rs b/src/librustdoc/clean/mod.rs index 1734071f849bc..b42696b098512 100644 --- a/src/librustdoc/clean/mod.rs +++ b/src/librustdoc/clean/mod.rs @@ -904,6 +904,32 @@ impl Clean for [ast::Attribute] { } }; + let macro_resolve = || { + use syntax::ext::base::MacroKind; + use syntax::ext::hygiene::Mark; + let segment = ast::PathSegment { + identifier: ast::Ident::from_str(path_str), + span: DUMMY_SP, + parameters: None, + }; + let path = ast::Path { + span: DUMMY_SP, + segments: vec![segment], + }; + + let mut resolver = cx.resolver.borrow_mut(); + let mark = Mark::root(); + let res = resolver + .resolve_macro_to_def_inner(mark, &path, MacroKind::Bang, false); + if let Ok(def) = res { + Some(def) + } else if let Some(def) = resolver.all_macros.get(&path_str.into()) { + Some(*def) + } else { + None + } + }; + match kind { PathKind::Value => { if let Ok(path) = resolve(true) { @@ -974,34 +1000,18 @@ impl Clean for [ast::Attribute] { path.def } else if let Ok(path) = resolve(true) { path.def + } else if let Some(def) = macro_resolve() { + def } else { // this could just be a normal link continue; } } PathKind::Macro => { - use syntax::ext::base::MacroKind; - use syntax::ext::hygiene::Mark; - let segment = ast::PathSegment { - identifier: ast::Ident::from_str(path_str), - span: DUMMY_SP, - parameters: None, - }; - let path = ast::Path { - span: DUMMY_SP, - segments: vec![segment], - }; - - let mut resolver = cx.resolver.borrow_mut(); - let mark = Mark::root(); - let res = resolver - .resolve_macro_to_def_inner(mark, &path, MacroKind::Bang, false); - if let Ok(def) = res { + if let Some(def) = macro_resolve() { def - } else if let Some(def) = resolver.all_macros.get(&path_str.into()) { - *def } else { - continue; + continue } } } From 6256bff7a355a0dca497d8352080883d437d7765 Mon Sep 17 00:00:00 2001 From: Manish Goregaokar Date: Fri, 19 Jan 2018 17:27:18 +0530 Subject: [PATCH 39/44] Move the figuring out of the 'kind' of def out into functions --- src/librustdoc/clean/mod.rs | 101 +++++++++++++++++++++++------------- 1 file changed, 64 insertions(+), 37 deletions(-) diff --git a/src/librustdoc/clean/mod.rs b/src/librustdoc/clean/mod.rs index b42696b098512..adc2390e93dde 100644 --- a/src/librustdoc/clean/mod.rs +++ b/src/librustdoc/clean/mod.rs @@ -836,6 +836,62 @@ impl AttributesExt for Attributes { } } +/// Given a def, returns its name and disambiguator +/// for a value namespace +/// +/// Returns None for things which cannot be ambiguous since +/// they exist in both namespaces (structs and modules) +fn value_ns_kind(def: Def, path_str: &str) -> Option<(&'static str, String)> { + match def { + // structs and mods exist in both namespaces. skip them + Def::StructCtor(..) | Def::Mod(..) => None, + Def::Variant(..) | Def::VariantCtor(..) + => Some(("variant", format!("{}()", path_str))), + Def::Fn(..) + => Some(("function", format!("{}()", path_str))), + Def::Method(..) + => Some(("method", format!("{}()", path_str))), + Def::Const(..) + => Some(("const", format!("const@{}", path_str))), + Def::Static(..) + => Some(("static", format!("static@{}", path_str))), + _ => Some(("value", format!("value@{}", path_str))), + } +} + +/// Given a def, returns its name, the article to be used, and a disambiguator +/// for the type namespace +fn type_ns_kind(def: Def, path_str: &str) -> (&'static str, &'static str, String) { + let (kind, article) = match def { + // we can still have non-tuple structs + Def::Struct(..) => ("struct", "a"), + Def::Enum(..) => ("enum", "an"), + Def::Trait(..) => ("trait", "a"), + Def::Union(..) => ("union", "a"), + _ => ("type", "a"), + }; + (kind, article, format!("{}@{}", kind, path_str)) +} + +fn ambiguity_error(cx: &DocContext, attrs: &Attributes, + path_str: &str, + article1: &str, kind1: &str, disambig1: &str, + article2: &str, kind2: &str, disambig2: &str) { + let sp = attrs.doc_strings.first() + .map_or(DUMMY_SP, |a| a.span()); + cx.sess() + .struct_span_err(sp, + &format!("`{}` is both {} {} and {} {}", + path_str, article1, kind1, + article2, kind2)) + .help(&format!("try `{0}` if you want to select the {1}, \ + or `{2}@{3}` if you want to \ + select the {2}", + disambig1, kind1, disambig2, + kind2)) + .emit(); +} + enum PathKind { /// can be either value or type, not a macro Unknown, @@ -846,6 +902,7 @@ enum PathKind { /// types, traits, everything in the type namespace Type } + impl Clean for [ast::Attribute] { fn clean(&self, cx: &DocContext) -> Attributes { let mut attrs = Attributes::from_ast(cx.sess().diagnostic(), self); @@ -957,43 +1014,13 @@ impl Clean for [ast::Attribute] { if let Ok(path) = resolve(false) { // if there is something in both namespaces if let Ok(value_path) = resolve(true) { - let kind = match value_path.def { - // structs and mods exist in both namespaces. skip them - Def::StructCtor(..) | Def::Mod(..) => None, - Def::Variant(..) | Def::VariantCtor(..) - => Some(("variant", format!("{}()", path_str))), - Def::Fn(..) - => Some(("function", format!("{}()", path_str))), - Def::Method(..) - => Some(("method", format!("{}()", path_str))), - Def::Const(..) - => Some(("const", format!("const@{}", path_str))), - Def::Static(..) - => Some(("static", format!("static@{}", path_str))), - _ => Some(("value", format!("static@{}", path_str))), - }; - if let Some((value_kind, disambig)) = kind { - let (type_kind, article) = match path.def { - // we can still have non-tuple structs - Def::Struct(..) => ("struct", "a"), - Def::Enum(..) => ("enum", "an"), - Def::Trait(..) => ("trait", "a"), - Def::Union(..) => ("union", "a"), - _ => ("type", "a"), - }; - let sp = attrs.doc_strings.first() - .map_or(DUMMY_SP, |a| a.span()); - cx.sess() - .struct_span_err(sp, - &format!("`{}` is both {} {} and a {}", - path_str, article, type_kind, - value_kind)) - .help(&format!("try `{0}` if you want to select the {1}, \ - or `{2}@{3}` if you want to \ - select the {2}", - disambig, value_kind, type_kind, - path_str)) - .emit(); + let kind = value_ns_kind(value_path.def, path_str); + if let Some((value_kind, value_disambig)) = kind { + let (type_kind, article, type_disambig) + = type_ns_kind(path.def); + ambiguity_error(cx, &attrs, + article, type_kind, type_disambig, + "a", value_kind, value_disambig); continue; } } From fbd2d16c3f9a4ce04547fa30aa60e8cb0362fdc7 Mon Sep 17 00:00:00 2001 From: Manish Goregaokar Date: Fri, 19 Jan 2018 17:43:08 +0530 Subject: [PATCH 40/44] Add ambiguity errors for macros --- src/librustdoc/clean/mod.rs | 54 ++++++++++++++++++++++++------------- 1 file changed, 35 insertions(+), 19 deletions(-) diff --git a/src/librustdoc/clean/mod.rs b/src/librustdoc/clean/mod.rs index adc2390e93dde..4e86827bf2fe3 100644 --- a/src/librustdoc/clean/mod.rs +++ b/src/librustdoc/clean/mod.rs @@ -884,9 +884,9 @@ fn ambiguity_error(cx: &DocContext, attrs: &Attributes, &format!("`{}` is both {} {} and {} {}", path_str, article1, kind1, article2, kind2)) - .help(&format!("try `{0}` if you want to select the {1}, \ - or `{2}@{3}` if you want to \ - select the {2}", + .help(&format!("try `{}` if you want to select the {}, \ + or `{}` if you want to \ + select the {}", disambig1, kind1, disambig2, kind2)) .emit(); @@ -920,8 +920,8 @@ impl Clean for [ast::Attribute] { link.trim_left_matches(prefix) } else if let Some(prefix) = ["const@", "static@", - "value@", "function@"].iter() - .find(|p| link.starts_with(**p)) { + "value@", "function@", "mod@", "fn@", "module@"] + .iter().find(|p| link.starts_with(**p)) { kind = PathKind::Value; link.trim_left_matches(prefix) } else if link.ends_with("()") { @@ -1007,28 +1007,44 @@ impl Clean for [ast::Attribute] { } } PathKind::Unknown => { - // try both! - // It is imperative we search for not-a-value first - // Otherwise we will find struct ctors for when we are looking - // for structs, and the link won't work. - if let Ok(path) = resolve(false) { + // try everything! + if let Some(macro_def) = macro_resolve() { + if let Ok(type_path) = resolve(false) { + let (type_kind, article, type_disambig) + = type_ns_kind(type_path.def, path_str); + ambiguity_error(cx, &attrs, path_str, + article, type_kind, &type_disambig, + "a", "macro", &format!("macro@{}", path_str)); + continue; + } else if let Ok(value_path) = resolve(true) { + let (value_kind, value_disambig) + = value_ns_kind(value_path.def, path_str) + .expect("struct and mod cases should have been \ + caught in previous branch"); + ambiguity_error(cx, &attrs, path_str, + "a", value_kind, &value_disambig, + "a", "macro", &format!("macro@{}", path_str)); + } + macro_def + } else if let Ok(type_path) = resolve(false) { + // It is imperative we search for not-a-value first + // Otherwise we will find struct ctors for when we are looking + // for structs, and the link won't work. // if there is something in both namespaces if let Ok(value_path) = resolve(true) { let kind = value_ns_kind(value_path.def, path_str); if let Some((value_kind, value_disambig)) = kind { let (type_kind, article, type_disambig) - = type_ns_kind(path.def); - ambiguity_error(cx, &attrs, - article, type_kind, type_disambig, - "a", value_kind, value_disambig); + = type_ns_kind(type_path.def, path_str); + ambiguity_error(cx, &attrs, path_str, + article, type_kind, &type_disambig, + "a", value_kind, &value_disambig); continue; } } - path.def - } else if let Ok(path) = resolve(true) { - path.def - } else if let Some(def) = macro_resolve() { - def + type_path.def + } else if let Ok(value_path) = resolve(true) { + value_path.def } else { // this could just be a normal link continue; From 7739bb2d3520fbbd2ae26efc9fd1bf4657a35d9b Mon Sep 17 00:00:00 2001 From: Manish Goregaokar Date: Fri, 19 Jan 2018 17:57:10 +0530 Subject: [PATCH 41/44] Move resolve() into a function --- src/librustdoc/clean/mod.rs | 48 +++++++++++++++++++------------------ 1 file changed, 25 insertions(+), 23 deletions(-) diff --git a/src/librustdoc/clean/mod.rs b/src/librustdoc/clean/mod.rs index 4e86827bf2fe3..66b7cd0cff99e 100644 --- a/src/librustdoc/clean/mod.rs +++ b/src/librustdoc/clean/mod.rs @@ -892,6 +892,24 @@ fn ambiguity_error(cx: &DocContext, attrs: &Attributes, .emit(); } +/// Resolve a given string as a path, along with whether or not it is +/// in the value namespace +fn resolve(cx: &DocContext, path_str: &str, is_val: bool) -> Result { + // In case we're in a module, try to resolve the relative + // path + if let Some(id) = cx.mod_ids.borrow().last() { + cx.resolver.borrow_mut() + .with_scope(*id, |resolver| { + resolver.resolve_str_path_error(DUMMY_SP, + &path_str, is_val) + }) + } else { + // FIXME(Manishearth) this branch doesn't seem to ever be hit, really + cx.resolver.borrow_mut() + .resolve_str_path_error(DUMMY_SP, &path_str, is_val) + } +} + enum PathKind { /// can be either value or type, not a macro Unknown, @@ -945,22 +963,6 @@ impl Clean for [ast::Attribute] { continue; } - let resolve = |is_val| { - // In case we're in a module, try to resolve the relative - // path - if let Some(id) = cx.mod_ids.borrow().last() { - cx.resolver.borrow_mut() - .with_scope(*id, |resolver| { - resolver.resolve_str_path_error(DUMMY_SP, - &path_str, is_val) - }) - } else { - // FIXME(Manishearth) this branch doesn't seem to ever be hit, really - cx.resolver.borrow_mut() - .resolve_str_path_error(DUMMY_SP, &path_str, is_val) - } - }; - let macro_resolve = || { use syntax::ext::base::MacroKind; use syntax::ext::hygiene::Mark; @@ -989,7 +991,7 @@ impl Clean for [ast::Attribute] { match kind { PathKind::Value => { - if let Ok(path) = resolve(true) { + if let Ok(path) = resolve(cx, path_str, true) { path.def } else { // this could just be a normal link or a broken link @@ -999,7 +1001,7 @@ impl Clean for [ast::Attribute] { } } PathKind::Type => { - if let Ok(path) = resolve(false) { + if let Ok(path) = resolve(cx, path_str, false) { path.def } else { // this could just be a normal link @@ -1009,14 +1011,14 @@ impl Clean for [ast::Attribute] { PathKind::Unknown => { // try everything! if let Some(macro_def) = macro_resolve() { - if let Ok(type_path) = resolve(false) { + if let Ok(type_path) = resolve(cx, path_str, false) { let (type_kind, article, type_disambig) = type_ns_kind(type_path.def, path_str); ambiguity_error(cx, &attrs, path_str, article, type_kind, &type_disambig, "a", "macro", &format!("macro@{}", path_str)); continue; - } else if let Ok(value_path) = resolve(true) { + } else if let Ok(value_path) = resolve(cx, path_str, true) { let (value_kind, value_disambig) = value_ns_kind(value_path.def, path_str) .expect("struct and mod cases should have been \ @@ -1026,12 +1028,12 @@ impl Clean for [ast::Attribute] { "a", "macro", &format!("macro@{}", path_str)); } macro_def - } else if let Ok(type_path) = resolve(false) { + } else if let Ok(type_path) = resolve(cx, path_str, false) { // It is imperative we search for not-a-value first // Otherwise we will find struct ctors for when we are looking // for structs, and the link won't work. // if there is something in both namespaces - if let Ok(value_path) = resolve(true) { + if let Ok(value_path) = resolve(cx, path_str, true) { let kind = value_ns_kind(value_path.def, path_str); if let Some((value_kind, value_disambig)) = kind { let (type_kind, article, type_disambig) @@ -1043,7 +1045,7 @@ impl Clean for [ast::Attribute] { } } type_path.def - } else if let Ok(value_path) = resolve(true) { + } else if let Ok(value_path) = resolve(cx, path_str, true) { value_path.def } else { // this could just be a normal link From 4ced272780321c597df740dd904fac59b0bff608 Mon Sep 17 00:00:00 2001 From: Manish Goregaokar Date: Fri, 19 Jan 2018 18:13:50 +0530 Subject: [PATCH 42/44] Move macro_resolve() into a function --- src/librustdoc/clean/mod.rs | 56 +++++++++++++++++++------------------ 1 file changed, 29 insertions(+), 27 deletions(-) diff --git a/src/librustdoc/clean/mod.rs b/src/librustdoc/clean/mod.rs index 66b7cd0cff99e..fb1d21d6527af 100644 --- a/src/librustdoc/clean/mod.rs +++ b/src/librustdoc/clean/mod.rs @@ -910,6 +910,33 @@ fn resolve(cx: &DocContext, path_str: &str, is_val: bool) -> Result Option { + use syntax::ext::base::MacroKind; + use syntax::ext::hygiene::Mark; + let segment = ast::PathSegment { + identifier: ast::Ident::from_str(path_str), + span: DUMMY_SP, + parameters: None, + }; + let path = ast::Path { + span: DUMMY_SP, + segments: vec![segment], + }; + + let mut resolver = cx.resolver.borrow_mut(); + let mark = Mark::root(); + let res = resolver + .resolve_macro_to_def_inner(mark, &path, MacroKind::Bang, false); + if let Ok(def) = res { + Some(def) + } else if let Some(def) = resolver.all_macros.get(&path_str.into()) { + Some(*def) + } else { + None + } +} + enum PathKind { /// can be either value or type, not a macro Unknown, @@ -963,31 +990,6 @@ impl Clean for [ast::Attribute] { continue; } - let macro_resolve = || { - use syntax::ext::base::MacroKind; - use syntax::ext::hygiene::Mark; - let segment = ast::PathSegment { - identifier: ast::Ident::from_str(path_str), - span: DUMMY_SP, - parameters: None, - }; - let path = ast::Path { - span: DUMMY_SP, - segments: vec![segment], - }; - - let mut resolver = cx.resolver.borrow_mut(); - let mark = Mark::root(); - let res = resolver - .resolve_macro_to_def_inner(mark, &path, MacroKind::Bang, false); - if let Ok(def) = res { - Some(def) - } else if let Some(def) = resolver.all_macros.get(&path_str.into()) { - Some(*def) - } else { - None - } - }; match kind { PathKind::Value => { @@ -1010,7 +1012,7 @@ impl Clean for [ast::Attribute] { } PathKind::Unknown => { // try everything! - if let Some(macro_def) = macro_resolve() { + if let Some(macro_def) = macro_resolve(cx, path_str) { if let Ok(type_path) = resolve(cx, path_str, false) { let (type_kind, article, type_disambig) = type_ns_kind(type_path.def, path_str); @@ -1053,7 +1055,7 @@ impl Clean for [ast::Attribute] { } } PathKind::Macro => { - if let Some(def) = macro_resolve() { + if let Some(def) = macro_resolve(cx, path_str) { def } else { continue From dc5475257f02e489f4107b8666dd342a31dff86a Mon Sep 17 00:00:00 2001 From: Manish Goregaokar Date: Mon, 22 Jan 2018 15:29:34 +0530 Subject: [PATCH 43/44] Review fixes --- src/librustc_resolve/lib.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs index c55e23e917a1c..55c7e5f392416 100644 --- a/src/librustc_resolve/lib.rs +++ b/src/librustc_resolve/lib.rs @@ -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; @@ -1413,8 +1414,7 @@ impl<'a> hir::lowering::Resolver for Resolver<'a> { } fn resolve_str_path(&mut self, span: Span, crate_root: Option<&str>, - components: &[&str], is_value: bool) -> hir::Path { - use std::iter; + components: &[&str], is_value: bool) -> hir::Path { let mut path = hir::Path { span, def: Def::Err, From fe93adad2c4f7a9709e44f157d97f2fe623c0236 Mon Sep 17 00:00:00 2001 From: Manish Goregaokar Date: Mon, 22 Jan 2018 16:25:59 +0530 Subject: [PATCH 44/44] Update to new commonmark arg --- src/librustdoc/lib.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/librustdoc/lib.rs b/src/librustdoc/lib.rs index 01c2a5620da25..6347c4a58dddd 100644 --- a/src/librustdoc/lib.rs +++ b/src/librustdoc/lib.rs @@ -503,10 +503,10 @@ where R: 'static + Send, F: 'static + Send + FnOnce(Output) -> R { let crate_name = matches.opt_str("crate-name"); let crate_version = matches.opt_str("crate-version"); let plugin_path = matches.opt_str("plugin-path"); - let render_type = if matches.opt_present("enable-commonmark") { - RenderType::Pulldown - } else { + let render_type = if matches.opt_present("disable-commonmark") { RenderType::Hoedown + } else { + RenderType::Pulldown }; info!("starting to run rustc");