From 9683f8a9656580aae49f7664a5134d061b795b3f Mon Sep 17 00:00:00 2001 From: Michael Howell Date: Mon, 20 Mar 2023 11:31:17 -0700 Subject: [PATCH 01/15] rustdoc: use let chain in `CacheBuilder::fold_item` --- src/librustdoc/formats/cache.rs | 60 ++++++++++++++++----------------- 1 file changed, 29 insertions(+), 31 deletions(-) diff --git a/src/librustdoc/formats/cache.rs b/src/librustdoc/formats/cache.rs index 4c6e7dfb9873b..898aaa3e1962e 100644 --- a/src/librustdoc/formats/cache.rs +++ b/src/librustdoc/formats/cache.rs @@ -223,17 +223,16 @@ impl<'a, 'tcx> DocFolder for CacheBuilder<'a, 'tcx> { // If the impl is from a masked crate or references something from a // masked crate then remove it completely. - if let clean::ImplItem(ref i) = *item.kind { - if self.cache.masked_crates.contains(&item.item_id.krate()) + if let clean::ImplItem(ref i) = *item.kind && + (self.cache.masked_crates.contains(&item.item_id.krate()) || i.trait_ .as_ref() .map_or(false, |t| self.cache.masked_crates.contains(&t.def_id().krate)) || i.for_ .def_id(self.cache) - .map_or(false, |d| self.cache.masked_crates.contains(&d.krate)) - { - return None; - } + .map_or(false, |d| self.cache.masked_crates.contains(&d.krate))) + { + return None; } // Propagate a trait method's documentation to all implementors of the @@ -334,33 +333,32 @@ impl<'a, 'tcx> DocFolder for CacheBuilder<'a, 'tcx> { // A crate has a module at its root, containing all items, // which should not be indexed. The crate-item itself is // inserted later on when serializing the search-index. - if item.item_id.as_def_id().map_or(false, |idx| !idx.is_crate_root()) { + if item.item_id.as_def_id().map_or(false, |idx| !idx.is_crate_root()) + && let ty = item.type_() + && (ty != ItemType::StructField + || u16::from_str_radix(s.as_str(), 10).is_err()) + { let desc = short_markdown_summary(&item.doc_value(), &item.link_names(self.cache)); - let ty = item.type_(); - if ty != ItemType::StructField - || u16::from_str_radix(s.as_str(), 10).is_err() - { - // In case this is a field from a tuple struct, we don't add it into - // the search index because its name is something like "0", which is - // not useful for rustdoc search. - self.cache.search_index.push(IndexItem { - ty, - name: s, - path: join_with_double_colon(path), - desc, - parent, - parent_idx: None, - search_type: get_function_type_for_search( - &item, - self.tcx, - clean_impl_generics(self.cache.parent_stack.last()).as_ref(), - self.cache, - ), - aliases: item.attrs.get_doc_aliases(), - deprecation: item.deprecation(self.tcx), - }); - } + // In case this is a field from a tuple struct, we don't add it into + // the search index because its name is something like "0", which is + // not useful for rustdoc search. + self.cache.search_index.push(IndexItem { + ty, + name: s, + path: join_with_double_colon(path), + desc, + parent, + parent_idx: None, + search_type: get_function_type_for_search( + &item, + self.tcx, + clean_impl_generics(self.cache.parent_stack.last()).as_ref(), + self.cache, + ), + aliases: item.attrs.get_doc_aliases(), + deprecation: item.deprecation(self.tcx), + }); } } (Some(parent), None) if is_inherent_impl_item => { From 3fbfe2bca5e0227e2b9c9363558dc6a5dec54351 Mon Sep 17 00:00:00 2001 From: Michael Howell Date: Mon, 20 Mar 2023 16:02:51 -0700 Subject: [PATCH 02/15] rustdoc-search: add impl disambiguator to duplicate assoc items Helps with #90929 This changes the search results, specifically, when there's more than one impl with an associated item with the same name. For example, the search queries `simd -> simd` and `simd -> simd` don't link to the same function, but most of the functions have the same names. This change should probably be FCP-ed, especially since it adds a new anchor link format for `main.js` to handle, so that URLs like `struct.Vec.html#impl-AsMut<[T]>-for-Vec/method.as_mut` redirect to `struct.Vec.html#method.as_mut-2`. It's a strange design, but there are a few reasons for it: * I'd like to avoid making the HTML bigger. Obviously, fixing this bug is going to add at least a little more data to the search index, but adding more HTML penalises viewers for the benefit of searchers. * Breaking `struct.Vec.html#method.len` would also be a disappointment. On the other hand: * The path-style anchors might be less prone to link rot than the numbered anchors. It's definitely less likely to have URLs that appear to "work", but silently point at the wrong thing. * This commit arranges the path-style anchor to redirect to the numbered anchor. Nothing stops rustdoc from doing the opposite, making path-style anchors the default and redirecting the "legacy" numbered ones. --- src/librustdoc/formats/cache.rs | 13 ++++ src/librustdoc/html/render/mod.rs | 41 +++++++---- src/librustdoc/html/render/search_index.rs | 33 ++++++++- src/librustdoc/html/render/sidebar.rs | 3 +- src/librustdoc/html/static/js/main.js | 24 +++++++ src/librustdoc/html/static/js/search.js | 19 ++++- .../search-result-impl-disambiguation.goml | 41 +++++++++++ tests/rustdoc-gui/src/test_docs/lib.rs | 18 +++++ tests/rustdoc-js-std/simd-type-signatures.js | 70 +++++++++++++++++++ .../rustdoc-js/search-method-disambiguate.js | 28 ++++++++ .../rustdoc-js/search-method-disambiguate.rs | 22 ++++++ tests/rustdoc/issue-32077-type-alias-impls.rs | 2 +- 12 files changed, 293 insertions(+), 21 deletions(-) create mode 100644 tests/rustdoc-gui/search-result-impl-disambiguation.goml create mode 100644 tests/rustdoc-js-std/simd-type-signatures.js create mode 100644 tests/rustdoc-js/search-method-disambiguate.js create mode 100644 tests/rustdoc-js/search-method-disambiguate.rs diff --git a/src/librustdoc/formats/cache.rs b/src/librustdoc/formats/cache.rs index 898aaa3e1962e..b916baecc1407 100644 --- a/src/librustdoc/formats/cache.rs +++ b/src/librustdoc/formats/cache.rs @@ -350,6 +350,11 @@ impl<'a, 'tcx> DocFolder for CacheBuilder<'a, 'tcx> { desc, parent, parent_idx: None, + impl_id: if let Some(ParentStackItem::Impl { item_id, .. }) = self.cache.parent_stack.last() { + item_id.as_def_id() + } else { + None + }, search_type: get_function_type_for_search( &item, self.tcx, @@ -369,6 +374,13 @@ impl<'a, 'tcx> DocFolder for CacheBuilder<'a, 'tcx> { parent, item: item.clone(), impl_generics, + impl_id: if let Some(ParentStackItem::Impl { item_id, .. }) = + self.cache.parent_stack.last() + { + item_id.as_def_id() + } else { + None + }, }); } _ => {} @@ -539,6 +551,7 @@ impl<'a, 'tcx> DocFolder for CacheBuilder<'a, 'tcx> { pub(crate) struct OrphanImplItem { pub(crate) parent: DefId, + pub(crate) impl_id: Option, pub(crate) item: clean::Item, pub(crate) impl_generics: Option<(clean::Type, clean::Generics)>, } diff --git a/src/librustdoc/html/render/mod.rs b/src/librustdoc/html/render/mod.rs index 3e671a64b54f7..b0ce475850cf8 100644 --- a/src/librustdoc/html/render/mod.rs +++ b/src/librustdoc/html/render/mod.rs @@ -54,7 +54,7 @@ use rustc_data_structures::fx::{FxHashMap, FxHashSet}; use rustc_hir::def_id::{DefId, DefIdSet}; use rustc_hir::Mutability; use rustc_middle::middle::stability; -use rustc_middle::ty::TyCtxt; +use rustc_middle::ty::{self, TyCtxt}; use rustc_span::{ symbol::{sym, Symbol}, BytePos, FileName, RealFileName, @@ -102,6 +102,7 @@ pub(crate) struct IndexItem { pub(crate) desc: String, pub(crate) parent: Option, pub(crate) parent_idx: Option, + pub(crate) impl_id: Option, pub(crate) search_type: Option, pub(crate) aliases: Box<[Symbol]>, pub(crate) deprecation: Option, @@ -1877,7 +1878,7 @@ pub(crate) fn render_impl_summary( aliases: &[String], ) { let inner_impl = i.inner_impl(); - let id = cx.derive_id(get_id_for_impl(&inner_impl.for_, inner_impl.trait_.as_ref(), cx)); + let id = cx.derive_id(get_id_for_impl(cx.tcx(), i.impl_item.item_id)); let aliases = if aliases.is_empty() { String::new() } else { @@ -1994,21 +1995,35 @@ pub(crate) fn small_url_encode(s: String) -> String { } } -fn get_id_for_impl(for_: &clean::Type, trait_: Option<&clean::Path>, cx: &Context<'_>) -> String { - match trait_ { - Some(t) => small_url_encode(format!("impl-{:#}-for-{:#}", t.print(cx), for_.print(cx))), - None => small_url_encode(format!("impl-{:#}", for_.print(cx))), - } +fn get_id_for_impl<'tcx>(tcx: TyCtxt<'tcx>, impl_id: ItemId) -> String { + use rustc_middle::ty::print::with_forced_trimmed_paths; + let (type_, trait_) = match impl_id { + ItemId::Auto { trait_, for_ } => { + let ty = tcx.type_of(for_).skip_binder(); + (ty, Some(ty::TraitRef::new(tcx, trait_, [ty]))) + } + ItemId::Blanket { impl_id, .. } | ItemId::DefId(impl_id) => { + match tcx.impl_subject(impl_id).skip_binder() { + ty::ImplSubject::Trait(trait_ref) => { + (trait_ref.args[0].expect_ty(), Some(trait_ref)) + } + ty::ImplSubject::Inherent(ty) => (ty, None), + } + } + }; + with_forced_trimmed_paths!(small_url_encode(if let Some(trait_) = trait_ { + format!("impl-{trait_}-for-{type_}", trait_ = trait_.print_only_trait_path()) + } else { + format!("impl-{type_}") + })) } fn extract_for_impl_name(item: &clean::Item, cx: &Context<'_>) -> Option<(String, String)> { match *item.kind { - clean::ItemKind::ImplItem(ref i) => { - i.trait_.as_ref().map(|trait_| { - // Alternative format produces no URLs, - // so this parameter does nothing. - (format!("{:#}", i.for_.print(cx)), get_id_for_impl(&i.for_, Some(trait_), cx)) - }) + clean::ItemKind::ImplItem(ref i) if i.trait_.is_some() => { + // Alternative format produces no URLs, + // so this parameter does nothing. + Some((format!("{:#}", i.for_.print(cx)), get_id_for_impl(cx.tcx(), item.item_id))) } _ => None, } diff --git a/src/librustdoc/html/render/search_index.rs b/src/librustdoc/html/render/search_index.rs index 78c443b2257d0..af1dab5949613 100644 --- a/src/librustdoc/html/render/search_index.rs +++ b/src/librustdoc/html/render/search_index.rs @@ -12,7 +12,7 @@ use crate::formats::cache::{Cache, OrphanImplItem}; use crate::formats::item_type::ItemType; use crate::html::format::join_with_double_colon; use crate::html::markdown::short_markdown_summary; -use crate::html::render::{IndexItem, IndexItemFunctionType, RenderType, RenderTypeId}; +use crate::html::render::{self, IndexItem, IndexItemFunctionType, RenderType, RenderTypeId}; /// Builds the search index from the collected metadata pub(crate) fn build_index<'tcx>( @@ -26,7 +26,8 @@ pub(crate) fn build_index<'tcx>( // Attach all orphan items to the type's definition if the type // has since been learned. - for &OrphanImplItem { parent, ref item, ref impl_generics } in &cache.orphan_impl_items { + for &OrphanImplItem { impl_id, parent, ref item, ref impl_generics } in &cache.orphan_impl_items + { if let Some((fqp, _)) = cache.paths.get(&parent) { let desc = short_markdown_summary(&item.doc_value(), &item.link_names(cache)); cache.search_index.push(IndexItem { @@ -36,6 +37,7 @@ pub(crate) fn build_index<'tcx>( desc, parent: Some(parent), parent_idx: None, + impl_id, search_type: get_function_type_for_search(item, tcx, impl_generics.as_ref(), cache), aliases: item.attrs.get_doc_aliases(), deprecation: item.deprecation(tcx), @@ -222,6 +224,29 @@ pub(crate) fn build_index<'tcx>( }) .collect(); + // Find associated items that need disambiguators + let mut associated_item_duplicates = FxHashMap::<(isize, ItemType, Symbol), usize>::default(); + + for &item in &crate_items { + if item.impl_id.is_some() && let Some(parent_idx) = item.parent_idx { + let count = associated_item_duplicates + .entry((parent_idx, item.ty, item.name)) + .or_insert(0); + *count += 1; + } + } + + let associated_item_disambiguators = crate_items + .iter() + .enumerate() + .filter_map(|(index, item)| { + let impl_id = ItemId::DefId(item.impl_id?); + let parent_idx = item.parent_idx?; + let count = *associated_item_duplicates.get(&(parent_idx, item.ty, item.name))?; + if count > 1 { Some((index, render::get_id_for_impl(tcx, impl_id))) } else { None } + }) + .collect::>(); + struct CrateData<'a> { doc: String, items: Vec<&'a IndexItem>, @@ -230,6 +255,8 @@ pub(crate) fn build_index<'tcx>( // // To be noted: the `usize` elements are indexes to `items`. aliases: &'a BTreeMap>, + // Used when a type has more than one impl with an associated item with the same name. + associated_item_disambiguators: &'a Vec<(usize, String)>, } struct Paths { @@ -382,6 +409,7 @@ pub(crate) fn build_index<'tcx>( crate_data.serialize_field("f", &functions)?; crate_data.serialize_field("c", &deprecated)?; crate_data.serialize_field("p", &paths)?; + crate_data.serialize_field("b", &self.associated_item_disambiguators)?; if has_aliases { crate_data.serialize_field("a", &self.aliases)?; } @@ -398,6 +426,7 @@ pub(crate) fn build_index<'tcx>( items: crate_items, paths: crate_paths, aliases: &aliases, + associated_item_disambiguators: &associated_item_disambiguators, }) .expect("failed serde conversion") // All these `replace` calls are because we have to go through JS string for JSON content. diff --git a/src/librustdoc/html/render/sidebar.rs b/src/librustdoc/html/render/sidebar.rs index 76f63c6f63e36..3f8b22050c8e1 100644 --- a/src/librustdoc/html/render/sidebar.rs +++ b/src/librustdoc/html/render/sidebar.rs @@ -503,8 +503,7 @@ fn sidebar_render_assoc_items( .iter() .filter_map(|it| { let trait_ = it.inner_impl().trait_.as_ref()?; - let encoded = - id_map.derive(super::get_id_for_impl(&it.inner_impl().for_, Some(trait_), cx)); + let encoded = id_map.derive(super::get_id_for_impl(cx.tcx(), it.impl_item.item_id)); let prefix = match it.inner_impl().polarity { ty::ImplPolarity::Positive | ty::ImplPolarity::Reservation => "", diff --git a/src/librustdoc/html/static/js/main.js b/src/librustdoc/html/static/js/main.js index eb256455b0878..970c2f2d45d15 100644 --- a/src/librustdoc/html/static/js/main.js +++ b/src/librustdoc/html/static/js/main.js @@ -354,6 +354,30 @@ function preLoadCss(cssUrl) { expandSection(pageId); } } + if (savedHash.startsWith("#impl-")) { + // impl-disambiguated links, used by the search engine + // format: impl-X[-for-Y]/method.WHATEVER + // turn this into method.WHATEVER[-NUMBER] + const splitAt = savedHash.indexOf("/"); + if (splitAt !== -1) { + const implId = savedHash.slice(1, splitAt); + const assocId = savedHash.slice(splitAt + 1); + const implElem = document.getElementById(implId); + if (implElem && implElem.parentElement.tagName === "SUMMARY" && + implElem.parentElement.parentElement.tagName === "DETAILS") { + onEachLazy(implElem.parentElement.parentElement.querySelectorAll( + `[id^="${assocId}"]`), + item => { + const numbered = /([^-]+)-([0-9]+)/.exec(item.id); + if (item.id === assocId || (numbered && numbered[1] === assocId)) { + expandSection(item.id); + window.location = "#" + item.id; + } + } + ); + } + } + } } function onHashChange(ev) { diff --git a/src/librustdoc/html/static/js/search.js b/src/librustdoc/html/static/js/search.js index 2f0cae0a48e21..6cc0707a552dd 100644 --- a/src/librustdoc/html/static/js/search.js +++ b/src/librustdoc/html/static/js/search.js @@ -1752,6 +1752,7 @@ function initSearch(rawSearchIndex) { type: item.type, is_alias: true, deprecated: item.deprecated, + implDisambiguator: item.implDisambiguator, }; } @@ -2218,7 +2219,7 @@ function initSearch(rawSearchIndex) { href = ROOT_PATH + name + "/index.html"; } else if (item.parent !== undefined) { const myparent = item.parent; - let anchor = "#" + type + "." + name; + let anchor = type + "." + name; const parentType = itemTypes[myparent.ty]; let pageType = parentType; let pageName = myparent.name; @@ -2232,16 +2233,19 @@ function initSearch(rawSearchIndex) { const enumName = item.path.substr(enumNameIdx + 2); path = item.path.substr(0, enumNameIdx); displayPath = path + "::" + enumName + "::" + myparent.name + "::"; - anchor = "#variant." + myparent.name + ".field." + name; + anchor = "variant." + myparent.name + ".field." + name; pageType = "enum"; pageName = enumName; } else { displayPath = path + "::" + myparent.name + "::"; } + if (item.implDisambiguator !== null) { + anchor = item.implDisambiguator + "/" + anchor; + } href = ROOT_PATH + path.replace(/::/g, "/") + "/" + pageType + "." + pageName + - ".html" + anchor; + ".html#" + anchor; } else { displayPath = item.path + "::"; href = ROOT_PATH + item.path.replace(/::/g, "/") + @@ -2727,6 +2731,10 @@ ${item.displayPath}${name}\ * Types are also represented as arrays; the first item is an index into the `p` * array, while the second is a list of types representing any generic parameters. * + * b[i] contains an item's impl disambiguator. This is only present if an item + * is defined in an impl block and, the impl block's type has more than one associated + * item with the same name. + * * `a` defines aliases with an Array of pairs: [name, offset], where `offset` * points into the n/t/d/q/i/f arrays. * @@ -2746,6 +2754,7 @@ ${item.displayPath}${name}\ * i: Array, * f: Array, * p: Array, + * b: Array<[Number, String]>, * c: Array * }} */ @@ -2766,6 +2775,7 @@ ${item.displayPath}${name}\ id: id, normalizedName: crate.indexOf("_") === -1 ? crate : crate.replace(/_/g, ""), deprecated: null, + implDisambiguator: null, }; id += 1; searchIndex.push(crateRow); @@ -2789,6 +2799,8 @@ ${item.displayPath}${name}\ const itemFunctionSearchTypes = crateCorpus.f; // an array of (Number) indices for the deprecated items const deprecatedItems = new Set(crateCorpus.c); + // an array of (Number) indices for the deprecated items + const implDisambiguator = new Map(crateCorpus.b); // an array of [(Number) item type, // (String) name] const paths = crateCorpus.p; @@ -2849,6 +2861,7 @@ ${item.displayPath}${name}\ id: id, normalizedName: word.indexOf("_") === -1 ? word : word.replace(/_/g, ""), deprecated: deprecatedItems.has(i), + implDisambiguator: implDisambiguator.has(i) ? implDisambiguator.get(i) : null, }; id += 1; searchIndex.push(row); diff --git a/tests/rustdoc-gui/search-result-impl-disambiguation.goml b/tests/rustdoc-gui/search-result-impl-disambiguation.goml new file mode 100644 index 0000000000000..98a2cd9577397 --- /dev/null +++ b/tests/rustdoc-gui/search-result-impl-disambiguation.goml @@ -0,0 +1,41 @@ +// ignore-tidy-linelength + +// Checks that, if a type has two methods with the same name, they both get +// linked correctly. +goto: "file://" + |DOC_PATH| + "/test_docs/index.html" + +// This should link to the inherent impl +write: (".search-input", "ZyxwvutMethodDisambiguation -> bool") +// To be SURE that the search will be run. +press-key: 'Enter' +// Waiting for the search results to appear... +wait-for: "#search-tabs" +// Check the disambiguated link. +assert-count: ("a.result-method", 1) +assert-attribute: ("a.result-method", { + "href": "../test_docs/struct.ZyxwvutMethodDisambiguation.html#impl-ZyxwvutMethodDisambiguation/method.method_impl_disambiguation" +}) +click: "a.result-method" +wait-for: "#impl-ZyxwvutMethodDisambiguation" +assert-document-property: ({ + "URL": "struct.ZyxwvutMethodDisambiguation.html#method.method_impl_disambiguation" +}, ENDS_WITH) + +goto: "file://" + |DOC_PATH| + "/test_docs/index.html" + +// This should link to the trait impl +write: (".search-input", "ZyxwvutMethodDisambiguation, usize -> usize") +// To be SURE that the search will be run. +press-key: 'Enter' +// Waiting for the search results to appear... +wait-for: "#search-tabs" +// Check the disambiguated link. +assert-count: ("a.result-method", 1) +assert-attribute: ("a.result-method", { + "href": "../test_docs/struct.ZyxwvutMethodDisambiguation.html#impl-ZyxwvutTrait-for-ZyxwvutMethodDisambiguation/method.method_impl_disambiguation" +}) +click: "a.result-method" +wait-for: "#impl-ZyxwvutMethodDisambiguation" +assert-document-property: ({ + "URL": "struct.ZyxwvutMethodDisambiguation.html#method.method_impl_disambiguation-1" +}, ENDS_WITH) diff --git a/tests/rustdoc-gui/src/test_docs/lib.rs b/tests/rustdoc-gui/src/test_docs/lib.rs index 38180aef762b4..5c91bcbb4eecb 100644 --- a/tests/rustdoc-gui/src/test_docs/lib.rs +++ b/tests/rustdoc-gui/src/test_docs/lib.rs @@ -529,3 +529,21 @@ pub mod cfgs { /// Some docs. pub mod cfgs {} } + +pub struct ZyxwvutMethodDisambiguation; + +impl ZyxwvutMethodDisambiguation { + pub fn method_impl_disambiguation(&self) -> bool { + true + } +} + +pub trait ZyxwvutTrait { + fn method_impl_disambiguation(&self, x: usize) -> usize; +} + +impl ZyxwvutTrait for ZyxwvutMethodDisambiguation { + fn method_impl_disambiguation(&self, x: usize) -> usize { + x + } +} diff --git a/tests/rustdoc-js-std/simd-type-signatures.js b/tests/rustdoc-js-std/simd-type-signatures.js new file mode 100644 index 0000000000000..bd16827095736 --- /dev/null +++ b/tests/rustdoc-js-std/simd-type-signatures.js @@ -0,0 +1,70 @@ +// exact-check +// ignore-order +// ignore-tidy-linelength + +// This test case verifies that the href points at the correct impl + +const FILTER_CRATE = "std"; + +const EXPECTED = [ + { + 'query': 'simd, simd -> simd', + 'others': [ + { + 'path': 'std::simd::Simd', + 'name': 'simd_max', + 'href': '../std/simd/struct.Simd.html#impl-core::simd::SimdOrd-for-core::simd::Simd%3Ci16,+LANES%3E/method.simd_max' + }, + { + 'path': 'std::simd::Simd', + 'name': 'simd_min', + 'href': '../std/simd/struct.Simd.html#impl-core::simd::SimdOrd-for-core::simd::Simd%3Ci16,+LANES%3E/method.simd_min' + }, + { + 'path': 'std::simd::Simd', + 'name': 'simd_clamp', + 'href': '../std/simd/struct.Simd.html#impl-core::simd::SimdOrd-for-core::simd::Simd%3Ci16,+LANES%3E/method.simd_clamp' + }, + { + 'path': 'std::simd::Simd', + 'name': 'saturating_add', + 'href': '../std/simd/struct.Simd.html#impl-core::simd::SimdInt-for-core::simd::Simd%3Ci16,+LANES%3E/method.saturating_add' + }, + { + 'path': 'std::simd::Simd', + 'name': 'saturating_sub', + 'href': '../std/simd/struct.Simd.html#impl-core::simd::SimdInt-for-core::simd::Simd%3Ci16,+LANES%3E/method.saturating_sub' + }, + ], + }, + { + 'query': 'simd, simd -> simd', + 'others': [ + { + 'path': 'std::simd::Simd', + 'name': 'simd_max', + 'href': '../std/simd/struct.Simd.html#impl-core::simd::SimdOrd-for-core::simd::Simd%3Ci8,+LANES%3E/method.simd_max' + }, + { + 'path': 'std::simd::Simd', + 'name': 'simd_min', + 'href': '../std/simd/struct.Simd.html#impl-core::simd::SimdOrd-for-core::simd::Simd%3Ci8,+LANES%3E/method.simd_min' + }, + { + 'path': 'std::simd::Simd', + 'name': 'simd_clamp', + 'href': '../std/simd/struct.Simd.html#impl-core::simd::SimdOrd-for-core::simd::Simd%3Ci8,+LANES%3E/method.simd_clamp' + }, + { + 'path': 'std::simd::Simd', + 'name': 'saturating_add', + 'href': '../std/simd/struct.Simd.html#impl-core::simd::SimdInt-for-core::simd::Simd%3Ci8,+LANES%3E/method.saturating_add' + }, + { + 'path': 'std::simd::Simd', + 'name': 'saturating_sub', + 'href': '../std/simd/struct.Simd.html#impl-core::simd::SimdInt-for-core::simd::Simd%3Ci8,+LANES%3E/method.saturating_sub' + }, + ], + }, +]; diff --git a/tests/rustdoc-js/search-method-disambiguate.js b/tests/rustdoc-js/search-method-disambiguate.js new file mode 100644 index 0000000000000..70aa895f99458 --- /dev/null +++ b/tests/rustdoc-js/search-method-disambiguate.js @@ -0,0 +1,28 @@ +// exact-check +// ignore-order +// ignore-tidy-linelength + +const FILTER_CRATE = "search_method_disambiguate"; + +const EXPECTED = [ + { + 'query': 'MyTy -> bool', + 'others': [ + { + 'path': 'search_method_disambiguate::MyTy', + 'name': 'my_method', + 'href': '../search_method_disambiguate/struct.MyTy.html#impl-X-for-MyTy%3Cbool%3E/method.my_method' + }, + ], + }, + { + 'query': 'MyTy -> u8', + 'others': [ + { + 'path': 'search_method_disambiguate::MyTy', + 'name': 'my_method', + 'href': '../search_method_disambiguate/struct.MyTy.html#impl-X-for-MyTy%3Cu8%3E/method.my_method' + }, + ], + } +]; diff --git a/tests/rustdoc-js/search-method-disambiguate.rs b/tests/rustdoc-js/search-method-disambiguate.rs new file mode 100644 index 0000000000000..ae884447a92e7 --- /dev/null +++ b/tests/rustdoc-js/search-method-disambiguate.rs @@ -0,0 +1,22 @@ +pub trait X { + type InnerType; + fn my_method(&self) -> Self::InnerType; +} + +pub struct MyTy { + pub t: T, +} + +impl X for MyTy { + type InnerType = bool; + fn my_method(&self) -> bool { + self.t + } +} + +impl X for MyTy { + type InnerType = u8; + fn my_method(&self) -> u8 { + self.t + } +} diff --git a/tests/rustdoc/issue-32077-type-alias-impls.rs b/tests/rustdoc/issue-32077-type-alias-impls.rs index ac486c36ad044..664b678093e90 100644 --- a/tests/rustdoc/issue-32077-type-alias-impls.rs +++ b/tests/rustdoc/issue-32077-type-alias-impls.rs @@ -22,7 +22,7 @@ impl Bar for GenericStruct {} // We check that "Aliased type" is also present as a title in the sidebar. // @has - '//*[@class="sidebar-elems"]//h3/a[@href="#aliased-type"]' 'Aliased type' // We check that we have the implementation of the type alias itself. -// @has - '//*[@id="impl-TypedefStruct"]/h3' 'impl TypedefStruct' +// @has - '//*[@id="impl-GenericStruct%3Cu8%3E"]/h3' 'impl TypedefStruct' // @has - '//*[@id="method.on_alias"]/h4' 'pub fn on_alias()' // @has - '//*[@id="impl-GenericStruct%3CT%3E"]/h3' 'impl GenericStruct' // @has - '//*[@id="method.on_gen"]/h4' 'pub fn on_gen(arg: T)' From 3583e86674749c279e7edd96641255bbf8595de1 Mon Sep 17 00:00:00 2001 From: Michael Howell Date: Mon, 20 Mar 2023 17:56:45 -0700 Subject: [PATCH 03/15] rustdoc: update test cases for changes to the printing style This whole thing changes it so that the JS and the UI both use rustc's own path printing to handle the impl IDs. This results in the format changing a little bit; full paths are used in spots where they aren't strictly necessary, and the path sometimes uses generics where the old system used the trait's own name, but it shouldn't matter since the orphan rules will prevent it anyway. --- tests/rustdoc-js-std/simd-type-signatures.js | 40 +++++++++---------- tests/rustdoc/blanket-reexport-item.rs | 2 +- tests/rustdoc/const-generics/const-impl.rs | 2 +- tests/rustdoc/generic-impl.rs | 4 +- tests/rustdoc/issue-78701.rs | 2 +- .../primitive/primitive-generic-impl.rs | 2 +- .../rustdoc/sidebar-links-to-foreign-impl.rs | 4 +- tests/rustdoc/src-links-auto-impls.rs | 4 +- tests/rustdoc/where-clause-order.rs | 13 +++++- 9 files changed, 42 insertions(+), 31 deletions(-) diff --git a/tests/rustdoc-js-std/simd-type-signatures.js b/tests/rustdoc-js-std/simd-type-signatures.js index bd16827095736..5c7cf372bce14 100644 --- a/tests/rustdoc-js-std/simd-type-signatures.js +++ b/tests/rustdoc-js-std/simd-type-signatures.js @@ -11,29 +11,29 @@ const EXPECTED = [ 'query': 'simd, simd -> simd', 'others': [ { - 'path': 'std::simd::Simd', + 'path': 'std::simd::prelude::Simd', 'name': 'simd_max', - 'href': '../std/simd/struct.Simd.html#impl-core::simd::SimdOrd-for-core::simd::Simd%3Ci16,+LANES%3E/method.simd_max' + 'href': '../std/simd/prelude/struct.Simd.html#impl-SimdOrd-for-Simd%3Ci16,+LANES%3E/method.simd_max' }, { - 'path': 'std::simd::Simd', + 'path': 'std::simd::prelude::Simd', 'name': 'simd_min', - 'href': '../std/simd/struct.Simd.html#impl-core::simd::SimdOrd-for-core::simd::Simd%3Ci16,+LANES%3E/method.simd_min' + 'href': '../std/simd/prelude/struct.Simd.html#impl-SimdOrd-for-Simd%3Ci16,+LANES%3E/method.simd_min' }, { - 'path': 'std::simd::Simd', + 'path': 'std::simd::prelude::Simd', 'name': 'simd_clamp', - 'href': '../std/simd/struct.Simd.html#impl-core::simd::SimdOrd-for-core::simd::Simd%3Ci16,+LANES%3E/method.simd_clamp' + 'href': '../std/simd/prelude/struct.Simd.html#impl-SimdOrd-for-Simd%3Ci16,+LANES%3E/method.simd_clamp' }, { - 'path': 'std::simd::Simd', + 'path': 'std::simd::prelude::Simd', 'name': 'saturating_add', - 'href': '../std/simd/struct.Simd.html#impl-core::simd::SimdInt-for-core::simd::Simd%3Ci16,+LANES%3E/method.saturating_add' + 'href': '../std/simd/prelude/struct.Simd.html#impl-SimdInt-for-Simd%3Ci16,+LANES%3E/method.saturating_add' }, { - 'path': 'std::simd::Simd', + 'path': 'std::simd::prelude::Simd', 'name': 'saturating_sub', - 'href': '../std/simd/struct.Simd.html#impl-core::simd::SimdInt-for-core::simd::Simd%3Ci16,+LANES%3E/method.saturating_sub' + 'href': '../std/simd/prelude/struct.Simd.html#impl-SimdInt-for-Simd%3Ci16,+LANES%3E/method.saturating_sub' }, ], }, @@ -41,29 +41,29 @@ const EXPECTED = [ 'query': 'simd, simd -> simd', 'others': [ { - 'path': 'std::simd::Simd', + 'path': 'std::simd::prelude::Simd', 'name': 'simd_max', - 'href': '../std/simd/struct.Simd.html#impl-core::simd::SimdOrd-for-core::simd::Simd%3Ci8,+LANES%3E/method.simd_max' + 'href': '../std/simd/prelude/struct.Simd.html#impl-SimdOrd-for-Simd%3Ci8,+LANES%3E/method.simd_max' }, { - 'path': 'std::simd::Simd', + 'path': 'std::simd::prelude::Simd', 'name': 'simd_min', - 'href': '../std/simd/struct.Simd.html#impl-core::simd::SimdOrd-for-core::simd::Simd%3Ci8,+LANES%3E/method.simd_min' + 'href': '../std/simd/prelude/struct.Simd.html#impl-SimdOrd-for-Simd%3Ci8,+LANES%3E/method.simd_min' }, { - 'path': 'std::simd::Simd', + 'path': 'std::simd::prelude::Simd', 'name': 'simd_clamp', - 'href': '../std/simd/struct.Simd.html#impl-core::simd::SimdOrd-for-core::simd::Simd%3Ci8,+LANES%3E/method.simd_clamp' + 'href': '../std/simd/prelude/struct.Simd.html#impl-SimdOrd-for-Simd%3Ci8,+LANES%3E/method.simd_clamp' }, { - 'path': 'std::simd::Simd', + 'path': 'std::simd::prelude::Simd', 'name': 'saturating_add', - 'href': '../std/simd/struct.Simd.html#impl-core::simd::SimdInt-for-core::simd::Simd%3Ci8,+LANES%3E/method.saturating_add' + 'href': '../std/simd/prelude/struct.Simd.html#impl-SimdInt-for-Simd%3Ci8,+LANES%3E/method.saturating_add' }, { - 'path': 'std::simd::Simd', + 'path': 'std::simd::prelude::Simd', 'name': 'saturating_sub', - 'href': '../std/simd/struct.Simd.html#impl-core::simd::SimdInt-for-core::simd::Simd%3Ci8,+LANES%3E/method.saturating_sub' + 'href': '../std/simd/prelude/struct.Simd.html#impl-SimdInt-for-Simd%3Ci8,+LANES%3E/method.saturating_sub' }, ], }, diff --git a/tests/rustdoc/blanket-reexport-item.rs b/tests/rustdoc/blanket-reexport-item.rs index 437f0001fcfc4..315a38c30c54f 100644 --- a/tests/rustdoc/blanket-reexport-item.rs +++ b/tests/rustdoc/blanket-reexport-item.rs @@ -1,6 +1,6 @@ #![crate_name = "foo"] -// @has foo/struct.S.html '//*[@id="impl-Into%3CU%3E-for-S"]//h3[@class="code-header"]' 'impl Into for T' +// @has foo/struct.S.html '//*[@id="impl-Into%3CU%3E-for-T"]//h3[@class="code-header"]' 'impl Into for T' pub struct S2 {} mod m { pub struct S {} diff --git a/tests/rustdoc/const-generics/const-impl.rs b/tests/rustdoc/const-generics/const-impl.rs index 152b643bf4bd8..b424ea4b33cec 100644 --- a/tests/rustdoc/const-generics/const-impl.rs +++ b/tests/rustdoc/const-generics/const-impl.rs @@ -31,7 +31,7 @@ impl VSet { pub struct Escape; -// @has foo/struct.Escape.html '//*[@id="impl-Escape%3Cr%23%22%3Cscript%3Ealert(%22Escape%22);%3C/script%3E%22%23%3E"]/h3[@class="code-header"]' 'impl Escapealert("Escape");"#>' +// @has foo/struct.Escape.html '//*[@id="impl-Escape%3C%22%3Cscript%3Ealert(%5C%22Escape%5C%22);%3C/script%3E%22%3E"]/h3[@class="code-header"]' 'impl Escapealert("Escape");"#>' impl Escapealert("Escape");"#> { pub fn f() {} } diff --git a/tests/rustdoc/generic-impl.rs b/tests/rustdoc/generic-impl.rs index 6f68b1574992b..f62540c6bf963 100644 --- a/tests/rustdoc/generic-impl.rs +++ b/tests/rustdoc/generic-impl.rs @@ -5,9 +5,9 @@ use std::fmt; // @!has foo/struct.Bar.html '//*[@id="impl-ToString-for-Bar"]' '' pub struct Bar; -// @has foo/struct.Foo.html '//*[@id="impl-ToString-for-Foo"]//h3[@class="code-header"]' 'impl ToString for T' +// @has foo/struct.Foo.html '//*[@id="impl-ToString-for-T"]//h3[@class="code-header"]' 'impl ToString for T' pub struct Foo; -// @has foo/struct.Foo.html '//*[@class="sidebar-elems"]//section//a[@href="#impl-ToString-for-Foo"]' 'ToString' +// @has foo/struct.Foo.html '//*[@class="sidebar-elems"]//section//a[@href="#impl-ToString-for-T"]' 'ToString' impl fmt::Display for Foo { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { diff --git a/tests/rustdoc/issue-78701.rs b/tests/rustdoc/issue-78701.rs index e3e46468f3840..3f1638d5ffc4e 100644 --- a/tests/rustdoc/issue-78701.rs +++ b/tests/rustdoc/issue-78701.rs @@ -6,7 +6,7 @@ // @has 'foo/struct.AnotherStruct.html' // @count - '//*[@class="sidebar"]//a[@href="#impl-AnAmazingTrait-for-AnotherStruct%3C()%3E"]' 1 -// @count - '//*[@class="sidebar"]//a[@href="#impl-AnAmazingTrait-for-AnotherStruct%3CT%3E"]' 1 +// @count - '//*[@class="sidebar"]//a[@href="#impl-AnAmazingTrait-for-T"]' 1 pub trait Something {} diff --git a/tests/rustdoc/primitive/primitive-generic-impl.rs b/tests/rustdoc/primitive/primitive-generic-impl.rs index 2da8ae6ff38da..558336d731629 100644 --- a/tests/rustdoc/primitive/primitive-generic-impl.rs +++ b/tests/rustdoc/primitive/primitive-generic-impl.rs @@ -1,7 +1,7 @@ #![feature(rustc_attrs)] #![crate_name = "foo"] -// @has foo/primitive.i32.html '//*[@id="impl-ToString-for-i32"]//h3[@class="code-header"]' 'impl ToString for T' +// @has foo/primitive.i32.html '//*[@id="impl-ToString-for-T"]//h3[@class="code-header"]' 'impl ToString for T' #[rustc_doc_primitive = "i32"] /// Some useless docs, wouhou! diff --git a/tests/rustdoc/sidebar-links-to-foreign-impl.rs b/tests/rustdoc/sidebar-links-to-foreign-impl.rs index caa17dfbb1c73..733a18ad94a45 100644 --- a/tests/rustdoc/sidebar-links-to-foreign-impl.rs +++ b/tests/rustdoc/sidebar-links-to-foreign-impl.rs @@ -7,8 +7,8 @@ // @has - '//h2[@id="foreign-impls"]' 'Implementations on Foreign Types' // @has - '//*[@class="sidebar-elems"]//section//a[@href="#impl-Foo-for-u32"]' 'u32' // @has - '//*[@id="impl-Foo-for-u32"]//h3[@class="code-header"]' 'impl Foo for u32' -// @has - "//*[@class=\"sidebar-elems\"]//section//a[@href=\"#impl-Foo-for-%26'a+str\"]" "&'a str" -// @has - "//*[@id=\"impl-Foo-for-%26'a+str\"]//h3[@class=\"code-header\"]" "impl<'a> Foo for &'a str" +// @has - "//*[@class=\"sidebar-elems\"]//section//a[@href=\"#impl-Foo-for-%26str\"]" "&'a str" +// @has - "//*[@id=\"impl-Foo-for-%26str\"]//h3[@class=\"code-header\"]" "impl<'a> Foo for &'a str" pub trait Foo {} impl Foo for u32 {} diff --git a/tests/rustdoc/src-links-auto-impls.rs b/tests/rustdoc/src-links-auto-impls.rs index 1c8d157319254..08a497d4cf5e0 100644 --- a/tests/rustdoc/src-links-auto-impls.rs +++ b/tests/rustdoc/src-links-auto-impls.rs @@ -5,8 +5,8 @@ // @!has - '//*[@id="impl-Sized-for-Unsized"]//a[@class="src"]' 'source' // @has - '//*[@id="impl-Sync-for-Unsized"]/h3[@class="code-header"]' 'impl Sync for Unsized' // @!has - '//*[@id="impl-Sync-for-Unsized"]//a[@class="src"]' 'source' -// @has - '//*[@id="impl-Any-for-Unsized"]/h3[@class="code-header"]' 'impl Any for T' -// @has - '//*[@id="impl-Any-for-Unsized"]//a[@class="src rightside"]' 'source' +// @has - '//*[@id="impl-Any-for-T"]/h3[@class="code-header"]' 'impl Any for T' +// @has - '//*[@id="impl-Any-for-T"]//a[@class="src rightside"]' 'source' pub struct Unsized { data: [u8], } diff --git a/tests/rustdoc/where-clause-order.rs b/tests/rustdoc/where-clause-order.rs index b10f8f6856e8c..7261dfa7dd912 100644 --- a/tests/rustdoc/where-clause-order.rs +++ b/tests/rustdoc/where-clause-order.rs @@ -7,7 +7,7 @@ where } // @has 'foo/trait.SomeTrait.html' -// @has - "//*[@id='impl-SomeTrait%3C(A,+B,+C,+D,+E)%3E-for-(A,+B,+C,+D,+E)']/h3" "impl SomeTrait<(A, B, C, D, E)> for (A, B, C, D, E)where A: PartialOrd + PartialEq, B: PartialOrd + PartialEq, C: PartialOrd + PartialEq, D: PartialOrd + PartialEq, E: PartialOrd + PartialEq + ?Sized, " +// @has - "//*[@id='impl-SomeTrait-for-(A,+B,+C,+D,+E)']/h3" "impl SomeTrait<(A, B, C, D, E)> for (A, B, C, D, E)where A: PartialOrd + PartialEq, B: PartialOrd + PartialEq, C: PartialOrd + PartialEq, D: PartialOrd + PartialEq, E: PartialOrd + PartialEq + ?Sized, " impl SomeTrait<(A, B, C, D, E)> for (A, B, C, D, E) where A: PartialOrd + PartialEq, @@ -17,3 +17,14 @@ where E: PartialOrd + PartialEq + ?Sized, { } + +// @has - "//*[@id='impl-SomeTrait%3C(A,+B,+C,+D)%3E-for-(A,+B,+C,+D,+E)']/h3" "impl SomeTrait<(A, B, C, D)> for (A, B, C, D, E)where A: PartialOrd + PartialEq, B: PartialOrd + PartialEq, C: PartialOrd + PartialEq, D: PartialOrd + PartialEq, E: PartialOrd + PartialEq + ?Sized, " +impl SomeTrait<(A, B, C, D)> for (A, B, C, D, E) +where + A: PartialOrd + PartialEq, + B: PartialOrd + PartialEq, + C: PartialOrd + PartialEq, + D: PartialOrd + PartialEq, + E: PartialOrd + PartialEq + ?Sized, +{ +} From 20b93b951aaa75bd32ec1c1c63eac5dbccc35156 Mon Sep 17 00:00:00 2001 From: Michael Howell Date: Tue, 21 Mar 2023 10:38:24 -0700 Subject: [PATCH 04/15] rustdoc: wait for section to open before trying to highlight This fixes a problem where hash rewriting doesn't work with `:target` CSS rules. --- src/librustdoc/html/static/js/main.js | 8 ++++++-- tests/rustdoc-gui/search-result-impl-disambiguation.goml | 2 ++ 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/src/librustdoc/html/static/js/main.js b/src/librustdoc/html/static/js/main.js index 970c2f2d45d15..aa8fd7162eed1 100644 --- a/src/librustdoc/html/static/js/main.js +++ b/src/librustdoc/html/static/js/main.js @@ -370,8 +370,12 @@ function preLoadCss(cssUrl) { item => { const numbered = /([^-]+)-([0-9]+)/.exec(item.id); if (item.id === assocId || (numbered && numbered[1] === assocId)) { - expandSection(item.id); - window.location = "#" + item.id; + openParentDetails(item); + item.scrollIntoView(); + // Let the section expand itself before trying to highlight + setTimeout(() => { + window.location.replace("#" + item.id); + }, 0); } } ); diff --git a/tests/rustdoc-gui/search-result-impl-disambiguation.goml b/tests/rustdoc-gui/search-result-impl-disambiguation.goml index 98a2cd9577397..1596a3c4c6e7e 100644 --- a/tests/rustdoc-gui/search-result-impl-disambiguation.goml +++ b/tests/rustdoc-gui/search-result-impl-disambiguation.goml @@ -20,6 +20,7 @@ wait-for: "#impl-ZyxwvutMethodDisambiguation" assert-document-property: ({ "URL": "struct.ZyxwvutMethodDisambiguation.html#method.method_impl_disambiguation" }, ENDS_WITH) +assert: "section:target" goto: "file://" + |DOC_PATH| + "/test_docs/index.html" @@ -39,3 +40,4 @@ wait-for: "#impl-ZyxwvutMethodDisambiguation" assert-document-property: ({ "URL": "struct.ZyxwvutMethodDisambiguation.html#method.method_impl_disambiguation-1" }, ENDS_WITH) +assert: "section:target" From 2a4c9d07562c42950699609e32e77fbe9ceaa4e9 Mon Sep 17 00:00:00 2001 From: Michael Howell Date: Thu, 13 Jul 2023 10:53:21 -0700 Subject: [PATCH 05/15] Update search-result-impl-disambiguation.goml --- src/librustdoc/html/static/js/main.js | 4 ++-- tests/rustdoc-gui/search-result-impl-disambiguation.goml | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/librustdoc/html/static/js/main.js b/src/librustdoc/html/static/js/main.js index aa8fd7162eed1..43c4f2b6ff5bb 100644 --- a/src/librustdoc/html/static/js/main.js +++ b/src/librustdoc/html/static/js/main.js @@ -354,13 +354,13 @@ function preLoadCss(cssUrl) { expandSection(pageId); } } - if (savedHash.startsWith("#impl-")) { + if (savedHash.startsWith("impl-")) { // impl-disambiguated links, used by the search engine // format: impl-X[-for-Y]/method.WHATEVER // turn this into method.WHATEVER[-NUMBER] const splitAt = savedHash.indexOf("/"); if (splitAt !== -1) { - const implId = savedHash.slice(1, splitAt); + const implId = savedHash.slice(0, splitAt); const assocId = savedHash.slice(splitAt + 1); const implElem = document.getElementById(implId); if (implElem && implElem.parentElement.tagName === "SUMMARY" && diff --git a/tests/rustdoc-gui/search-result-impl-disambiguation.goml b/tests/rustdoc-gui/search-result-impl-disambiguation.goml index 1596a3c4c6e7e..6d12032e891b6 100644 --- a/tests/rustdoc-gui/search-result-impl-disambiguation.goml +++ b/tests/rustdoc-gui/search-result-impl-disambiguation.goml @@ -2,7 +2,7 @@ // Checks that, if a type has two methods with the same name, they both get // linked correctly. -goto: "file://" + |DOC_PATH| + "/test_docs/index.html" +go-to: "file://" + |DOC_PATH| + "/test_docs/index.html" // This should link to the inherent impl write: (".search-input", "ZyxwvutMethodDisambiguation -> bool") @@ -22,7 +22,7 @@ assert-document-property: ({ }, ENDS_WITH) assert: "section:target" -goto: "file://" + |DOC_PATH| + "/test_docs/index.html" +go-to: "file://" + |DOC_PATH| + "/test_docs/index.html" // This should link to the trait impl write: (".search-input", "ZyxwvutMethodDisambiguation, usize -> usize") From 7bb594f00ea563d19c23cb70382c66b35c182561 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Thu, 28 Sep 2023 22:03:39 +0000 Subject: [PATCH 06/15] On type error of closure call argument, point at earlier calls that affected inference Mitigate part of #71209. ``` error[E0308]: mismatched types --> $DIR/unboxed-closures-type-mismatch.rs:30:18 | LL | identity(1u16); | -------- ^^^^ expected `u8`, found `u16` | | | arguments to this function are incorrect | note: expected because the closure was earlier called with an argument of type `u8` --> $DIR/unboxed-closures-type-mismatch.rs:29:18 | LL | identity(1u8); | -------- ^^^ expected because this argument is of type `u8` | | | in this closure call note: closure parameter defined here --> $DIR/unboxed-closures-type-mismatch.rs:28:25 | LL | let identity = |x| x; | ^ help: change the type of the numeric literal from `u16` to `u8` | LL | identity(1u8); | ~~ ``` --- .../rustc_hir_typeck/src/fn_ctxt/checks.rs | 71 +++++++++- .../unboxed-closures-type-mismatch.rs | 32 ++++- .../unboxed-closures-type-mismatch.stderr | 123 +++++++++++++++++- 3 files changed, 221 insertions(+), 5 deletions(-) diff --git a/compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs b/compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs index c0332a48be3df..9b7f8f8031036 100644 --- a/compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs +++ b/compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs @@ -16,6 +16,7 @@ use rustc_errors::{ use rustc_hir as hir; use rustc_hir::def::{CtorOf, DefKind, Res}; use rustc_hir::def_id::DefId; +use rustc_hir::intravisit::Visitor; use rustc_hir::{ExprKind, Node, QPath}; use rustc_hir_analysis::astconv::AstConv; use rustc_hir_analysis::check::intrinsicck::InlineAsmCtxt; @@ -28,7 +29,7 @@ use rustc_infer::infer::TypeTrace; use rustc_infer::infer::{DefineOpaqueTypes, InferOk}; use rustc_middle::ty::adjustment::AllowTwoPhase; use rustc_middle::ty::visit::TypeVisitableExt; -use rustc_middle::ty::{self, IsSuggestable, Ty}; +use rustc_middle::ty::{self, IsSuggestable, Ty, TyCtxt}; use rustc_session::Session; use rustc_span::symbol::{kw, Ident}; use rustc_span::{self, sym, BytePos, Span}; @@ -722,6 +723,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { &mut err, fn_def_id, callee_ty, + call_expr, + None, Some(mismatch_idx), is_method, ); @@ -826,6 +829,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { &mut err, fn_def_id, callee_ty, + call_expr, + Some(expected_ty), Some(expected_idx.as_usize()), is_method, ); @@ -1208,7 +1213,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { } // Call out where the function is defined - self.label_fn_like(&mut err, fn_def_id, callee_ty, None, is_method); + self.label_fn_like(&mut err, fn_def_id, callee_ty, call_expr, None, None, is_method); // And add a suggestion block for all of the parameters let suggestion_text = match suggestion_text { @@ -1899,6 +1904,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { err: &mut Diagnostic, callable_def_id: Option, callee_ty: Option>, + call_expr: &'tcx hir::Expr<'tcx>, + expected_ty: Option>, // A specific argument should be labeled, instead of all of them expected_idx: Option, is_method: bool, @@ -2015,6 +2022,46 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { let param = expected_idx .and_then(|expected_idx| self.tcx.hir().body(*body).params.get(expected_idx)); let (kind, span) = if let Some(param) = param { + // Try to find earlier invocations of this closure to find if the type mismatch + // is because of inference. If we find one, point at them. + let mut call_finder = FindClosureArg { tcx: self.tcx, calls: vec![] }; + let node = self.tcx + .opt_local_def_id_to_hir_id(self.tcx.hir().get_parent_item(call_expr.hir_id)) + .and_then(|hir_id| self.tcx.hir().find(hir_id)); + match node { + Some(hir::Node::Item(item)) => call_finder.visit_item(item), + Some(hir::Node::TraitItem(item)) => call_finder.visit_trait_item(item), + Some(hir::Node::ImplItem(item)) => call_finder.visit_impl_item(item), + _ => {} + } + let typeck = self.typeck_results.borrow(); + for (rcvr, args) in call_finder.calls { + if let Some(rcvr_ty) = typeck.node_type_opt(rcvr.hir_id) + && let ty::Closure(call_def_id, _) = rcvr_ty.kind() + && def_id == *call_def_id + && let Some(idx) = expected_idx + && let Some(arg) = args.get(idx) + && let Some(arg_ty) = typeck.node_type_opt(arg.hir_id) + && let Some(expected_ty) = expected_ty + && self.can_eq(self.param_env, arg_ty, expected_ty) + { + let mut sp: MultiSpan = vec![arg.span].into(); + sp.push_span_label( + arg.span, + format!("expected because this argument is of type `{arg_ty}`"), + ); + sp.push_span_label(rcvr.span, "in this closure call"); + err.span_note( + sp, + format!( + "expected because the closure was earlier called with an \ + argument of type `{arg_ty}`", + ), + ); + break; + } + } + ("closure parameter", param.span) } else { ("closure", self.tcx.def_span(def_id)) @@ -2028,3 +2075,23 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { } } } + +struct FindClosureArg<'tcx> { + tcx: TyCtxt<'tcx>, + calls: Vec<(&'tcx hir::Expr<'tcx>, &'tcx [hir::Expr<'tcx>])>, +} + +impl<'tcx> Visitor<'tcx> for FindClosureArg<'tcx> { + type NestedFilter = rustc_middle::hir::nested_filter::All; + + fn nested_visit_map(&mut self) -> Self::Map { + self.tcx.hir() + } + + fn visit_expr(&mut self, ex: &'tcx hir::Expr<'tcx>) { + if let hir::ExprKind::Call(rcvr, args) = ex.kind { + self.calls.push((rcvr, args)); + } + hir::intravisit::walk_expr(self, ex); + } +} diff --git a/tests/ui/unboxed-closures/unboxed-closures-type-mismatch.rs b/tests/ui/unboxed-closures/unboxed-closures-type-mismatch.rs index 9f76849e5fbff..0999f61b01a1a 100644 --- a/tests/ui/unboxed-closures/unboxed-closures-type-mismatch.rs +++ b/tests/ui/unboxed-closures/unboxed-closures-type-mismatch.rs @@ -1,7 +1,35 @@ use std::ops::FnMut; -pub fn main() { +fn main() { let mut f = |x: isize, y: isize| -> isize { x + y }; - let z = f(1_usize, 2); //~ ERROR mismatched types + let z = f(1_usize, 2); //~ ERROR mismatched types println!("{}", z); + let mut g = |x, y| { x + y }; + let y = g(1_i32, 2); + let z = g(1_usize, 2); //~ ERROR mismatched types + println!("{}", z); +} + +trait T { + fn bar(&self) { + let identity = |x| x; + identity(1u8); + identity(1u16); //~ ERROR mismatched types + let identity = |x| x; + identity(&1u8); + identity(&1u16); //~ ERROR mismatched types + } +} + +struct S; + +impl T for S { + fn bar(&self) { + let identity = |x| x; + identity(1u8); + identity(1u16); //~ ERROR mismatched types + let identity = |x| x; + identity(&1u8); + identity(&1u16); //~ ERROR mismatched types + } } diff --git a/tests/ui/unboxed-closures/unboxed-closures-type-mismatch.stderr b/tests/ui/unboxed-closures/unboxed-closures-type-mismatch.stderr index 455f83f5721bd..327df50e645d5 100644 --- a/tests/ui/unboxed-closures/unboxed-closures-type-mismatch.stderr +++ b/tests/ui/unboxed-closures/unboxed-closures-type-mismatch.stderr @@ -16,6 +16,127 @@ help: change the type of the numeric literal from `usize` to `isize` LL | let z = f(1_isize, 2); | ~~~~~ -error: aborting due to previous error +error[E0308]: mismatched types + --> $DIR/unboxed-closures-type-mismatch.rs:9:15 + | +LL | let z = g(1_usize, 2); + | - ^^^^^^^ expected `i32`, found `usize` + | | + | arguments to this function are incorrect + | +note: expected because the closure was earlier called with an argument of type `i32` + --> $DIR/unboxed-closures-type-mismatch.rs:8:15 + | +LL | let y = g(1_i32, 2); + | - ^^^^^ expected because this argument is of type `i32` + | | + | in this closure call +note: closure parameter defined here + --> $DIR/unboxed-closures-type-mismatch.rs:7:18 + | +LL | let mut g = |x, y| { x + y }; + | ^ +help: change the type of the numeric literal from `usize` to `i32` + | +LL | let z = g(1_i32, 2); + | ~~~ + +error[E0308]: mismatched types + --> $DIR/unboxed-closures-type-mismatch.rs:17:18 + | +LL | identity(1u16); + | -------- ^^^^ expected `u8`, found `u16` + | | + | arguments to this function are incorrect + | +note: expected because the closure was earlier called with an argument of type `u8` + --> $DIR/unboxed-closures-type-mismatch.rs:16:18 + | +LL | identity(1u8); + | -------- ^^^ expected because this argument is of type `u8` + | | + | in this closure call +note: closure parameter defined here + --> $DIR/unboxed-closures-type-mismatch.rs:15:25 + | +LL | let identity = |x| x; + | ^ +help: change the type of the numeric literal from `u16` to `u8` + | +LL | identity(1u8); + | ~~ + +error[E0308]: mismatched types + --> $DIR/unboxed-closures-type-mismatch.rs:20:18 + | +LL | identity(&1u16); + | -------- ^^^^^ expected `&u8`, found `&u16` + | | + | arguments to this function are incorrect + | + = note: expected reference `&u8` + found reference `&u16` +note: expected because the closure was earlier called with an argument of type `&u8` + --> $DIR/unboxed-closures-type-mismatch.rs:19:18 + | +LL | identity(&1u8); + | -------- ^^^^ expected because this argument is of type `&u8` + | | + | in this closure call +note: closure parameter defined here + --> $DIR/unboxed-closures-type-mismatch.rs:18:25 + | +LL | let identity = |x| x; + | ^ + +error[E0308]: mismatched types + --> $DIR/unboxed-closures-type-mismatch.rs:30:18 + | +LL | identity(1u16); + | -------- ^^^^ expected `u8`, found `u16` + | | + | arguments to this function are incorrect + | +note: expected because the closure was earlier called with an argument of type `u8` + --> $DIR/unboxed-closures-type-mismatch.rs:29:18 + | +LL | identity(1u8); + | -------- ^^^ expected because this argument is of type `u8` + | | + | in this closure call +note: closure parameter defined here + --> $DIR/unboxed-closures-type-mismatch.rs:28:25 + | +LL | let identity = |x| x; + | ^ +help: change the type of the numeric literal from `u16` to `u8` + | +LL | identity(1u8); + | ~~ + +error[E0308]: mismatched types + --> $DIR/unboxed-closures-type-mismatch.rs:33:18 + | +LL | identity(&1u16); + | -------- ^^^^^ expected `&u8`, found `&u16` + | | + | arguments to this function are incorrect + | + = note: expected reference `&u8` + found reference `&u16` +note: expected because the closure was earlier called with an argument of type `&u8` + --> $DIR/unboxed-closures-type-mismatch.rs:32:18 + | +LL | identity(&1u8); + | -------- ^^^^ expected because this argument is of type `&u8` + | | + | in this closure call +note: closure parameter defined here + --> $DIR/unboxed-closures-type-mismatch.rs:31:25 + | +LL | let identity = |x| x; + | ^ + +error: aborting due to 6 previous errors For more information about this error, try `rustc --explain E0308`. From 8c7d232568d80bd365ef1c17e919f7fc637962d1 Mon Sep 17 00:00:00 2001 From: Eric Huss Date: Fri, 6 Oct 2023 18:26:34 -0700 Subject: [PATCH 07/15] Update docs for mips target tier demotion. --- RELEASES.md | 1 + src/doc/rustc/src/platform-support.md | 8 ++++---- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/RELEASES.md b/RELEASES.md index e261294a032fd..1cc110e6607f9 100644 --- a/RELEASES.md +++ b/RELEASES.md @@ -229,6 +229,7 @@ Compatibility Notes this should only impact users of other registries, or people who don't publish to a registry. [#12291](https://github.com/rust-lang/cargo/pull/12291) +- [Demoted `mips*-unknown-linux-gnu*` targets from host tier 2 to target tier 3 support.](https://github.com/rust-lang/rust/pull/113274) Version 1.71.1 (2023-08-03) =========================== diff --git a/src/doc/rustc/src/platform-support.md b/src/doc/rustc/src/platform-support.md index ef4eb5a196576..e5b713c536741 100644 --- a/src/doc/rustc/src/platform-support.md +++ b/src/doc/rustc/src/platform-support.md @@ -93,10 +93,6 @@ target | notes `arm-unknown-linux-gnueabihf` | ARMv6 Linux, hardfloat (kernel 3.2, glibc 2.17) `armv7-unknown-linux-gnueabihf` | ARMv7-A Linux, hardfloat (kernel 3.2, glibc 2.17) [`loongarch64-unknown-linux-gnu`](platform-support/loongarch-linux.md) | LoongArch64 Linux, LP64D ABI (kernel 5.19, glibc 2.36) -`mips-unknown-linux-gnu` | MIPS Linux (kernel 4.4, glibc 2.23) -`mips64-unknown-linux-gnuabi64` | MIPS64 Linux, n64 ABI (kernel 4.4, glibc 2.23) -`mips64el-unknown-linux-gnuabi64` | MIPS64 (LE) Linux, n64 ABI (kernel 4.4, glibc 2.23) -`mipsel-unknown-linux-gnu` | MIPS (LE) Linux (kernel 4.4, glibc 2.23) `powerpc-unknown-linux-gnu` | PowerPC Linux (kernel 3.2, glibc 2.17) `powerpc64-unknown-linux-gnu` | PPC64 Linux (kernel 3.2, glibc 2.17) `powerpc64le-unknown-linux-gnu` | PPC64LE Linux (kernel 3.10, glibc 2.17) @@ -286,6 +282,10 @@ target | std | host | notes [`mips64-openwrt-linux-musl`](platform-support/mips64-openwrt-linux-musl.md) | ? | | MIPS64 for OpenWrt Linux MUSL `mipsel-sony-psp` | * | | MIPS (LE) Sony PlayStation Portable (PSP) [`mipsel-sony-psx`](platform-support/mipsel-sony-psx.md) | * | | MIPS (LE) Sony PlayStation 1 (PSX) +`mips-unknown-linux-gnu` | MIPS Linux (kernel 4.4, glibc 2.23) +`mips64-unknown-linux-gnuabi64` | MIPS64 Linux, n64 ABI (kernel 4.4, glibc 2.23) +`mips64el-unknown-linux-gnuabi64` | MIPS64 (LE) Linux, n64 ABI (kernel 4.4, glibc 2.23) +`mipsel-unknown-linux-gnu` | MIPS (LE) Linux (kernel 4.4, glibc 2.23) `mipsel-unknown-linux-uclibc` | ✓ | | MIPS (LE) Linux with uClibc `mipsel-unknown-none` | * | | Bare MIPS (LE) softfloat [`mipsisa32r6-unknown-linux-gnu`](platform-support/mips-release-6.md) | ? | | 32-bit MIPS Release 6 Big Endian From 5e1b0cbe5d0c86601b70bd8cc883227421c40cac Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Thu, 5 Oct 2023 10:02:07 +0200 Subject: [PATCH 08/15] add test for const-eval error in dead code during monomorphization --- .../const-eval/unused-broken-const-late.rs | 20 +++++++++++++++++++ .../unused-broken-const-late.stderr | 11 ++++++++++ 2 files changed, 31 insertions(+) create mode 100644 tests/ui/consts/const-eval/unused-broken-const-late.rs create mode 100644 tests/ui/consts/const-eval/unused-broken-const-late.stderr diff --git a/tests/ui/consts/const-eval/unused-broken-const-late.rs b/tests/ui/consts/const-eval/unused-broken-const-late.rs new file mode 100644 index 0000000000000..a6528ec5fd6a3 --- /dev/null +++ b/tests/ui/consts/const-eval/unused-broken-const-late.rs @@ -0,0 +1,20 @@ +// build-fail +// compile-flags: -O +//! Make sure we detect erroneous constants post-monomorphization even when they are unused. This is +//! crucial, people rely on it for soundness. (https://github.com/rust-lang/rust/issues/112090) + +struct PrintName(T); +impl PrintName { + const VOID: () = panic!(); //~ERROR evaluation of `PrintName::::VOID` failed +} + +fn no_codegen() { + // Any function that is called is guaranteed to have all consts that syntactically + // appear in its body evaluated, even if they only appear in dead code. + if false { + let _ = PrintName::::VOID; + } +} +pub fn main() { + no_codegen::(); +} diff --git a/tests/ui/consts/const-eval/unused-broken-const-late.stderr b/tests/ui/consts/const-eval/unused-broken-const-late.stderr new file mode 100644 index 0000000000000..cdb70a69dfd8a --- /dev/null +++ b/tests/ui/consts/const-eval/unused-broken-const-late.stderr @@ -0,0 +1,11 @@ +error[E0080]: evaluation of `PrintName::::VOID` failed + --> $DIR/unused-broken-const-late.rs:8:22 + | +LL | const VOID: () = panic!(); + | ^^^^^^^^ the evaluated program panicked at 'explicit panic', $DIR/unused-broken-const-late.rs:8:22 + | + = note: this error originates in the macro `$crate::panic::panic_2015` which comes from the expansion of the macro `panic` (in Nightly builds, run with -Z macro-backtrace for more info) + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0080`. From bbc230478c5a282a83592830dc338407ed9804eb Mon Sep 17 00:00:00 2001 From: Sven Bartscher Date: Mon, 9 Oct 2023 11:08:48 +0200 Subject: [PATCH 09/15] Make BTreeMap::new_in const Closes rust-lang/wg-allocators#118 --- library/alloc/src/collections/btree/map.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/alloc/src/collections/btree/map.rs b/library/alloc/src/collections/btree/map.rs index 5481b327d691d..4bdd9639557d4 100644 --- a/library/alloc/src/collections/btree/map.rs +++ b/library/alloc/src/collections/btree/map.rs @@ -669,7 +669,7 @@ impl BTreeMap { /// map.insert(1, "a"); /// ``` #[unstable(feature = "btreemap_alloc", issue = "32838")] - pub fn new_in(alloc: A) -> BTreeMap { + pub const fn new_in(alloc: A) -> BTreeMap { BTreeMap { root: None, length: 0, alloc: ManuallyDrop::new(alloc), _marker: PhantomData } } } From d60b43c06aad0b181b72ba5ed389961436ac1609 Mon Sep 17 00:00:00 2001 From: Sven Bartscher Date: Mon, 9 Oct 2023 11:17:56 +0200 Subject: [PATCH 10/15] Make BTreeSet::new_in const --- library/alloc/src/collections/btree/set.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/alloc/src/collections/btree/set.rs b/library/alloc/src/collections/btree/set.rs index 9da230915b822..0e03551286e8a 100644 --- a/library/alloc/src/collections/btree/set.rs +++ b/library/alloc/src/collections/btree/set.rs @@ -358,7 +358,7 @@ impl BTreeSet { /// let mut set: BTreeSet = BTreeSet::new_in(Global); /// ``` #[unstable(feature = "btreemap_alloc", issue = "32838")] - pub fn new_in(alloc: A) -> BTreeSet { + pub const fn new_in(alloc: A) -> BTreeSet { BTreeSet { map: BTreeMap::new_in(alloc) } } From 0f27c1b5b550deef02c3916fa81ea3c6edf8145e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?O=C4=9Fuz=20A=C4=9Fcayaz=C4=B1?= Date: Mon, 9 Oct 2023 12:56:14 +0300 Subject: [PATCH 11/15] defids are indexmapped --- Cargo.lock | 1 + compiler/rustc_smir/Cargo.toml | 1 + compiler/rustc_smir/src/rustc_internal/mod.rs | 25 +++++++++++-------- compiler/rustc_smir/src/rustc_smir/mod.rs | 3 ++- 4 files changed, 19 insertions(+), 11 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 082bb4be93c0a..2882ec0682e2b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4504,6 +4504,7 @@ dependencies = [ name = "rustc_smir" version = "0.0.0" dependencies = [ + "rustc_data_structures", "rustc_driver", "rustc_hir", "rustc_interface", diff --git a/compiler/rustc_smir/Cargo.toml b/compiler/rustc_smir/Cargo.toml index 3e0d6baab6aab..2b77044d6bfc7 100644 --- a/compiler/rustc_smir/Cargo.toml +++ b/compiler/rustc_smir/Cargo.toml @@ -4,6 +4,7 @@ version = "0.0.0" edition = "2021" [dependencies] +rustc_data_structures = { path = "../rustc_data_structures" } rustc_driver = { path = "../rustc_driver" } rustc_hir = { path = "../rustc_hir" } rustc_interface = { path = "../rustc_interface" } diff --git a/compiler/rustc_smir/src/rustc_internal/mod.rs b/compiler/rustc_smir/src/rustc_internal/mod.rs index 36eb2247253a0..2f4bc5a0499d4 100644 --- a/compiler/rustc_smir/src/rustc_internal/mod.rs +++ b/compiler/rustc_smir/src/rustc_internal/mod.rs @@ -7,6 +7,7 @@ use std::ops::{ControlFlow, Index}; use crate::rustc_internal; use crate::rustc_smir::Tables; +use rustc_data_structures::fx; use rustc_driver::{Callbacks, Compilation, RunCompiler}; use rustc_interface::{interface, Queries}; use rustc_middle::mir::interpret::AllocId; @@ -20,7 +21,7 @@ impl<'tcx> Index for Tables<'tcx> { #[inline(always)] fn index(&self, index: stable_mir::DefId) -> &Self::Output { - &self.def_ids[index.0] + &self.def_ids.get_index(index.0).unwrap().0 } } @@ -95,15 +96,13 @@ impl<'tcx> Tables<'tcx> { } fn create_def_id(&mut self, did: DefId) -> stable_mir::DefId { - // FIXME: this becomes inefficient when we have too many ids - for (i, &d) in self.def_ids.iter().enumerate() { - if d == did { - return stable_mir::DefId(i); - } + if let Some(i) = self.def_ids.get(&did) { + return *i; + } else { + let id = self.def_ids.len(); + self.def_ids.insert(did, stable_mir::DefId(id)); + stable_mir::DefId(id) } - let id = self.def_ids.len(); - self.def_ids.push(did); - stable_mir::DefId(id) } fn create_alloc_id(&mut self, aid: AllocId) -> stable_mir::AllocId { @@ -134,7 +133,13 @@ pub fn crate_num(item: &stable_mir::Crate) -> CrateNum { pub fn run(tcx: TyCtxt<'_>, f: impl FnOnce()) { stable_mir::run( - Tables { tcx, def_ids: vec![], alloc_ids: vec![], spans: vec![], types: vec![] }, + Tables { + tcx, + def_ids: fx::FxIndexMap::default(), + alloc_ids: vec![], + spans: vec![], + types: vec![], + }, f, ); } diff --git a/compiler/rustc_smir/src/rustc_smir/mod.rs b/compiler/rustc_smir/src/rustc_smir/mod.rs index 7d1122c2fd21d..144f1625b04ee 100644 --- a/compiler/rustc_smir/src/rustc_smir/mod.rs +++ b/compiler/rustc_smir/src/rustc_smir/mod.rs @@ -9,6 +9,7 @@ use crate::rustc_smir::hir::def::DefKind; use crate::rustc_smir::stable_mir::ty::{BoundRegion, EarlyBoundRegion, Region}; +use rustc_data_structures::fx::FxIndexMap; use rustc_hir as hir; use rustc_middle::mir; use rustc_middle::mir::interpret::{alloc_range, AllocId}; @@ -194,7 +195,7 @@ impl PartialEq for MaybeStable { pub struct Tables<'tcx> { pub tcx: TyCtxt<'tcx>, - pub def_ids: Vec, + pub def_ids: FxIndexMap, pub alloc_ids: Vec, pub spans: Vec, pub types: Vec>>, From 5f079dd2ffd0d09a3f2f9250c7d302efff1df4d3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?O=C4=9Fuz=20A=C4=9Fcayaz=C4=B1?= Date: Mon, 9 Oct 2023 12:58:41 +0300 Subject: [PATCH 12/15] alloc id is indexmapped --- compiler/rustc_smir/src/rustc_internal/mod.rs | 15 ++++++++------- compiler/rustc_smir/src/rustc_smir/mod.rs | 2 +- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/compiler/rustc_smir/src/rustc_internal/mod.rs b/compiler/rustc_smir/src/rustc_internal/mod.rs index 2f4bc5a0499d4..d0515df9ad547 100644 --- a/compiler/rustc_smir/src/rustc_internal/mod.rs +++ b/compiler/rustc_smir/src/rustc_internal/mod.rs @@ -107,12 +107,13 @@ impl<'tcx> Tables<'tcx> { fn create_alloc_id(&mut self, aid: AllocId) -> stable_mir::AllocId { // FIXME: this becomes inefficient when we have too many ids - if let Some(i) = self.alloc_ids.iter().position(|a| *a == aid) { - return stable_mir::AllocId(i); - }; - let id = self.def_ids.len(); - self.alloc_ids.push(aid); - stable_mir::AllocId(id) + if let Some(i) = self.alloc_ids.get(&aid) { + return *i; + } else { + let id = self.def_ids.len(); + self.alloc_ids.insert(aid, stable_mir::AllocId(id)); + stable_mir::AllocId(id) + } } pub(crate) fn create_span(&mut self, span: Span) -> stable_mir::ty::Span { @@ -136,7 +137,7 @@ pub fn run(tcx: TyCtxt<'_>, f: impl FnOnce()) { Tables { tcx, def_ids: fx::FxIndexMap::default(), - alloc_ids: vec![], + alloc_ids: fx::FxIndexMap::default(), spans: vec![], types: vec![], }, diff --git a/compiler/rustc_smir/src/rustc_smir/mod.rs b/compiler/rustc_smir/src/rustc_smir/mod.rs index 144f1625b04ee..42688613a61be 100644 --- a/compiler/rustc_smir/src/rustc_smir/mod.rs +++ b/compiler/rustc_smir/src/rustc_smir/mod.rs @@ -196,7 +196,7 @@ impl PartialEq for MaybeStable { pub struct Tables<'tcx> { pub tcx: TyCtxt<'tcx>, pub def_ids: FxIndexMap, - pub alloc_ids: Vec, + pub alloc_ids: FxIndexMap, pub spans: Vec, pub types: Vec>>, } From 77df2cd9a5cbb461a0fd430f7d36f2e181a0386a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?O=C4=9Fuz=20A=C4=9Fcayaz=C4=B1?= Date: Mon, 9 Oct 2023 13:03:58 +0300 Subject: [PATCH 13/15] spans are now indexmapped --- compiler/rustc_smir/src/rustc_internal/mod.rs | 18 ++++++++---------- compiler/rustc_smir/src/rustc_smir/mod.rs | 2 +- 2 files changed, 9 insertions(+), 11 deletions(-) diff --git a/compiler/rustc_smir/src/rustc_internal/mod.rs b/compiler/rustc_smir/src/rustc_internal/mod.rs index d0515df9ad547..25c7bd748eebd 100644 --- a/compiler/rustc_smir/src/rustc_internal/mod.rs +++ b/compiler/rustc_smir/src/rustc_internal/mod.rs @@ -30,7 +30,7 @@ impl<'tcx> Index for Tables<'tcx> { #[inline(always)] fn index(&self, index: stable_mir::ty::Span) -> &Self::Output { - &self.spans[index.0] + &self.spans.get_index(index.0).unwrap().0 } } @@ -106,7 +106,6 @@ impl<'tcx> Tables<'tcx> { } fn create_alloc_id(&mut self, aid: AllocId) -> stable_mir::AllocId { - // FIXME: this becomes inefficient when we have too many ids if let Some(i) = self.alloc_ids.get(&aid) { return *i; } else { @@ -117,14 +116,13 @@ impl<'tcx> Tables<'tcx> { } pub(crate) fn create_span(&mut self, span: Span) -> stable_mir::ty::Span { - for (i, &sp) in self.spans.iter().enumerate() { - if sp == span { - return stable_mir::ty::Span(i); - } + if let Some(i) = self.spans.get(&span) { + return *i; + } else { + let id = self.spans.len(); + self.spans.insert(span, stable_mir::ty::Span(id)); + stable_mir::ty::Span(id) } - let id = self.spans.len(); - self.spans.push(span); - stable_mir::ty::Span(id) } } @@ -138,7 +136,7 @@ pub fn run(tcx: TyCtxt<'_>, f: impl FnOnce()) { tcx, def_ids: fx::FxIndexMap::default(), alloc_ids: fx::FxIndexMap::default(), - spans: vec![], + spans: fx::FxIndexMap::default(), types: vec![], }, f, diff --git a/compiler/rustc_smir/src/rustc_smir/mod.rs b/compiler/rustc_smir/src/rustc_smir/mod.rs index 42688613a61be..757cfa82d3c11 100644 --- a/compiler/rustc_smir/src/rustc_smir/mod.rs +++ b/compiler/rustc_smir/src/rustc_smir/mod.rs @@ -197,7 +197,7 @@ pub struct Tables<'tcx> { pub tcx: TyCtxt<'tcx>, pub def_ids: FxIndexMap, pub alloc_ids: FxIndexMap, - pub spans: Vec, + pub spans: FxIndexMap, pub types: Vec>>, } From 7962b9641a109da50c4e9960eecd81c0bfd55084 Mon Sep 17 00:00:00 2001 From: rustbot <47979223+rustbot@users.noreply.github.com> Date: Mon, 9 Oct 2023 13:00:35 -0400 Subject: [PATCH 14/15] Update books --- src/doc/reference | 2 +- src/doc/rust-by-example | 2 +- src/doc/rustc-dev-guide | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/doc/reference b/src/doc/reference index 5262e1c3b43a2..142b2ed77d33f 160000 --- a/src/doc/reference +++ b/src/doc/reference @@ -1 +1 @@ -Subproject commit 5262e1c3b43a2c489df8f6717683a44c7a2260fd +Subproject commit 142b2ed77d33f37a9973772bd95e6144ed9dce43 diff --git a/src/doc/rust-by-example b/src/doc/rust-by-example index c954202c1e172..8eb3a01ab74c5 160000 --- a/src/doc/rust-by-example +++ b/src/doc/rust-by-example @@ -1 +1 @@ -Subproject commit c954202c1e1720cba5628f99543cc01188c7d6fc +Subproject commit 8eb3a01ab74c567b7174784892fb807f2c632d6b diff --git a/src/doc/rustc-dev-guide b/src/doc/rustc-dev-guide index a13b7c28ed705..b98af7d661e47 160000 --- a/src/doc/rustc-dev-guide +++ b/src/doc/rustc-dev-guide @@ -1 +1 @@ -Subproject commit a13b7c28ed705891c681ce5417b3d1cdb12cecd1 +Subproject commit b98af7d661e4744baab81fb8dc7a049e44a4a998 From 0bcb058fb13242bf6804e66413315a784484ab09 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?O=C4=9Fuz=20A=C4=9Fcayaz=C4=B1?= Date: Tue, 10 Oct 2023 11:28:45 +0300 Subject: [PATCH 15/15] add new wrapper for FxIndexMap --- compiler/rustc_smir/src/rustc_internal/mod.rs | 66 +++++++++++-------- compiler/rustc_smir/src/rustc_smir/mod.rs | 8 +-- compiler/stable_mir/src/lib.rs | 26 +++++++- compiler/stable_mir/src/ty.rs | 17 ++++- 4 files changed, 81 insertions(+), 36 deletions(-) diff --git a/compiler/rustc_smir/src/rustc_internal/mod.rs b/compiler/rustc_smir/src/rustc_internal/mod.rs index 25c7bd748eebd..e3c84f06543ed 100644 --- a/compiler/rustc_smir/src/rustc_internal/mod.rs +++ b/compiler/rustc_smir/src/rustc_internal/mod.rs @@ -3,8 +3,6 @@ //! For that, we define APIs that will temporarily be public to 3P that exposes rustc internal APIs //! until stable MIR is complete. -use std::ops::{ControlFlow, Index}; - use crate::rustc_internal; use crate::rustc_smir::Tables; use rustc_data_structures::fx; @@ -14,14 +12,18 @@ use rustc_middle::mir::interpret::AllocId; use rustc_middle::ty::TyCtxt; use rustc_span::def_id::{CrateNum, DefId}; use rustc_span::Span; +use stable_mir::ty::IndexedVal; use stable_mir::CompilerError; +use std::fmt::Debug; +use std::hash::Hash; +use std::ops::{ControlFlow, Index}; impl<'tcx> Index for Tables<'tcx> { type Output = DefId; #[inline(always)] fn index(&self, index: stable_mir::DefId) -> &Self::Output { - &self.def_ids.get_index(index.0).unwrap().0 + &self.def_ids[index] } } @@ -30,7 +32,7 @@ impl<'tcx> Index for Tables<'tcx> { #[inline(always)] fn index(&self, index: stable_mir::ty::Span) -> &Self::Output { - &self.spans.get_index(index.0).unwrap().0 + &self.spans[index] } } @@ -96,33 +98,15 @@ impl<'tcx> Tables<'tcx> { } fn create_def_id(&mut self, did: DefId) -> stable_mir::DefId { - if let Some(i) = self.def_ids.get(&did) { - return *i; - } else { - let id = self.def_ids.len(); - self.def_ids.insert(did, stable_mir::DefId(id)); - stable_mir::DefId(id) - } + self.def_ids.create_or_fetch(did) } fn create_alloc_id(&mut self, aid: AllocId) -> stable_mir::AllocId { - if let Some(i) = self.alloc_ids.get(&aid) { - return *i; - } else { - let id = self.def_ids.len(); - self.alloc_ids.insert(aid, stable_mir::AllocId(id)); - stable_mir::AllocId(id) - } + self.alloc_ids.create_or_fetch(aid) } pub(crate) fn create_span(&mut self, span: Span) -> stable_mir::ty::Span { - if let Some(i) = self.spans.get(&span) { - return *i; - } else { - let id = self.spans.len(); - self.spans.insert(span, stable_mir::ty::Span(id)); - stable_mir::ty::Span(id) - } + self.spans.create_or_fetch(span) } } @@ -134,9 +118,9 @@ pub fn run(tcx: TyCtxt<'_>, f: impl FnOnce()) { stable_mir::run( Tables { tcx, - def_ids: fx::FxIndexMap::default(), - alloc_ids: fx::FxIndexMap::default(), - spans: fx::FxIndexMap::default(), + def_ids: rustc_internal::IndexMap { index_map: fx::FxIndexMap::default() }, + alloc_ids: rustc_internal::IndexMap { index_map: fx::FxIndexMap::default() }, + spans: rustc_internal::IndexMap { index_map: fx::FxIndexMap::default() }, types: vec![], }, f, @@ -201,3 +185,29 @@ where }) } } + +/// Simmilar to rustc's `FxIndexMap`, `IndexMap` with extra +/// safety features added. +pub struct IndexMap { + index_map: fx::FxIndexMap, +} + +impl IndexMap { + pub fn create_or_fetch(&mut self, key: K) -> V { + let len = self.index_map.len(); + let v = self.index_map.entry(key).or_insert(V::to_val(len)); + *v + } +} + +impl Index + for IndexMap +{ + type Output = K; + + fn index(&self, index: V) -> &Self::Output { + let (k, v) = self.index_map.get_index(index.to_index()).unwrap(); + assert_eq!(*v, index, "Provided value doesn't match with indexed value"); + k + } +} diff --git a/compiler/rustc_smir/src/rustc_smir/mod.rs b/compiler/rustc_smir/src/rustc_smir/mod.rs index 757cfa82d3c11..09dc7475ec86b 100644 --- a/compiler/rustc_smir/src/rustc_smir/mod.rs +++ b/compiler/rustc_smir/src/rustc_smir/mod.rs @@ -7,9 +7,9 @@ //! //! For now, we are developing everything inside `rustc`, thus, we keep this module private. +use crate::rustc_internal::IndexMap; use crate::rustc_smir::hir::def::DefKind; use crate::rustc_smir::stable_mir::ty::{BoundRegion, EarlyBoundRegion, Region}; -use rustc_data_structures::fx::FxIndexMap; use rustc_hir as hir; use rustc_middle::mir; use rustc_middle::mir::interpret::{alloc_range, AllocId}; @@ -195,9 +195,9 @@ impl PartialEq for MaybeStable { pub struct Tables<'tcx> { pub tcx: TyCtxt<'tcx>, - pub def_ids: FxIndexMap, - pub alloc_ids: FxIndexMap, - pub spans: FxIndexMap, + pub def_ids: IndexMap, + pub alloc_ids: IndexMap, + pub spans: IndexMap, pub types: Vec>>, } diff --git a/compiler/stable_mir/src/lib.rs b/compiler/stable_mir/src/lib.rs index 104985493efc1..0a5c62eee04a9 100644 --- a/compiler/stable_mir/src/lib.rs +++ b/compiler/stable_mir/src/lib.rs @@ -22,7 +22,8 @@ use std::fmt; use std::fmt::Debug; use self::ty::{ - GenericPredicates, Generics, ImplDef, ImplTrait, Span, TraitDecl, TraitDef, Ty, TyKind, + GenericPredicates, Generics, ImplDef, ImplTrait, IndexedVal, Span, TraitDecl, TraitDef, Ty, + TyKind, }; #[macro_use] @@ -41,7 +42,7 @@ pub type CrateNum = usize; /// A unique identification number for each item accessible for the current compilation unit. #[derive(Clone, Copy, PartialEq, Eq)] -pub struct DefId(pub usize); +pub struct DefId(usize); impl Debug for DefId { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { @@ -52,9 +53,28 @@ impl Debug for DefId { } } +impl IndexedVal for DefId { + fn to_val(index: usize) -> Self { + DefId(index) + } + + fn to_index(&self) -> usize { + self.0 + } +} + /// A unique identification number for each provenance #[derive(Clone, Copy, PartialEq, Eq, Debug)] -pub struct AllocId(pub usize); +pub struct AllocId(usize); + +impl IndexedVal for AllocId { + fn to_val(index: usize) -> Self { + AllocId(index) + } + fn to_index(&self) -> usize { + self.0 + } +} /// A list of crate items. pub type CrateItems = Vec; diff --git a/compiler/stable_mir/src/ty.rs b/compiler/stable_mir/src/ty.rs index 6029e3c11adef..691af15da8ca8 100644 --- a/compiler/stable_mir/src/ty.rs +++ b/compiler/stable_mir/src/ty.rs @@ -75,7 +75,7 @@ pub struct Placeholder { } #[derive(Clone, Copy, PartialEq, Eq)] -pub struct Span(pub usize); +pub struct Span(usize); impl Debug for Span { fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { @@ -86,6 +86,15 @@ impl Debug for Span { } } +impl IndexedVal for Span { + fn to_val(index: usize) -> Self { + Span(index) + } + fn to_index(&self) -> usize { + self.0 + } +} + #[derive(Clone, Debug)] pub enum TyKind { RigidTy(RigidTy), @@ -565,3 +574,9 @@ pub enum ImplPolarity { Negative, Reservation, } + +pub trait IndexedVal { + fn to_val(index: usize) -> Self; + + fn to_index(&self) -> usize; +}