Skip to content

Commit 355e6ed

Browse files
GuillaumeGomezGuillaumeGomez
committed
Fix invalid handling of generics
1 parent f2707fe commit 355e6ed

File tree

7 files changed

+147
-93
lines changed

7 files changed

+147
-93
lines changed

‎src/librustdoc/formats/item_type.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@ crate enum ItemType {
4848
ProcAttribute = 23,
4949
ProcDerive = 24,
5050
TraitAlias = 25,
51+
Generic = 26,
5152
}
5253

5354
impl Serialize for ItemType {
@@ -173,6 +174,7 @@ impl ItemType {
173174
ItemType::ProcAttribute => "attr",
174175
ItemType::ProcDerive => "derive",
175176
ItemType::TraitAlias => "traitalias",
177+
ItemType::Generic => "generic",
176178
}
177179
}
178180
}

‎src/librustdoc/html/render/cache.rs

Lines changed: 95 additions & 75 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
use std::collections::hash_map::Entry;
22
use std::collections::BTreeMap;
33

4-
use rustc_data_structures::fx::{FxHashMap, FxHashSet};
4+
use rustc_data_structures::fx::FxHashMap;
55
use rustc_middle::ty::TyCtxt;
66
use rustc_span::symbol::Symbol;
77
use serde::ser::{Serialize, SerializeStruct, Serializer};
@@ -192,32 +192,24 @@ crate fn get_index_search_type<'tcx>(
192192
item: &clean::Item,
193193
tcx: TyCtxt<'tcx>,
194194
) -> Option<IndexItemFunctionType> {
195-
let (all_types, ret_types) = match *item.kind {
195+
let (mut inputs, mut output) = match *item.kind {
196196
clean::FunctionItem(ref f) => get_all_types(&f.generics, &f.decl, tcx),
197197
clean::MethodItem(ref m, _) => get_all_types(&m.generics, &m.decl, tcx),
198198
clean::TyMethodItem(ref m) => get_all_types(&m.generics, &m.decl, tcx),
199199
_ => return None,
200200
};
201201

202-
let inputs = all_types
203-
.iter()
204-
.map(|(ty, kind)| TypeWithKind::from((get_index_type(ty), *kind)))
205-
.filter(|a| a.ty.name.is_some())
206-
.collect();
207-
let output = ret_types
208-
.iter()
209-
.map(|(ty, kind)| TypeWithKind::from((get_index_type(ty), *kind)))
210-
.filter(|a| a.ty.name.is_some())
211-
.collect::<Vec<_>>();
202+
inputs.retain(|a| a.ty.name.is_some());
203+
output.retain(|a| a.ty.name.is_some());
212204
let output = if output.is_empty() { None } else { Some(output) };
213205

214206
Some(IndexItemFunctionType { inputs, output })
215207
}
216208

217-
fn get_index_type(clean_type: &clean::Type) -> RenderType {
209+
fn get_index_type(clean_type: &clean::Type, generics: Vec<TypeWithKind>) -> RenderType {
218210
RenderType {
219211
name: get_index_type_name(clean_type, true).map(|s| s.as_str().to_ascii_lowercase()),
220-
generics: get_generics(clean_type),
212+
generics: if generics.is_empty() { None } else { Some(generics) },
221213
}
222214
}
223215

@@ -246,23 +238,6 @@ fn get_index_type_name(clean_type: &clean::Type, accept_generic: bool) -> Option
246238
}
247239
}
248240

249-
/// Return a list of generic parameters for use in the search index.
250-
///
251-
/// This function replaces bounds with types, so that `T where T: Debug` just becomes `Debug`.
252-
/// It does return duplicates, and that's intentional, since search queries like `Result<usize, usize>`
253-
/// are supposed to match only results where both parameters are `usize`.
254-
fn get_generics(clean_type: &clean::Type) -> Option<Vec<String>> {
255-
clean_type.generics().and_then(|types| {
256-
let r = types
257-
.iter()
258-
.filter_map(|t| {
259-
get_index_type_name(t, false).map(|name| name.as_str().to_ascii_lowercase())
260-
})
261-
.collect::<Vec<_>>();
262-
if r.is_empty() { None } else { Some(r) }
263-
})
264-
}
265-
266241
/// The point of this function is to replace bounds with types.
267242
///
268243
/// i.e. `[T, U]` when you have the following bounds: `T: Display, U: Option<T>` will return
@@ -272,27 +247,77 @@ crate fn get_real_types<'tcx>(
272247
generics: &Generics,
273248
arg: &Type,
274249
tcx: TyCtxt<'tcx>,
275-
recurse: i32,
276-
res: &mut FxHashSet<(Type, ItemType)>,
277-
) -> usize {
278-
fn insert(res: &mut FxHashSet<(Type, ItemType)>, tcx: TyCtxt<'_>, ty: Type) -> usize {
279-
if let Some(kind) = ty.def_id_no_primitives().map(|did| tcx.def_kind(did).into()) {
280-
res.insert((ty, kind));
281-
1
250+
recurse: usize,
251+
res: &mut Vec<TypeWithKind>,
252+
) {
253+
fn insert_ty(
254+
res: &mut Vec<TypeWithKind>,
255+
tcx: TyCtxt<'_>,
256+
ty: Type,
257+
mut generics: Vec<TypeWithKind>,
258+
) {
259+
let is_full_generic = ty.is_full_generic();
260+
261+
if is_full_generic && generics.len() == 1 {
262+
// In this case, no need to go through an intermediate state if the generics
263+
// contains only one element.
264+
//
265+
// For example:
266+
//
267+
// fn foo<T: Display>(r: Option<T>) {}
268+
//
269+
// In this case, it would contain:
270+
//
271+
// ```
272+
// [{
273+
// name: "option",
274+
// generics: [{
275+
// name: "",
276+
// generics: [
277+
// name: "Display",
278+
// generics: []
279+
// }]
280+
// }]
281+
// }]
282+
// ```
283+
//
284+
// After removing the intermediate (unnecessary) full generic, it'll become:
285+
//
286+
// ```
287+
// [{
288+
// name: "option",
289+
// generics: [{
290+
// name: "Display",
291+
// generics: []
292+
// }]
293+
// }]
294+
// ```
295+
//
296+
// To be noted that it can work if there is ONLY ONE generic, otherwise we still
297+
// need to keep it as is!
298+
res.push(generics.pop().unwrap());
299+
return;
300+
}
301+
let mut index_ty = get_index_type(&ty, generics);
302+
if index_ty.name.as_ref().map(|s| s.is_empty()).unwrap_or(true) {
303+
return;
304+
}
305+
if is_full_generic {
306+
// We remove the name of the full generic because we have no use for it.
307+
index_ty.name = Some(String::new());
308+
res.push(TypeWithKind::from((index_ty, ItemType::Generic)));
309+
} else if let Some(kind) = ty.def_id_no_primitives().map(|did| tcx.def_kind(did).into()) {
310+
res.push(TypeWithKind::from((index_ty, kind)));
282311
} else if ty.is_primitive() {
283312
// This is a primitive, let's store it as such.
284-
res.insert((ty, ItemType::Primitive));
285-
1
286-
} else {
287-
0
313+
res.push(TypeWithKind::from((index_ty, ItemType::Primitive)));
288314
}
289315
}
290316

291317
if recurse >= 10 {
292318
// FIXME: remove this whole recurse thing when the recursion bug is fixed
293-
return 0;
319+
return;
294320
}
295-
let mut nb_added = 0;
296321

297322
if let Type::Generic(arg_s) = *arg {
298323
if let Some(where_pred) = generics.where_predicates.iter().find(|g| match g {
@@ -301,6 +326,7 @@ crate fn get_real_types<'tcx>(
301326
}
302327
_ => false,
303328
}) {
329+
let mut ty_generics = Vec::new();
304330
let bounds = where_pred.get_bounds().unwrap_or_else(|| &[]);
305331
for bound in bounds.iter() {
306332
if let GenericBound::TraitBound(poly_trait, _) = bound {
@@ -309,41 +335,32 @@ crate fn get_real_types<'tcx>(
309335
continue;
310336
}
311337
if let Some(ty) = x.get_type() {
312-
let adds = get_real_types(generics, &ty, tcx, recurse + 1, res);
313-
nb_added += adds;
314-
if adds == 0 && !ty.is_full_generic() {
315-
nb_added += insert(res, tcx, ty);
316-
}
338+
get_real_types(generics, &ty, tcx, recurse + 1, &mut ty_generics);
317339
}
318340
}
319341
}
320342
}
343+
insert_ty(res, tcx, arg.clone(), ty_generics);
321344
}
322345
if let Some(bound) = generics.params.iter().find(|g| g.is_type() && g.name == arg_s) {
346+
let mut ty_generics = Vec::new();
323347
for bound in bound.get_bounds().unwrap_or(&[]) {
324348
if let Some(path) = bound.get_trait_path() {
325349
let ty = Type::ResolvedPath { did: path.def_id(), path };
326-
let adds = get_real_types(generics, &ty, tcx, recurse + 1, res);
327-
nb_added += adds;
328-
if adds == 0 && !ty.is_full_generic() {
329-
nb_added += insert(res, tcx, ty);
330-
}
350+
get_real_types(generics, &ty, tcx, recurse + 1, &mut ty_generics);
331351
}
332352
}
353+
insert_ty(res, tcx, arg.clone(), ty_generics);
333354
}
334355
} else {
335-
nb_added += insert(res, tcx, arg.clone());
336-
if let Some(gens) = arg.generics() {
337-
for gen in gens.iter() {
338-
if gen.is_full_generic() {
339-
nb_added += get_real_types(generics, gen, tcx, recurse + 1, res);
340-
} else {
341-
nb_added += insert(res, tcx, (*gen).clone());
342-
}
356+
let mut ty_generics = Vec::new();
357+
if let Some(arg_generics) = arg.generics() {
358+
for gen in arg_generics.iter() {
359+
get_real_types(generics, gen, tcx, recurse + 1, &mut ty_generics);
343360
}
344361
}
362+
insert_ty(res, tcx, arg.clone(), ty_generics);
345363
}
346-
nb_added
347364
}
348365

349366
/// Return the full list of types when bounds have been resolved.
@@ -354,38 +371,41 @@ crate fn get_all_types<'tcx>(
354371
generics: &Generics,
355372
decl: &FnDecl,
356373
tcx: TyCtxt<'tcx>,
357-
) -> (Vec<(Type, ItemType)>, Vec<(Type, ItemType)>) {
358-
let mut all_types = FxHashSet::default();
374+
) -> (Vec<TypeWithKind>, Vec<TypeWithKind>) {
375+
let mut all_types = Vec::new();
359376
for arg in decl.inputs.values.iter() {
360377
if arg.type_.is_self_type() {
361378
continue;
362379
}
363-
let mut args = FxHashSet::default();
380+
// FIXME: performance wise, it'd be much better to move `args` declaration outside of the
381+
// loop and replace this line with `args.clear()`.
382+
let mut args = Vec::new();
364383
get_real_types(generics, &arg.type_, tcx, 0, &mut args);
365384
if !args.is_empty() {
385+
// FIXME: once back to performance improvements, replace this line with:
386+
// `all_types.extend(args.drain(..));`.
366387
all_types.extend(args);
367388
} else {
368389
if let Some(kind) = arg.type_.def_id_no_primitives().map(|did| tcx.def_kind(did).into())
369390
{
370-
all_types.insert((arg.type_.clone(), kind));
391+
all_types.push(TypeWithKind::from((get_index_type(&arg.type_, vec![]), kind)));
371392
}
372393
}
373394
}
374395

375-
let ret_types = match decl.output {
396+
let mut ret_types = Vec::new();
397+
match decl.output {
376398
FnRetTy::Return(ref return_type) => {
377-
let mut ret = FxHashSet::default();
378-
get_real_types(generics, return_type, tcx, 0, &mut ret);
379-
if ret.is_empty() {
399+
get_real_types(generics, return_type, tcx, 0, &mut ret_types);
400+
if ret_types.is_empty() {
380401
if let Some(kind) =
381402
return_type.def_id_no_primitives().map(|did| tcx.def_kind(did).into())
382403
{
383-
ret.insert((return_type.clone(), kind));
404+
ret_types.push(TypeWithKind::from((get_index_type(return_type, vec![]), kind)));
384405
}
385406
}
386-
ret.into_iter().collect()
387407
}
388-
_ => Vec::new(),
408+
_ => {}
389409
};
390-
(all_types.into_iter().collect(), ret_types)
410+
(all_types, ret_types)
391411
}

‎src/librustdoc/html/render/mod.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,7 @@ crate struct IndexItem {
108108
#[derive(Debug)]
109109
crate struct RenderType {
110110
name: Option<String>,
111-
generics: Option<Vec<String>>,
111+
generics: Option<Vec<TypeWithKind>>,
112112
}
113113

114114
/// Full type of functions/methods in the search index.
@@ -2387,6 +2387,7 @@ fn item_ty_to_strs(ty: ItemType) -> (&'static str, &'static str) {
23872387
ItemType::ProcAttribute => ("attributes", "Attribute Macros"),
23882388
ItemType::ProcDerive => ("derives", "Derive Macros"),
23892389
ItemType::TraitAlias => ("trait-aliases", "Trait aliases"),
2390+
ItemType::Generic => unreachable!(),
23902391
}
23912392
}
23922393

‎src/librustdoc/html/static/js/search.js

Lines changed: 24 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -299,10 +299,10 @@ window.initSearch = function(rawSearchIndex) {
299299
var elems = Object.create(null);
300300
var elength = obj[GENERICS_DATA].length;
301301
for (var x = 0; x < elength; ++x) {
302-
if (!elems[obj[GENERICS_DATA][x]]) {
303-
elems[obj[GENERICS_DATA][x]] = 0;
302+
if (!elems[obj[GENERICS_DATA][x][NAME]]) {
303+
elems[obj[GENERICS_DATA][x][NAME]] = 0;
304304
}
305-
elems[obj[GENERICS_DATA][x]] += 1;
305+
elems[obj[GENERICS_DATA][x][NAME]] += 1;
306306
}
307307
var total = 0;
308308
var done = 0;
@@ -345,6 +345,7 @@ window.initSearch = function(rawSearchIndex) {
345345
// Check for type name and type generics (if any).
346346
function checkType(obj, val, literalSearch) {
347347
var lev_distance = MAX_LEV_DISTANCE + 1;
348+
var tmp_lev = MAX_LEV_DISTANCE + 1;
348349
var len, x, firstGeneric;
349350
if (obj[NAME] === val.name) {
350351
if (literalSearch) {
@@ -354,10 +355,10 @@ window.initSearch = function(rawSearchIndex) {
354355
var elems = Object.create(null);
355356
len = obj[GENERICS_DATA].length;
356357
for (x = 0; x < len; ++x) {
357-
if (!elems[obj[GENERICS_DATA][x]]) {
358-
elems[obj[GENERICS_DATA][x]] = 0;
358+
if (!elems[obj[GENERICS_DATA][x][NAME]]) {
359+
elems[obj[GENERICS_DATA][x][NAME]] = 0;
359360
}
360-
elems[obj[GENERICS_DATA][x]] += 1;
361+
elems[obj[GENERICS_DATA][x][NAME]] += 1;
361362
}
362363

363364
var allFound = true;
@@ -382,7 +383,7 @@ window.initSearch = function(rawSearchIndex) {
382383
// If the type has generics but don't match, then it won't return at this point.
383384
// Otherwise, `checkGenerics` will return 0 and it'll return.
384385
if (obj.length > GENERICS_DATA && obj[GENERICS_DATA].length !== 0) {
385-
var tmp_lev = checkGenerics(obj, val);
386+
tmp_lev = checkGenerics(obj, val);
386387
if (tmp_lev <= MAX_LEV_DISTANCE) {
387388
return tmp_lev;
388389
}
@@ -392,8 +393,8 @@ window.initSearch = function(rawSearchIndex) {
392393
if ((!val.generics || val.generics.length === 0) &&
393394
obj.length > GENERICS_DATA && obj[GENERICS_DATA].length > 0) {
394395
return obj[GENERICS_DATA].some(
395-
function(name) {
396-
return name === val.name;
396+
function(gen) {
397+
return gen[NAME] === val.name;
397398
});
398399
}
399400
return false;
@@ -404,17 +405,27 @@ window.initSearch = function(rawSearchIndex) {
404405
// a levenshtein distance value that isn't *this* good so it goes
405406
// into the search results but not too high.
406407
lev_distance = Math.ceil((checkGenerics(obj, val) + lev_distance) / 2);
407-
} else if (obj.length > GENERICS_DATA && obj[GENERICS_DATA].length > 0) {
408+
}
409+
if (obj.length > GENERICS_DATA && obj[GENERICS_DATA].length > 0) {
408410
// We can check if the type we're looking for is inside the generics!
409411
var olength = obj[GENERICS_DATA].length;
410412
for (x = 0; x < olength; ++x) {
411-
lev_distance = Math.min(levenshtein(obj[GENERICS_DATA][x], val.name),
412-
lev_distance);
413+
tmp_lev = Math.min(levenshtein(obj[GENERICS_DATA][x][NAME], val.name), tmp_lev);
414+
}
415+
if (tmp_lev !== 0) {
416+
// If we didn't find a good enough result, we go check inside the generics of
417+
// the generics.
418+
for (x = 0; x < olength && tmp_lev !== 0; ++x) {
419+
tmp_lev = Math.min(
420+
checkType(obj[GENERICS_DATA][x], val, literalSearch),
421+
tmp_lev
422+
);
423+
}
413424
}
414425
}
415426
// Now whatever happens, the returned distance is "less good" so we should mark it
416427
// as such, and so we add 1 to the distance to make it "less good".
417-
return lev_distance + 1;
428+
return Math.min(lev_distance, tmp_lev) + 1;
418429
}
419430

420431
function findArg(obj, val, literalSearch, typeFilter) {

0 commit comments

Comments
 (0)