Skip to content

Commit e1cce06

Browse files
borsbors
committed
Auto merge of #77700 - bugadani:rustdoc-link-cache, r=jyn514
Rustdoc: Cache resolved links in current module A step towards #77681
2 parents 5d77fc8 + fa64c27 commit e1cce06

File tree

2 files changed

+134
-47
lines changed

2 files changed

+134
-47
lines changed

‎src/librustdoc/passes/collect_intra_doc_links.rs

+120-47
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
//! [RFC 1946]: https://github.com/rust-lang/rfcs/blob/master/text/1946-intra-rustdoc-links.md
44
55
use rustc_ast as ast;
6-
use rustc_data_structures::stable_set::FxHashSet;
6+
use rustc_data_structures::{fx::FxHashMap, stable_set::FxHashSet};
77
use rustc_errors::{Applicability, DiagnosticBuilder};
88
use rustc_expand::base::SyntaxExtensionKind;
99
use rustc_hir as hir;
@@ -168,6 +168,27 @@ enum AnchorFailure {
168168
RustdocAnchorConflict(Res),
169169
}
170170

171+
#[derive(Clone, Debug, Hash, PartialEq, Eq)]
172+
struct ResolutionInfo {
173+
module_id: DefId,
174+
dis: Option<Disambiguator>,
175+
path_str: String,
176+
extra_fragment: Option<String>,
177+
}
178+
179+
struct DiagnosticInfo<'a> {
180+
item: &'a Item,
181+
dox: &'a str,
182+
ori_link: &'a str,
183+
link_range: Option<Range<usize>>,
184+
}
185+
186+
#[derive(Clone, Debug, Hash)]
187+
struct CachedLink {
188+
pub res: (Res, Option<String>),
189+
pub side_channel: Option<(DefKind, DefId)>,
190+
}
191+
171192
struct LinkCollector<'a, 'tcx> {
172193
cx: &'a DocContext<'tcx>,
173194
/// A stack of modules used to decide what scope to resolve in.
@@ -179,11 +200,18 @@ struct LinkCollector<'a, 'tcx> {
179200
/// because `clean` and the disambiguator code expect them to be different.
180201
/// See the code for associated items on inherent impls for details.
181202
kind_side_channel: Cell<Option<(DefKind, DefId)>>,
203+
/// Cache the resolved links so we can avoid resolving (and emitting errors for) the same link
204+
visited_links: FxHashMap<ResolutionInfo, CachedLink>,
182205
}
183206

184207
impl<'a, 'tcx> LinkCollector<'a, 'tcx> {
185208
fn new(cx: &'a DocContext<'tcx>) -> Self {
186-
LinkCollector { cx, mod_ids: Vec::new(), kind_side_channel: Cell::new(None) }
209+
LinkCollector {
210+
cx,
211+
mod_ids: Vec::new(),
212+
kind_side_channel: Cell::new(None),
213+
visited_links: FxHashMap::default(),
214+
}
187215
}
188216

189217
/// Given a full link, parse it as an [enum struct variant].
@@ -937,7 +965,7 @@ impl LinkCollector<'_, '_> {
937965
///
938966
/// FIXME(jynelson): this is way too many arguments
939967
fn resolve_link(
940-
&self,
968+
&mut self,
941969
item: &Item,
942970
dox: &str,
943971
self_name: &Option<String>,
@@ -962,6 +990,7 @@ impl LinkCollector<'_, '_> {
962990
let link = ori_link.replace("`", "");
963991
let parts = link.split('#').collect::<Vec<_>>();
964992
let (link, extra_fragment) = if parts.len() > 2 {
993+
// A valid link can't have multiple #'s
965994
anchor_failure(cx, &item, &link, dox, link_range, AnchorFailure::MultipleAnchors);
966995
return None;
967996
} else if parts.len() == 2 {
@@ -1075,16 +1104,15 @@ impl LinkCollector<'_, '_> {
10751104
return None;
10761105
}
10771106

1078-
let (mut res, mut fragment) = self.resolve_with_disambiguator(
1079-
disambiguator,
1080-
item,
1081-
dox,
1082-
path_str,
1107+
let key = ResolutionInfo {
10831108
module_id,
1109+
dis: disambiguator,
1110+
path_str: path_str.to_owned(),
10841111
extra_fragment,
1085-
&ori_link,
1086-
link_range.clone(),
1087-
)?;
1112+
};
1113+
let diag =
1114+
DiagnosticInfo { item, dox, ori_link: &ori_link, link_range: link_range.clone() };
1115+
let (mut res, mut fragment) = self.resolve_with_disambiguator_cached(key, diag)?;
10881116

10891117
// Check for a primitive which might conflict with a module
10901118
// Report the ambiguity and require that the user specify which one they meant.
@@ -1192,22 +1220,49 @@ impl LinkCollector<'_, '_> {
11921220
}
11931221
}
11941222

1223+
fn resolve_with_disambiguator_cached(
1224+
&mut self,
1225+
key: ResolutionInfo,
1226+
diag: DiagnosticInfo<'_>,
1227+
) -> Option<(Res, Option<String>)> {
1228+
// Try to look up both the result and the corresponding side channel value
1229+
if let Some(ref cached) = self.visited_links.get(&key) {
1230+
self.kind_side_channel.set(cached.side_channel.clone());
1231+
return Some(cached.res.clone());
1232+
}
1233+
1234+
let res = self.resolve_with_disambiguator(&key, diag);
1235+
1236+
// Cache only if resolved successfully - don't silence duplicate errors
1237+
if let Some(res) = &res {
1238+
// Store result for the actual namespace
1239+
self.visited_links.insert(
1240+
key,
1241+
CachedLink {
1242+
res: res.clone(),
1243+
side_channel: self.kind_side_channel.clone().into_inner(),
1244+
},
1245+
);
1246+
}
1247+
1248+
res
1249+
}
1250+
11951251
/// After parsing the disambiguator, resolve the main part of the link.
11961252
// FIXME(jynelson): wow this is just so much
11971253
fn resolve_with_disambiguator(
11981254
&self,
1199-
disambiguator: Option<Disambiguator>,
1200-
item: &Item,
1201-
dox: &str,
1202-
path_str: &str,
1203-
base_node: DefId,
1204-
extra_fragment: Option<String>,
1205-
ori_link: &str,
1206-
link_range: Option<Range<usize>>,
1255+
key: &ResolutionInfo,
1256+
diag: DiagnosticInfo<'_>,
12071257
) -> Option<(Res, Option<String>)> {
1258+
let disambiguator = key.dis;
1259+
let path_str = &key.path_str;
1260+
let base_node = key.module_id;
1261+
let extra_fragment = &key.extra_fragment;
1262+
12081263
match disambiguator.map(Disambiguator::ns) {
12091264
Some(ns @ (ValueNS | TypeNS)) => {
1210-
match self.resolve(path_str, ns, base_node, &extra_fragment) {
1265+
match self.resolve(path_str, ns, base_node, extra_fragment) {
12111266
Ok(res) => Some(res),
12121267
Err(ErrorKind::Resolve(box mut kind)) => {
12131268
// We only looked in one namespace. Try to give a better error if possible.
@@ -1216,24 +1271,21 @@ impl LinkCollector<'_, '_> {
12161271
// FIXME: really it should be `resolution_failure` that does this, not `resolve_with_disambiguator`
12171272
// See https://github.com/rust-lang/rust/pull/76955#discussion_r493953382 for a good approach
12181273
for &new_ns in &[other_ns, MacroNS] {
1219-
if let Some(res) = self.check_full_res(
1220-
new_ns,
1221-
path_str,
1222-
base_node,
1223-
&extra_fragment,
1224-
) {
1274+
if let Some(res) =
1275+
self.check_full_res(new_ns, path_str, base_node, extra_fragment)
1276+
{
12251277
kind = ResolutionFailure::WrongNamespace(res, ns);
12261278
break;
12271279
}
12281280
}
12291281
}
12301282
resolution_failure(
12311283
self,
1232-
&item,
1284+
diag.item,
12331285
path_str,
12341286
disambiguator,
1235-
dox,
1236-
link_range,
1287+
diag.dox,
1288+
diag.link_range,
12371289
smallvec![kind],
12381290
);
12391291
// This could just be a normal link or a broken link
@@ -1242,7 +1294,14 @@ impl LinkCollector<'_, '_> {
12421294
return None;
12431295
}
12441296
Err(ErrorKind::AnchorFailure(msg)) => {
1245-
anchor_failure(self.cx, &item, &ori_link, dox, link_range, msg);
1297+
anchor_failure(
1298+
self.cx,
1299+
diag.item,
1300+
diag.ori_link,
1301+
diag.dox,
1302+
diag.link_range,
1303+
msg,
1304+
);
12461305
return None;
12471306
}
12481307
}
@@ -1253,21 +1312,35 @@ impl LinkCollector<'_, '_> {
12531312
macro_ns: self
12541313
.resolve_macro(path_str, base_node)
12551314
.map(|res| (res, extra_fragment.clone())),
1256-
type_ns: match self.resolve(path_str, TypeNS, base_node, &extra_fragment) {
1315+
type_ns: match self.resolve(path_str, TypeNS, base_node, extra_fragment) {
12571316
Ok(res) => {
12581317
debug!("got res in TypeNS: {:?}", res);
12591318
Ok(res)
12601319
}
12611320
Err(ErrorKind::AnchorFailure(msg)) => {
1262-
anchor_failure(self.cx, &item, ori_link, dox, link_range, msg);
1321+
anchor_failure(
1322+
self.cx,
1323+
diag.item,
1324+
diag.ori_link,
1325+
diag.dox,
1326+
diag.link_range,
1327+
msg,
1328+
);
12631329
return None;
12641330
}
12651331
Err(ErrorKind::Resolve(box kind)) => Err(kind),
12661332
},
1267-
value_ns: match self.resolve(path_str, ValueNS, base_node, &extra_fragment) {
1333+
value_ns: match self.resolve(path_str, ValueNS, base_node, extra_fragment) {
12681334
Ok(res) => Ok(res),
12691335
Err(ErrorKind::AnchorFailure(msg)) => {
1270-
anchor_failure(self.cx, &item, ori_link, dox, link_range, msg);
1336+
anchor_failure(
1337+
self.cx,
1338+
diag.item,
1339+
diag.ori_link,
1340+
diag.dox,
1341+
diag.link_range,
1342+
msg,
1343+
);
12711344
return None;
12721345
}
12731346
Err(ErrorKind::Resolve(box kind)) => Err(kind),
@@ -1278,7 +1351,7 @@ impl LinkCollector<'_, '_> {
12781351
Res::Def(DefKind::Ctor(..), _) | Res::SelfCtor(..) => {
12791352
Err(ResolutionFailure::WrongNamespace(res, TypeNS))
12801353
}
1281-
_ => match (fragment, extra_fragment) {
1354+
_ => match (fragment, extra_fragment.clone()) {
12821355
(Some(fragment), Some(_)) => {
12831356
// Shouldn't happen but who knows?
12841357
Ok((res, Some(fragment)))
@@ -1294,11 +1367,11 @@ impl LinkCollector<'_, '_> {
12941367
if len == 0 {
12951368
resolution_failure(
12961369
self,
1297-
&item,
1370+
diag.item,
12981371
path_str,
12991372
disambiguator,
1300-
dox,
1301-
link_range,
1373+
diag.dox,
1374+
diag.link_range,
13021375
candidates.into_iter().filter_map(|res| res.err()).collect(),
13031376
);
13041377
// this could just be a normal link
@@ -1317,35 +1390,35 @@ impl LinkCollector<'_, '_> {
13171390
let candidates = candidates.map(|candidate| candidate.ok().map(|(res, _)| res));
13181391
ambiguity_error(
13191392
self.cx,
1320-
&item,
1393+
diag.item,
13211394
path_str,
1322-
dox,
1323-
link_range,
1395+
diag.dox,
1396+
diag.link_range,
13241397
candidates.present_items().collect(),
13251398
);
13261399
return None;
13271400
}
13281401
}
13291402
Some(MacroNS) => {
13301403
match self.resolve_macro(path_str, base_node) {
1331-
Ok(res) => Some((res, extra_fragment)),
1404+
Ok(res) => Some((res, extra_fragment.clone())),
13321405
Err(mut kind) => {
13331406
// `resolve_macro` only looks in the macro namespace. Try to give a better error if possible.
13341407
for &ns in &[TypeNS, ValueNS] {
13351408
if let Some(res) =
1336-
self.check_full_res(ns, path_str, base_node, &extra_fragment)
1409+
self.check_full_res(ns, path_str, base_node, extra_fragment)
13371410
{
13381411
kind = ResolutionFailure::WrongNamespace(res, MacroNS);
13391412
break;
13401413
}
13411414
}
13421415
resolution_failure(
13431416
self,
1344-
&item,
1417+
diag.item,
13451418
path_str,
13461419
disambiguator,
1347-
dox,
1348-
link_range,
1420+
diag.dox,
1421+
diag.link_range,
13491422
smallvec![kind],
13501423
);
13511424
return None;
@@ -1356,7 +1429,7 @@ impl LinkCollector<'_, '_> {
13561429
}
13571430
}
13581431

1359-
#[derive(Copy, Clone, Debug, PartialEq, Eq)]
1432+
#[derive(Copy, Clone, Debug, PartialEq, Eq, Hash)]
13601433
/// Disambiguators for a link.
13611434
enum Disambiguator {
13621435
/// `prim@`

‎src/test/rustdoc/intra-link-self-cache.rs

+14
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
#![crate_name = "foo"]
2+
// @has foo/enum.E1.html '//a/@href' '../foo/enum.E1.html#variant.A'
3+
4+
/// [Self::A::b]
5+
pub enum E1 {
6+
A { b: usize }
7+
}
8+
9+
// @has foo/enum.E2.html '//a/@href' '../foo/enum.E2.html#variant.A'
10+
11+
/// [Self::A::b]
12+
pub enum E2 {
13+
A { b: usize }
14+
}

0 commit comments

Comments
 (0)