Skip to content

Commit

Permalink
Rollup merge of #90583 - willcrichton:example-analyzer, r=jyn514
Browse files Browse the repository at this point in the history
Fix ICE when rustdoc is scraping examples inside of a proc macro

This PR provides a clearer semantics for how --scrape-examples interacts with macros. If an expression's span AND it's enclosing item's span both are not `from_expansion`, then the example will be scraped. The added test case `rustdoc-scrape-examples-macros` shows a variety of situations.

* A macro-rules macro that takes a function call as input: good
* A macro-rules macro that generates a function call as output: bad
* A proc-macro that generates a function call as output: bad
* An attribute macro that generates a function call as output: bad
* An attribute macro that takes a function call as input: good, if the proc macro is designed to propagate the input spans

I ran this updated rustdoc on pyo3 and confirmed that it successfully scrapes examples from inside a proc macro, eg

<img width="1013" alt="Screen Shot 2021-11-04 at 1 11 28 PM" src="https://user-images.githubusercontent.com/663326/140412691-81a3bb6b-a448-4a1b-a293-f7a795553634.png">

(cc `@mejrs)`

Additionally, this PR fixes an ordering bug in the highlighting logic.

Fixes #90567.

r? `@jyn514`
  • Loading branch information
matthiaskrgr authored Nov 4, 2021
2 parents 7dc54bf + 4846d10 commit ad03584
Show file tree
Hide file tree
Showing 9 changed files with 141 additions and 13 deletions.
8 changes: 7 additions & 1 deletion src/librustdoc/html/highlight.rs
Original file line number Diff line number Diff line change
Expand Up @@ -270,12 +270,18 @@ struct Decorations {

impl Decorations {
fn new(info: DecorationInfo) -> Self {
let (starts, ends) = info
// Extract tuples (start, end, kind) into separate sequences of (start, kind) and (end).
let (mut starts, mut ends): (Vec<_>, Vec<_>) = info
.0
.into_iter()
.map(|(kind, ranges)| ranges.into_iter().map(move |(lo, hi)| ((lo, kind), hi)))
.flatten()
.unzip();

// Sort the sequences in document order.
starts.sort_by_key(|(lo, _)| *lo);
ends.sort();

Decorations { starts, ends }
}
}
Expand Down
36 changes: 27 additions & 9 deletions src/librustdoc/scrape_examples.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ use rustc_data_structures::fx::FxHashMap;
use rustc_hir::{
self as hir,
intravisit::{self, Visitor},
HirId,
};
use rustc_interface::interface;
use rustc_macros::{Decodable, Encodable};
Expand Down Expand Up @@ -83,15 +82,10 @@ crate struct CallLocation {

impl CallLocation {
fn new(
tcx: TyCtxt<'_>,
expr_span: rustc_span::Span,
expr_id: HirId,
enclosing_item_span: rustc_span::Span,
source_file: &SourceFile,
) -> Self {
let enclosing_item_span =
tcx.hir().span_with_body(tcx.hir().get_parent_item(expr_id)).source_callsite();
assert!(enclosing_item_span.contains(expr_span));

CallLocation {
call_expr: SyntaxRange::new(expr_span, source_file),
enclosing_item: SyntaxRange::new(enclosing_item_span, source_file),
Expand Down Expand Up @@ -168,13 +162,29 @@ where
// If this span comes from a macro expansion, then the source code may not actually show
// a use of the given item, so it would be a poor example. Hence, we skip all uses in macros.
if span.from_expansion() {
trace!("Rejecting expr from macro: {:?}", span);
return;
}

// If the enclosing item has a span coming from a proc macro, then we also don't want to include
// the example.
let enclosing_item_span = tcx.hir().span_with_body(tcx.hir().get_parent_item(ex.hir_id));
if enclosing_item_span.from_expansion() {
trace!("Rejecting expr ({:?}) from macro item: {:?}", span, enclosing_item_span);
return;
}

assert!(
enclosing_item_span.contains(span),
"Attempted to scrape call at [{:?}] whose enclosing item [{:?}] doesn't contain the span of the call.",
span,
enclosing_item_span
);

// Save call site if the function resolves to a concrete definition
if let ty::FnDef(def_id, _) = ty.kind() {
// Ignore functions not from the crate being documented
if self.target_crates.iter().all(|krate| *krate != def_id.krate) {
trace!("Rejecting expr from crate not being documented: {:?}", span);
return;
}

Expand All @@ -198,7 +208,8 @@ where
let fn_key = tcx.def_path_hash(*def_id);
let fn_entries = self.calls.entry(fn_key).or_default();

let location = CallLocation::new(tcx, span, ex.hir_id, &file);
trace!("Including expr: {:?}", span);
let location = CallLocation::new(span, enclosing_item_span, &file);
fn_entries.entry(abs_path).or_insert_with(mk_call_data).locations.push(location);
}
}
Expand Down Expand Up @@ -240,6 +251,13 @@ crate fn run(
let mut finder = FindCalls { calls: &mut calls, tcx, map: tcx.hir(), cx, target_crates };
tcx.hir().visit_all_item_likes(&mut finder.as_deep_visitor());

// Sort call locations within a given file in document order
for fn_calls in calls.values_mut() {
for file_calls in fn_calls.values_mut() {
file_calls.locations.sort_by_key(|loc| loc.call_expr.byte_span.0);
}
}

// Save output to provided path
let mut encoder = FileEncoder::new(options.output_path).map_err(|e| e.to_string())?;
calls.encode(&mut encoder).map_err(|e| e.to_string())?;
Expand Down
17 changes: 17 additions & 0 deletions src/test/run-make/rustdoc-scrape-examples-macros/Makefile
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
-include ../../run-make-fulldeps/tools.mk

OUTPUT_DIR := "$(TMPDIR)/rustdoc"

all:
$(RUSTC) src/proc.rs --crate-name foobar_macro --edition=2021 --crate-type proc-macro --emit=dep-info,link

$(RUSTC) src/lib.rs --crate-name foobar --edition=2021 --crate-type lib --emit=dep-info,link

$(RUSTDOC) examples/ex.rs --crate-name ex --crate-type bin --output $(OUTPUT_DIR) \
--extern foobar=$(TMPDIR)/libfoobar.rlib --extern foobar_macro=$(TMPDIR)/libfoobar_macro.so \
-Z unstable-options --scrape-examples-output-path $(TMPDIR)/ex.calls --scrape-examples-target-crate foobar

$(RUSTDOC) src/lib.rs --crate-name foobar --crate-type lib --output $(OUTPUT_DIR) \
-Z unstable-options --with-examples $(TMPDIR)/ex.calls

$(HTMLDOCCK) $(OUTPUT_DIR) src/lib.rs
27 changes: 27 additions & 0 deletions src/test/run-make/rustdoc-scrape-examples-macros/examples/ex.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
extern crate foobar;
extern crate foobar_macro;

use foobar::*;
use foobar_macro::*;

a_proc_macro!(); // no

#[an_attr_macro]
fn a() {
f(); // no
}

#[an_attr_macro(with_span)]
fn b() {
f(); // yes
}

fn c() {
a_rules_macro!(f()); // yes
}

fn d() {
a_rules_macro!(()); // no
}

fn main(){}
12 changes: 12 additions & 0 deletions src/test/run-make/rustdoc-scrape-examples-macros/src/lib.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
// Scraped example should only include line numbers for items b and c in ex.rs
// @!has foobar/fn.f.html '//*[@class="line-numbers"]' '14'
// @has foobar/fn.f.html '//*[@class="line-numbers"]' '15'
// @has foobar/fn.f.html '//*[@class="line-numbers"]' '21'
// @!has foobar/fn.f.html '//*[@class="line-numbers"]' '22'

pub fn f() {}

#[macro_export]
macro_rules! a_rules_macro {
($e:expr) => { ($e, foobar::f()); }
}
39 changes: 39 additions & 0 deletions src/test/run-make/rustdoc-scrape-examples-macros/src/proc.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
extern crate proc_macro;
use proc_macro::*;

#[proc_macro]
pub fn a_proc_macro(_item: TokenStream) -> TokenStream {
"fn ex() { foobar::f(); }".parse().unwrap()
}

// inserts foobar::f() to the end of the function
#[proc_macro_attribute]
pub fn an_attr_macro(attr: TokenStream, item: TokenStream) -> TokenStream {
let new_call: TokenStream = "foobar::f();".parse().unwrap();

let mut tokens = item.into_iter();

let fn_tok = tokens.next().unwrap();
let ident_tok = tokens.next().unwrap();
let args_tok = tokens.next().unwrap();
let body = match tokens.next().unwrap() {
TokenTree::Group(g) => {
let new_g = Group::new(g.delimiter(), new_call);
let mut outer_g = Group::new(
g.delimiter(),
[TokenTree::Group(g.clone()), TokenTree::Group(new_g)].into_iter().collect(),
);

if attr.to_string() == "with_span" {
outer_g.set_span(g.span());
}

TokenTree::Group(outer_g)
}
_ => unreachable!(),
};

let tokens = vec![fn_tok, ident_tok, args_tok, body].into_iter().collect::<TokenStream>();

tokens
}
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
fn main() {
foobar::ok();
foobar::ok(0);

// this is a

// ..

// BIG

// item
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
fn main() {
foobar::ok();
foobar::ok(1);
// small item
}

fn f() {
foobar::ok(2);
}
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
// @has foobar/fn.ok.html '//*[@class="docblock scraped-example-list"]' 'ex2'
// @has foobar/fn.ok.html '//*[@class="more-scraped-examples"]' 'ex1'
// @has foobar/fn.ok.html '//*[@class="highlight focus"]' '1'
// @has foobar/fn.ok.html '//*[@class="highlight"]' '2'
// @has foobar/fn.ok.html '//*[@class="highlight focus"]' '0'

pub fn ok() {}
pub fn ok(_x: i32) {}

0 comments on commit ad03584

Please sign in to comment.