From 786e0bb1dcfdb9deee31ae7b181692467191ea2a Mon Sep 17 00:00:00 2001 From: Xi Ruoyao Date: Sat, 30 Dec 2023 02:54:40 +0800 Subject: [PATCH 01/23] bootstrap: Move -Clto= setting from Rustc::run to rustc_cargo It prevents a full rebuild of stage 1 compiler when issuing "x.py test" with rust.lto != thin-local in config.toml. --- src/bootstrap/src/core/build_steps/compile.rs | 56 +++++++++---------- 1 file changed, 28 insertions(+), 28 deletions(-) diff --git a/src/bootstrap/src/core/build_steps/compile.rs b/src/bootstrap/src/core/build_steps/compile.rs index dbb64583d561c..8e1f3b03d7863 100644 --- a/src/bootstrap/src/core/build_steps/compile.rs +++ b/src/bootstrap/src/core/build_steps/compile.rs @@ -905,34 +905,6 @@ impl Step for Rustc { )); } - // We currently don't support cross-crate LTO in stage0. This also isn't hugely necessary - // and may just be a time sink. - if compiler.stage != 0 { - match builder.config.rust_lto { - RustcLto::Thin | RustcLto::Fat => { - // Since using LTO for optimizing dylibs is currently experimental, - // we need to pass -Zdylib-lto. - cargo.rustflag("-Zdylib-lto"); - // Cargo by default passes `-Cembed-bitcode=no` and doesn't pass `-Clto` when - // compiling dylibs (and their dependencies), even when LTO is enabled for the - // crate. Therefore, we need to override `-Clto` and `-Cembed-bitcode` here. - let lto_type = match builder.config.rust_lto { - RustcLto::Thin => "thin", - RustcLto::Fat => "fat", - _ => unreachable!(), - }; - cargo.rustflag(&format!("-Clto={lto_type}")); - cargo.rustflag("-Cembed-bitcode=yes"); - } - RustcLto::ThinLocal => { /* Do nothing, this is the default */ } - RustcLto::Off => { - cargo.rustflag("-Clto=off"); - } - } - } else if builder.config.rust_lto == RustcLto::Off { - cargo.rustflag("-Clto=off"); - } - for krate in &*self.crates { cargo.arg("-p").arg(krate); } @@ -989,6 +961,34 @@ pub fn rustc_cargo(builder: &Builder<'_>, cargo: &mut Cargo, target: TargetSelec cargo.rustdocflag("-Zcrate-attr=warn(rust_2018_idioms)"); + // We currently don't support cross-crate LTO in stage0. This also isn't hugely necessary + // and may just be a time sink. + if stage != 0 { + match builder.config.rust_lto { + RustcLto::Thin | RustcLto::Fat => { + // Since using LTO for optimizing dylibs is currently experimental, + // we need to pass -Zdylib-lto. + cargo.rustflag("-Zdylib-lto"); + // Cargo by default passes `-Cembed-bitcode=no` and doesn't pass `-Clto` when + // compiling dylibs (and their dependencies), even when LTO is enabled for the + // crate. Therefore, we need to override `-Clto` and `-Cembed-bitcode` here. + let lto_type = match builder.config.rust_lto { + RustcLto::Thin => "thin", + RustcLto::Fat => "fat", + _ => unreachable!(), + }; + cargo.rustflag(&format!("-Clto={lto_type}")); + cargo.rustflag("-Cembed-bitcode=yes"); + } + RustcLto::ThinLocal => { /* Do nothing, this is the default */ } + RustcLto::Off => { + cargo.rustflag("-Clto=off"); + } + } + } else if builder.config.rust_lto == RustcLto::Off { + cargo.rustflag("-Clto=off"); + } + rustc_cargo_env(builder, cargo, target, stage); } From 1ab60f2a646e119db42a192771a4cdbc294fd5ff Mon Sep 17 00:00:00 2001 From: Michael Howell Date: Sat, 30 Dec 2023 17:27:20 -0700 Subject: [PATCH 02/23] rustdoc-search: fix inaccurate type descriptions --- src/librustdoc/html/static/js/externs.js | 56 ++++++++++++++++++++++ src/librustdoc/html/static/js/search.js | 61 +++--------------------- 2 files changed, 63 insertions(+), 54 deletions(-) diff --git a/src/librustdoc/html/static/js/externs.js b/src/librustdoc/html/static/js/externs.js index 93709e4e830ad..d24148b9556b9 100644 --- a/src/librustdoc/html/static/js/externs.js +++ b/src/librustdoc/html/static/js/externs.js @@ -200,3 +200,59 @@ let FunctionSearchType; * }} */ let FunctionType; + +/** + * The raw search data for a given crate. `n`, `t`, `d`, `i`, and `f` + * are arrays with the same length. `q`, `a`, and `c` use a sparse + * representation for compactness. + * + * `n[i]` contains the name of an item. + * + * `t[i]` contains the type of that item + * (as a string of characters that represent an offset in `itemTypes`). + * + * `d[i]` contains the description of that item. + * + * `q` contains the full paths of the items. For compactness, it is a set of + * (index, path) pairs used to create a map. If a given index `i` is + * not present, this indicates "same as the last index present". + * + * `i[i]` contains an item's parent, usually a module. For compactness, + * it is a set of indexes into the `p` array. + * + * `f` contains function signatures, or `0` if the item isn't a function. + * More information on how they're encoded can be found in rustc-dev-guide + * + * Functions are themselves encoded as arrays. The first item is a list of + * types representing the function's inputs, and the second list item is a list + * of types representing the function's output. Tuples are flattened. + * 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. + * + * `doc` contains the description of the crate. + * + * `p` is a list of path/type pairs. It is used for parents and function parameters. + * + * `c` is an array of item indices that are deprecated. + * @typedef {{ + * doc: string, + * a: Object, + * n: Array, + * t: String, + * d: Array, + * q: Array<[Number, string]>, + * i: Array, + * f: string, + * p: Array, + * b: Array<[Number, String]>, + * c: Array + * }} + */ +let RawSearchIndexCrate; diff --git a/src/librustdoc/html/static/js/search.js b/src/librustdoc/html/static/js/search.js index e6263db32835d..2df4f7020dfbb 100644 --- a/src/librustdoc/html/static/js/search.js +++ b/src/librustdoc/html/static/js/search.js @@ -2924,6 +2924,11 @@ ${item.displayPath}${name}\ return functionTypeFingerprint[(fullId * 4) + 3]; } + /** + * Convert raw search index into in-memory search index. + * + * @param {[string, RawSearchIndexCrate][]} rawSearchIndex + */ function buildIndex(rawSearchIndex) { searchIndex = []; typeNameIdMap = new Map(); @@ -2950,59 +2955,7 @@ ${item.displayPath}${name}\ // This loop actually generates the search item indexes, including // normalized names, type signature objects and fingerprints, and aliases. id = 0; - /** - * The raw search data for a given crate. `n`, `t`, `d`, `i`, and `f` - * are arrays with the same length. `q`, `a`, and `c` use a sparse - * representation for compactness. - * - * `n[i]` contains the name of an item. - * - * `t[i]` contains the type of that item - * (as a string of characters that represent an offset in `itemTypes`). - * - * `d[i]` contains the description of that item. - * - * `q` contains the full paths of the items. For compactness, it is a set of - * (index, path) pairs used to create a map. If a given index `i` is - * not present, this indicates "same as the last index present". - * - * `i[i]` contains an item's parent, usually a module. For compactness, - * it is a set of indexes into the `p` array. - * - * `f[i]` contains function signatures, or `0` if the item isn't a function. - * Functions are themselves encoded as arrays. The first item is a list of - * types representing the function's inputs, and the second list item is a list - * of types representing the function's output. Tuples are flattened. - * 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. - * - * `doc` contains the description of the crate. - * - * `p` is a list of path/type pairs. It is used for parents and function parameters. - * - * `c` is an array of item indices that are deprecated. - * - * @type {{ - * doc: string, - * a: Object, - * n: Array, - * t: String, - * d: Array, - * q: Array<[Number, string]>, - * i: Array, - * f: Array, - * p: Array, - * b: Array<[Number, String]>, - * c: Array - * }} - */ + for (const [crate, crateCorpus] of rawSearchIndex) { // This object should have exactly the same set of fields as the "row" // object defined below. Your JavaScript runtime will thank you. @@ -3039,7 +2992,7 @@ ${item.displayPath}${name}\ const itemDescs = crateCorpus.d; // an array of (Number) the parent path index + 1 to `paths`, or 0 if none const itemParentIdxs = crateCorpus.i; - // an array of (Object | null) the type of the function, if any + // an array of (Array | 0) the type of the function, if any const itemFunctionSearchTypes = crateCorpus.f; // an array of (Number) indices for the deprecated items const deprecatedItems = new Set(crateCorpus.c); From 86b9550811b359a0af7a93523fbd14c5cdcc635f Mon Sep 17 00:00:00 2001 From: Michael Howell Date: Sat, 30 Dec 2023 20:56:59 -0700 Subject: [PATCH 03/23] rustdoc-search: tighter encoding for `f` index Two optimizations for the function signature search: * Instead of using JSON arrays, like `[1,20]`, it uses VLQ hex with no commas, like `[aAd]`. * This also adds backrefs: if you have more than one function with exactly the same signature, it'll not only store it once, it'll *decode* it once, and store in the typeIdMap only once. Size change ----------- standard library ```console $ du -bs search-index-old.js search-index-new.js 4976370 search-index-old.js 4404391 search-index-new.js ``` ((4976370-4404391)/4404391)*100% = 12.9% Benchmarks are similarly shrunk: ```console $ du -hs tmp/{arti,cortex-m,sqlx,stm32f4,ripgrep}/toolchain_{old,new}/doc/search-index.js 10555067 tmp/arti/toolchain_old/doc/search-index.js 8921236 tmp/arti/toolchain_new/doc/search-index.js 77018 tmp/cortex-m/toolchain_old/doc/search-index.js 66676 tmp/cortex-m/toolchain_new/doc/search-index.js 2876330 tmp/sqlx/toolchain_old/doc/search-index.js 2436812 tmp/sqlx/toolchain_new/doc/search-index.js 63632890 tmp/stm32f4/toolchain_old/doc/search-index.js 52337438 tmp/stm32f4/toolchain_new/doc/search-index.js 631150 tmp/ripgrep/toolchain_old/doc/search-index.js 541646 tmp/ripgrep/toolchain_new/doc/search-index.js ``` --- src/librustdoc/html/render/mod.rs | 146 ++++++++++++++------- src/librustdoc/html/render/search_index.rs | 29 +--- src/librustdoc/html/static/js/search.js | 81 +++++++++--- 3 files changed, 170 insertions(+), 86 deletions(-) diff --git a/src/librustdoc/html/render/mod.rs b/src/librustdoc/html/render/mod.rs index 118c6eeb289bb..345a47a21b8f7 100644 --- a/src/librustdoc/html/render/mod.rs +++ b/src/librustdoc/html/render/mod.rs @@ -58,7 +58,7 @@ use rustc_span::{ symbol::{sym, Symbol}, BytePos, FileName, RealFileName, }; -use serde::ser::{SerializeMap, SerializeSeq}; +use serde::ser::SerializeMap; use serde::{Serialize, Serializer}; use crate::clean::{self, ItemId, RenderedLink, SelfTy}; @@ -123,44 +123,53 @@ pub(crate) struct IndexItem { } /// A type used for the search index. -#[derive(Debug)] +#[derive(Debug, Eq, PartialEq)] pub(crate) struct RenderType { id: Option, generics: Option>, bindings: Option)>>, } -impl Serialize for RenderType { - fn serialize(&self, serializer: S) -> Result - where - S: Serializer, - { - let id = match &self.id { - // 0 is a sentinel, everything else is one-indexed - None => 0, - // concrete type - Some(RenderTypeId::Index(idx)) if *idx >= 0 => idx + 1, - // generic type parameter - Some(RenderTypeId::Index(idx)) => *idx, - _ => panic!("must convert render types to indexes before serializing"), - }; +impl RenderType { + pub fn write_to_string(&self, string: &mut String) { if self.generics.is_some() || self.bindings.is_some() { - let mut seq = serializer.serialize_seq(None)?; - seq.serialize_element(&id)?; - seq.serialize_element(self.generics.as_ref().map(Vec::as_slice).unwrap_or_default())?; + string.push('{'); + // 0 is a sentinel, everything else is one-indexed + match self.id { + Some(id) => id.write_to_string(string), + None => string.push('`'), + } + string.push('{'); + for generic in &self.generics.as_ref().map(Vec::as_slice).unwrap_or_default()[..] { + generic.write_to_string(string); + } + string.push('}'); if self.bindings.is_some() { - seq.serialize_element( - self.bindings.as_ref().map(Vec::as_slice).unwrap_or_default(), - )?; + string.push('{'); + for binding in &self.bindings.as_ref().map(Vec::as_slice).unwrap_or_default()[..] { + string.push('{'); + binding.0.write_to_string(string); + string.push('{'); + for constraint in &binding.1[..] { + constraint.write_to_string(string); + } + string.push('}'); + string.push('}'); + } + string.push('}'); } - seq.end() + string.push('}'); } else { - id.serialize(serializer) + // 0 is a sentinel, everything else is one-indexed + match self.id { + Some(id) => id.write_to_string(string), + None => string.push('`'), + } } } } -#[derive(Clone, Copy, Debug)] +#[derive(Clone, Copy, Debug, Eq, PartialEq)] pub(crate) enum RenderTypeId { DefId(DefId), Primitive(clean::PrimitiveType), @@ -168,36 +177,50 @@ pub(crate) enum RenderTypeId { Index(isize), } -impl Serialize for RenderTypeId { - fn serialize(&self, serializer: S) -> Result - where - S: Serializer, - { - let id = match &self { +impl RenderTypeId { + pub fn write_to_string(&self, string: &mut String) { + // (sign, value) + let (sign, id): (bool, u32) = match &self { // 0 is a sentinel, everything else is one-indexed // concrete type - RenderTypeId::Index(idx) if *idx >= 0 => idx + 1, + RenderTypeId::Index(idx) if *idx >= 0 => (false, (idx + 1isize).try_into().unwrap()), // generic type parameter - RenderTypeId::Index(idx) => *idx, + RenderTypeId::Index(idx) => (true, (-*idx).try_into().unwrap()), _ => panic!("must convert render types to indexes before serializing"), }; - id.serialize(serializer) + // zig-zag notation + let value: u32 = (id << 1) | (if sign { 1 } else { 0 }); + // encode + let mut shift: u32 = 28; + let mut mask: u32 = 0xF0_00_00_00; + while shift < 32 { + let hexit = (value & mask) >> shift; + if hexit != 0 || shift == 0 { + let hex = + char::try_from(if shift == 0 { '`' } else { '@' } as u32 + hexit).unwrap(); + string.push(hex); + } + shift = shift.wrapping_sub(4); + mask = mask >> 4; + } } } /// Full type of functions/methods in the search index. -#[derive(Debug)] +#[derive(Debug, Eq, PartialEq)] pub(crate) struct IndexItemFunctionType { inputs: Vec, output: Vec, where_clause: Vec>, } -impl Serialize for IndexItemFunctionType { - fn serialize(&self, serializer: S) -> Result - where - S: Serializer, - { +impl IndexItemFunctionType { + pub fn write_to_string<'a>( + &'a self, + string: &mut String, + backref_queue: &mut VecDeque<&'a IndexItemFunctionType>, + ) { + assert!(backref_queue.len() < 16); // If we couldn't figure out a type, just write `0`. let has_missing = self .inputs @@ -205,33 +228,58 @@ impl Serialize for IndexItemFunctionType { .chain(self.output.iter()) .any(|i| i.id.is_none() && i.generics.is_none()); if has_missing { - 0.serialize(serializer) + string.push('`'); + } else if let Some(idx) = backref_queue.iter().position(|other| *other == self) { + string.push( + char::try_from('0' as u32 + u32::try_from(idx).unwrap()) + .expect("last possible value is '?'"), + ); } else { - let mut seq = serializer.serialize_seq(None)?; + backref_queue.push_front(self); + if backref_queue.len() >= 16 { + backref_queue.pop_back(); + } + string.push('{'); match &self.inputs[..] { [one] if one.generics.is_none() && one.bindings.is_none() => { - seq.serialize_element(one)? + one.write_to_string(string); + } + _ => { + string.push('{'); + for item in &self.inputs[..] { + item.write_to_string(string); + } + string.push('}'); } - _ => seq.serialize_element(&self.inputs)?, } match &self.output[..] { [] if self.where_clause.is_empty() => {} [one] if one.generics.is_none() && one.bindings.is_none() => { - seq.serialize_element(one)? + one.write_to_string(string); + } + _ => { + string.push('{'); + for item in &self.output[..] { + item.write_to_string(string); + } + string.push('}'); } - _ => seq.serialize_element(&self.output)?, } for constraint in &self.where_clause { if let [one] = &constraint[..] && one.generics.is_none() && one.bindings.is_none() { - seq.serialize_element(one)?; + one.write_to_string(string); } else { - seq.serialize_element(constraint)?; + string.push('{'); + for item in &constraint[..] { + item.write_to_string(string); + } + string.push('}'); } } - seq.end() + string.push('}'); } } } diff --git a/src/librustdoc/html/render/search_index.rs b/src/librustdoc/html/render/search_index.rs index a1029320d2d27..e49df400c83ed 100644 --- a/src/librustdoc/html/render/search_index.rs +++ b/src/librustdoc/html/render/search_index.rs @@ -1,5 +1,5 @@ use std::collections::hash_map::Entry; -use std::collections::BTreeMap; +use std::collections::{BTreeMap, VecDeque}; use rustc_data_structures::fx::{FxHashMap, FxIndexMap}; use rustc_middle::ty::TyCtxt; @@ -409,9 +409,11 @@ pub(crate) fn build_index<'tcx>( let mut full_paths = Vec::with_capacity(self.items.len()); let mut descriptions = Vec::with_capacity(self.items.len()); let mut parents = Vec::with_capacity(self.items.len()); - let mut functions = Vec::with_capacity(self.items.len()); + let mut functions = String::with_capacity(self.items.len()); let mut deprecated = Vec::with_capacity(self.items.len()); + let mut backref_queue = VecDeque::new(); + for (index, item) in self.items.iter().enumerate() { let n = item.ty as u8; let c = char::try_from(n + b'A').expect("item types must fit in ASCII"); @@ -434,27 +436,10 @@ pub(crate) fn build_index<'tcx>( full_paths.push((index, &item.path)); } - // Fake option to get `0` out as a sentinel instead of `null`. - // We want to use `0` because it's three less bytes. - enum FunctionOption<'a> { - Function(&'a IndexItemFunctionType), - None, - } - impl<'a> Serialize for FunctionOption<'a> { - fn serialize(&self, serializer: S) -> Result - where - S: Serializer, - { - match self { - FunctionOption::None => 0.serialize(serializer), - FunctionOption::Function(ty) => ty.serialize(serializer), - } - } + match &item.search_type { + Some(ty) => ty.write_to_string(&mut functions, &mut backref_queue), + None => functions.push('`'), } - functions.push(match &item.search_type { - Some(ty) => FunctionOption::Function(ty), - None => FunctionOption::None, - }); if item.deprecation.is_some() { deprecated.push(index); diff --git a/src/librustdoc/html/static/js/search.js b/src/librustdoc/html/static/js/search.js index 2df4f7020dfbb..6fb92d8fbb123 100644 --- a/src/librustdoc/html/static/js/search.js +++ b/src/librustdoc/html/static/js/search.js @@ -2767,19 +2767,65 @@ ${item.displayPath}${name}\ * The raw function search type format is generated using serde in * librustdoc/html/render/mod.rs: impl Serialize for IndexItemFunctionType * - * @param {RawFunctionSearchType} functionSearchType + * @param {{ + * string: string, + * offset: number, + * backrefQueue: FunctionSearchType[] + * }} itemFunctionDecoder * @param {Array<{name: string, ty: number}>} lowercasePaths * @param {Map} * * @return {null|FunctionSearchType} */ - function buildFunctionSearchType(functionSearchType, lowercasePaths) { - const INPUTS_DATA = 0; - const OUTPUT_DATA = 1; - // `0` is used as a sentinel because it's fewer bytes than `null` - if (functionSearchType === 0) { + function buildFunctionSearchType(itemFunctionDecoder, lowercasePaths) { + const c = itemFunctionDecoder.string.charCodeAt(itemFunctionDecoder.offset); + itemFunctionDecoder.offset += 1; + const [zero, ua, la, ob, cb] = ["0", "@", "`", "{", "}"].map(c => c.charCodeAt(0)); + // `` ` `` is used as a sentinel because it's fewer bytes than `null`, and decodes to zero + // `0` is a backref + if (c === la) { return null; } + // sixteen characters after "0" are backref + if (c >= zero && c < ua) { + return itemFunctionDecoder.backrefQueue[c - zero]; + } + if (c !== ob) { + throw ["Unexpected ", c, " in function: expected ", "{", "; this is a bug"]; + } + // call after consuming `{` + function decodeList() { + let c = itemFunctionDecoder.string.charCodeAt(itemFunctionDecoder.offset); + const ret = []; + while (c !== cb) { + ret.push(decode()); + c = itemFunctionDecoder.string.charCodeAt(itemFunctionDecoder.offset); + } + itemFunctionDecoder.offset += 1; // eat cb + return ret; + } + // consumes and returns a list or integer + function decode() { + let n = 0; + let c = itemFunctionDecoder.string.charCodeAt(itemFunctionDecoder.offset); + if (c === ob) { + itemFunctionDecoder.offset += 1; + return decodeList(); + } + while (c < la) { + n = (n << 4) | (c & 0xF); + itemFunctionDecoder.offset += 1; + c = itemFunctionDecoder.string.charCodeAt(itemFunctionDecoder.offset); + } + // last character >= la + n = (n << 4) | (c & 0xF); + const [sign, value] = [n & 1, n >> 1]; + itemFunctionDecoder.offset += 1; + return sign ? -value : value; + } + const functionSearchType = decodeList(); + const INPUTS_DATA = 0; + const OUTPUT_DATA = 1; let inputs, output; if (typeof functionSearchType[INPUTS_DATA] === "number") { inputs = [buildItemSearchType(functionSearchType[INPUTS_DATA], lowercasePaths)]; @@ -2808,9 +2854,14 @@ ${item.displayPath}${name}\ ? [buildItemSearchType(functionSearchType[i], lowercasePaths)] : buildItemSearchTypeAll(functionSearchType[i], lowercasePaths)); } - return { + const ret = { inputs, output, where_clause, }; + itemFunctionDecoder.backrefQueue.unshift(ret); + if (itemFunctionDecoder.backrefQueue.length >= 16) { + itemFunctionDecoder.backrefQueue.pop(); + } + return ret; } /** @@ -2992,8 +3043,12 @@ ${item.displayPath}${name}\ const itemDescs = crateCorpus.d; // an array of (Number) the parent path index + 1 to `paths`, or 0 if none const itemParentIdxs = crateCorpus.i; - // an array of (Array | 0) the type of the function, if any - const itemFunctionSearchTypes = crateCorpus.f; + // a string representing the list of function types + const itemFunctionDecoder = { + string: crateCorpus.f, + offset: 0, + backrefQueue: [], + }; // an array of (Number) indices for the deprecated items const deprecatedItems = new Set(crateCorpus.c); // an array of (Number) indices for the deprecated items @@ -3041,12 +3096,8 @@ ${item.displayPath}${name}\ word = itemNames[i].toLowerCase(); } const path = itemPaths.has(i) ? itemPaths.get(i) : lastPath; - let type = null; - if (itemFunctionSearchTypes[i] !== 0) { - type = buildFunctionSearchType( - itemFunctionSearchTypes[i], - lowercasePaths - ); + const type = buildFunctionSearchType(itemFunctionDecoder, lowercasePaths); + if (type !== null) { if (type) { const fp = functionTypeFingerprint.subarray(id * 4, (id + 1) * 4); const fps = new Set(); From a68ac32de5003e5b03b4e7e7b7de0aebb974901a Mon Sep 17 00:00:00 2001 From: Michael Howell Date: Thu, 4 Jan 2024 12:28:18 -0700 Subject: [PATCH 04/23] Clean up serialization code nits --- src/librustdoc/html/render/mod.rs | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/src/librustdoc/html/render/mod.rs b/src/librustdoc/html/render/mod.rs index 345a47a21b8f7..8afeec81d8731 100644 --- a/src/librustdoc/html/render/mod.rs +++ b/src/librustdoc/html/render/mod.rs @@ -132,13 +132,16 @@ pub(crate) struct RenderType { impl RenderType { pub fn write_to_string(&self, string: &mut String) { - if self.generics.is_some() || self.bindings.is_some() { - string.push('{'); + fn write_optional_id(id: Option, string: &mut String) { // 0 is a sentinel, everything else is one-indexed - match self.id { + match id { Some(id) => id.write_to_string(string), None => string.push('`'), } + } + if self.generics.is_some() || self.bindings.is_some() { + string.push('{'); + write_optional_id(self.id, string); string.push('{'); for generic in &self.generics.as_ref().map(Vec::as_slice).unwrap_or_default()[..] { generic.write_to_string(string); @@ -153,18 +156,13 @@ impl RenderType { for constraint in &binding.1[..] { constraint.write_to_string(string); } - string.push('}'); - string.push('}'); + string.push_str("}}"); } string.push('}'); } string.push('}'); } else { - // 0 is a sentinel, everything else is one-indexed - match self.id { - Some(id) => id.write_to_string(string), - None => string.push('`'), - } + write_optional_id(self.id, string); } } } @@ -191,6 +189,7 @@ impl RenderTypeId { // zig-zag notation let value: u32 = (id << 1) | (if sign { 1 } else { 0 }); // encode + // Documented in https://rust-lang.github.io/rustc-dev-guide/rustdoc-internals/search.html let mut shift: u32 = 28; let mut mask: u32 = 0xF0_00_00_00; while shift < 32 { From 506b9f96893a56159334f05d85500313a783f199 Mon Sep 17 00:00:00 2001 From: Zalathar Date: Fri, 22 Dec 2023 14:11:42 +1100 Subject: [PATCH 05/23] coverage: Avoid early returns from `mir_to_initial_sorted_coverage_spans` --- .../src/coverage/spans/from_mir.rs | 29 +++++++++---------- 1 file changed, 14 insertions(+), 15 deletions(-) diff --git a/compiler/rustc_mir_transform/src/coverage/spans/from_mir.rs b/compiler/rustc_mir_transform/src/coverage/spans/from_mir.rs index 8f6592afe85cb..913819ef33d32 100644 --- a/compiler/rustc_mir_transform/src/coverage/spans/from_mir.rs +++ b/compiler/rustc_mir_transform/src/coverage/spans/from_mir.rs @@ -15,27 +15,26 @@ pub(super) fn mir_to_initial_sorted_coverage_spans( basic_coverage_blocks: &CoverageGraph, ) -> Vec { let &ExtractedHirInfo { is_async_fn, fn_sig_span, body_span, .. } = hir_info; + + let mut initial_spans = vec![CoverageSpan::for_fn_sig(fn_sig_span)]; + if is_async_fn { // An async function desugars into a function that returns a future, // with the user code wrapped in a closure. Any spans in the desugared - // outer function will be unhelpful, so just produce a single span - // associating the function signature with its entry BCB. - return vec![CoverageSpan::for_fn_sig(fn_sig_span)]; - } - - let mut initial_spans = Vec::with_capacity(mir_body.basic_blocks.len() * 2); - for (bcb, bcb_data) in basic_coverage_blocks.iter_enumerated() { - initial_spans.extend(bcb_to_initial_coverage_spans(mir_body, body_span, bcb, bcb_data)); - } + // outer function will be unhelpful, so just keep the signature span + // and ignore all of the spans in the MIR body. + } else { + for (bcb, bcb_data) in basic_coverage_blocks.iter_enumerated() { + initial_spans.extend(bcb_to_initial_coverage_spans(mir_body, body_span, bcb, bcb_data)); + } - if initial_spans.is_empty() { - // This can happen if, for example, the function is unreachable (contains only a - // `BasicBlock`(s) with an `Unreachable` terminator). - return initial_spans; + // If no spans were extracted from the body, discard the signature span. + // FIXME: This preserves existing behavior; consider getting rid of it. + if initial_spans.len() == 1 { + initial_spans.clear(); + } } - initial_spans.push(CoverageSpan::for_fn_sig(fn_sig_span)); - initial_spans.sort_by(|a, b| { // First sort by span start. Ord::cmp(&a.span.lo(), &b.span.lo()) From df0df5256b95023e1e5e6bc49ee3857d90d91555 Mon Sep 17 00:00:00 2001 From: Zalathar Date: Thu, 28 Dec 2023 15:08:24 +1100 Subject: [PATCH 06/23] coverage: Overhaul how "visible macros" are determined --- .../rustc_mir_transform/src/coverage/spans.rs | 65 ++----------------- .../src/coverage/spans/from_mir.rs | 59 ++++++++++++++--- tests/coverage/inline-dead.cov-map | 4 +- tests/coverage/inline-dead.coverage | 2 +- 4 files changed, 61 insertions(+), 69 deletions(-) diff --git a/compiler/rustc_mir_transform/src/coverage/spans.rs b/compiler/rustc_mir_transform/src/coverage/spans.rs index ed091752187ef..6c0c62a1b7649 100644 --- a/compiler/rustc_mir_transform/src/coverage/spans.rs +++ b/compiler/rustc_mir_transform/src/coverage/spans.rs @@ -1,9 +1,7 @@ -use std::cell::OnceCell; - use rustc_data_structures::graph::WithNumNodes; use rustc_index::IndexVec; use rustc_middle::mir; -use rustc_span::{BytePos, ExpnKind, MacroKind, Span, Symbol, DUMMY_SP}; +use rustc_span::{BytePos, Span, Symbol, DUMMY_SP}; use super::graph::{BasicCoverageBlock, CoverageGraph, START_BCB}; use crate::coverage::ExtractedHirInfo; @@ -71,8 +69,7 @@ impl CoverageSpans { #[derive(Debug, Clone)] struct CoverageSpan { pub span: Span, - pub expn_span: Span, - pub current_macro_or_none: OnceCell>, + pub visible_macro: Option, pub bcb: BasicCoverageBlock, /// List of all the original spans from MIR that have been merged into this /// span. Mainly used to precisely skip over gaps when truncating a span. @@ -82,23 +79,16 @@ struct CoverageSpan { impl CoverageSpan { pub fn for_fn_sig(fn_sig_span: Span) -> Self { - Self::new(fn_sig_span, fn_sig_span, START_BCB, false) + Self::new(fn_sig_span, None, START_BCB, false) } pub(super) fn new( span: Span, - expn_span: Span, + visible_macro: Option, bcb: BasicCoverageBlock, is_closure: bool, ) -> Self { - Self { - span, - expn_span, - current_macro_or_none: Default::default(), - bcb, - merged_spans: vec![span], - is_closure, - } + Self { span, visible_macro, bcb, merged_spans: vec![span], is_closure } } pub fn merge_from(&mut self, other: &Self) { @@ -123,37 +113,6 @@ impl CoverageSpan { pub fn is_in_same_bcb(&self, other: &Self) -> bool { self.bcb == other.bcb } - - /// If the span is part of a macro, returns the macro name symbol. - pub fn current_macro(&self) -> Option { - self.current_macro_or_none - .get_or_init(|| { - if let ExpnKind::Macro(MacroKind::Bang, current_macro) = - self.expn_span.ctxt().outer_expn_data().kind - { - return Some(current_macro); - } - None - }) - .map(|symbol| symbol) - } - - /// If the span is part of a macro, and the macro is visible (expands directly to the given - /// body_span), returns the macro name symbol. - pub fn visible_macro(&self, body_span: Span) -> Option { - let current_macro = self.current_macro()?; - let parent_callsite = self.expn_span.parent_callsite()?; - - // In addition to matching the context of the body span, the parent callsite - // must also be the source callsite, i.e. the parent must have no parent. - let is_visible_macro = - parent_callsite.parent_callsite().is_none() && parent_callsite.eq_ctxt(body_span); - is_visible_macro.then_some(current_macro) - } - - pub fn is_macro_expansion(&self) -> bool { - self.current_macro().is_some() - } } /// Converts the initial set of `CoverageSpan`s (one per MIR `Statement` or `Terminator`) into a @@ -164,10 +123,6 @@ impl CoverageSpan { /// execution /// * Carve out (leave uncovered) any span that will be counted by another MIR (notably, closures) struct CoverageSpansGenerator<'a> { - /// A `Span` covering the function body of the MIR (typically from left curly brace to right - /// curly brace). - body_span: Span, - /// The BasicCoverageBlock Control Flow Graph (BCB CFG). basic_coverage_blocks: &'a CoverageGraph, @@ -244,7 +199,6 @@ impl<'a> CoverageSpansGenerator<'a> { ); let coverage_spans = Self { - body_span: hir_info.body_span, basic_coverage_blocks, sorted_spans_iter: sorted_spans.into_iter(), some_curr: None, @@ -303,7 +257,7 @@ impl<'a> CoverageSpansGenerator<'a> { // **originally** the same as the original span of `prev()`. The original spans // reflect their original sort order, and for equal spans, conveys a partial // ordering based on CFG dominator priority. - if prev.is_macro_expansion() && curr.is_macro_expansion() { + if prev.visible_macro.is_some() && curr.visible_macro.is_some() { // Macros that expand to include branching (such as // `assert_eq!()`, `assert_ne!()`, `info!()`, `debug!()`, or // `trace!()`) typically generate callee spans with identical @@ -365,12 +319,7 @@ impl<'a> CoverageSpansGenerator<'a> { fn maybe_push_macro_name_span(&mut self) { let curr = self.curr(); - let Some(visible_macro) = curr.visible_macro(self.body_span) else { return }; - if let Some(prev) = &self.some_prev - && prev.expn_span.eq_ctxt(curr.expn_span) - { - return; - } + let Some(visible_macro) = curr.visible_macro else { return }; // The split point is relative to `curr_original_span`, // because `curr.span` may have been merged with preceding spans. diff --git a/compiler/rustc_mir_transform/src/coverage/spans/from_mir.rs b/compiler/rustc_mir_transform/src/coverage/spans/from_mir.rs index 913819ef33d32..fde25ad994fdf 100644 --- a/compiler/rustc_mir_transform/src/coverage/spans/from_mir.rs +++ b/compiler/rustc_mir_transform/src/coverage/spans/from_mir.rs @@ -3,7 +3,7 @@ use rustc_middle::mir::{ self, AggregateKind, FakeReadCause, Rvalue, Statement, StatementKind, Terminator, TerminatorKind, }; -use rustc_span::Span; +use rustc_span::{ExpnKind, MacroKind, Span, Symbol}; use crate::coverage::graph::{BasicCoverageBlock, BasicCoverageBlockData, CoverageGraph}; use crate::coverage::spans::CoverageSpan; @@ -71,16 +71,18 @@ fn bcb_to_initial_coverage_spans<'a, 'tcx>( let statement_spans = data.statements.iter().filter_map(move |statement| { let expn_span = filtered_statement_span(statement)?; - let span = unexpand_into_body_span(expn_span, body_span)?; + let (span, visible_macro) = + unexpand_into_body_span_with_visible_macro(expn_span, body_span)?; - Some(CoverageSpan::new(span, expn_span, bcb, is_closure_or_coroutine(statement))) + Some(CoverageSpan::new(span, visible_macro, bcb, is_closure_or_coroutine(statement))) }); let terminator_span = Some(data.terminator()).into_iter().filter_map(move |terminator| { let expn_span = filtered_terminator_span(terminator)?; - let span = unexpand_into_body_span(expn_span, body_span)?; + let (span, visible_macro) = + unexpand_into_body_span_with_visible_macro(expn_span, body_span)?; - Some(CoverageSpan::new(span, expn_span, bcb, false)) + Some(CoverageSpan::new(span, visible_macro, bcb, false)) }); statement_spans.chain(terminator_span) @@ -201,7 +203,48 @@ fn filtered_terminator_span(terminator: &Terminator<'_>) -> Option { /// /// [^1]Expansions result from Rust syntax including macros, syntactic sugar, /// etc.). -#[inline] -fn unexpand_into_body_span(span: Span, body_span: Span) -> Option { - span.find_ancestor_inside_same_ctxt(body_span) +fn unexpand_into_body_span_with_visible_macro( + original_span: Span, + body_span: Span, +) -> Option<(Span, Option)> { + let (span, prev) = unexpand_into_body_span_with_prev(original_span, body_span)?; + + let visible_macro = prev + .map(|prev| match prev.ctxt().outer_expn_data().kind { + ExpnKind::Macro(MacroKind::Bang, name) => Some(name), + _ => None, + }) + .flatten(); + + Some((span, visible_macro)) +} + +/// Walks through the expansion ancestors of `original_span` to find a span that +/// is contained in `body_span` and has the same [`SyntaxContext`] as `body_span`. +/// The ancestor that was traversed just before the matching span (if any) is +/// also returned. +/// +/// For example, a return value of `Some((ancestor, Some(prev))` means that: +/// - `ancestor == original_span.find_ancestor_inside_same_ctxt(body_span)` +/// - `ancestor == prev.parent_callsite()` +/// +/// [`SyntaxContext`]: rustc_span::SyntaxContext +fn unexpand_into_body_span_with_prev( + original_span: Span, + body_span: Span, +) -> Option<(Span, Option)> { + let mut prev = None; + let mut curr = original_span; + + while !body_span.contains(curr) || !curr.eq_ctxt(body_span) { + prev = Some(curr); + curr = curr.parent_callsite()?; + } + + debug_assert_eq!(Some(curr), original_span.find_ancestor_in_same_ctxt(body_span)); + if let Some(prev) = prev { + debug_assert_eq!(Some(curr), prev.parent_callsite()); + } + + Some((curr, prev)) } diff --git a/tests/coverage/inline-dead.cov-map b/tests/coverage/inline-dead.cov-map index 958b423f24cf8..ab04e746b9165 100644 --- a/tests/coverage/inline-dead.cov-map +++ b/tests/coverage/inline-dead.cov-map @@ -31,14 +31,14 @@ Number of file 0 mappings: 2 - Code(Counter(0)) at (prev + 7, 6) to (start + 2, 2) Function name: inline_dead::main::{closure#0} -Raw bytes (23): 0x[01, 01, 02, 00, 06, 01, 00, 03, 01, 07, 17, 00, 18, 00, 02, 0d, 00, 0e, 03, 02, 05, 00, 06] +Raw bytes (23): 0x[01, 01, 02, 00, 06, 01, 00, 03, 01, 07, 17, 01, 16, 00, 02, 0d, 00, 0e, 03, 02, 05, 00, 06] Number of files: 1 - file 0 => global file 1 Number of expressions: 2 - expression 0 operands: lhs = Zero, rhs = Expression(1, Sub) - expression 1 operands: lhs = Counter(0), rhs = Zero Number of file 0 mappings: 3 -- Code(Counter(0)) at (prev + 7, 23) to (start + 0, 24) +- Code(Counter(0)) at (prev + 7, 23) to (start + 1, 22) - Code(Zero) at (prev + 2, 13) to (start + 0, 14) - Code(Expression(0, Add)) at (prev + 2, 5) to (start + 0, 6) = (Zero + (c0 - Zero)) diff --git a/tests/coverage/inline-dead.coverage b/tests/coverage/inline-dead.coverage index de96aa17acd62..7c201f482db31 100644 --- a/tests/coverage/inline-dead.coverage +++ b/tests/coverage/inline-dead.coverage @@ -5,7 +5,7 @@ LL| 1| println!("{}", live::()); LL| 1| LL| 1| let f = |x: bool| { - LL| | debug_assert!( + LL| 1| debug_assert!( LL| 0| x LL| | ); LL| 1| }; From cd3a9760e453e9660c9c44316076a3e63248e730 Mon Sep 17 00:00:00 2001 From: Zalathar Date: Thu, 21 Dec 2023 23:03:45 +1100 Subject: [PATCH 07/23] coverage: Hoist the removal of unwanted macro expansion spans --- .../rustc_mir_transform/src/coverage/spans.rs | 29 ++----------------- .../src/coverage/spans/from_mir.rs | 24 +++++++++++++++ 2 files changed, 27 insertions(+), 26 deletions(-) diff --git a/compiler/rustc_mir_transform/src/coverage/spans.rs b/compiler/rustc_mir_transform/src/coverage/spans.rs index 6c0c62a1b7649..576f098e6e4aa 100644 --- a/compiler/rustc_mir_transform/src/coverage/spans.rs +++ b/compiler/rustc_mir_transform/src/coverage/spans.rs @@ -251,32 +251,9 @@ impl<'a> CoverageSpansGenerator<'a> { } else if curr.is_closure { self.carve_out_span_for_closure(); } else if self.prev_original_span == curr.span { - // Note that this compares the new (`curr`) span to `prev_original_span`. - // In this branch, the actual span byte range of `prev_original_span` is not - // important. What is important is knowing whether the new `curr` span was - // **originally** the same as the original span of `prev()`. The original spans - // reflect their original sort order, and for equal spans, conveys a partial - // ordering based on CFG dominator priority. - if prev.visible_macro.is_some() && curr.visible_macro.is_some() { - // Macros that expand to include branching (such as - // `assert_eq!()`, `assert_ne!()`, `info!()`, `debug!()`, or - // `trace!()`) typically generate callee spans with identical - // ranges (typically the full span of the macro) for all - // `BasicBlocks`. This makes it impossible to distinguish - // the condition (`if val1 != val2`) from the optional - // branched statements (such as the call to `panic!()` on - // assert failure). In this case it is better (or less - // worse) to drop the optional branch bcbs and keep the - // non-conditional statements, to count when reached. - debug!( - " curr and prev are part of a macro expansion, and curr has the same span \ - as prev, but is in a different bcb. Drop curr and keep prev for next iter. \ - prev={prev:?}", - ); - self.take_curr(); // Discards curr. - } else { - self.update_pending_dups(); - } + // `prev` and `curr` have the same span, or would have had the + // same span before `prev` was modified by other spans. + self.update_pending_dups(); } else { self.cutoff_prev_at_overlapping_curr(); self.maybe_push_macro_name_span(); diff --git a/compiler/rustc_mir_transform/src/coverage/spans/from_mir.rs b/compiler/rustc_mir_transform/src/coverage/spans/from_mir.rs index fde25ad994fdf..b28abcf71d57e 100644 --- a/compiler/rustc_mir_transform/src/coverage/spans/from_mir.rs +++ b/compiler/rustc_mir_transform/src/coverage/spans/from_mir.rs @@ -1,4 +1,5 @@ use rustc_data_structures::captures::Captures; +use rustc_data_structures::fx::FxHashSet; use rustc_middle::mir::{ self, AggregateKind, FakeReadCause, Rvalue, Statement, StatementKind, Terminator, TerminatorKind, @@ -35,6 +36,9 @@ pub(super) fn mir_to_initial_sorted_coverage_spans( } } + initial_spans.sort_by(|a, b| basic_coverage_blocks.cmp_in_dominator_order(a.bcb, b.bcb)); + remove_unwanted_macro_spans(&mut initial_spans); + initial_spans.sort_by(|a, b| { // First sort by span start. Ord::cmp(&a.span.lo(), &b.span.lo()) @@ -55,6 +59,26 @@ pub(super) fn mir_to_initial_sorted_coverage_spans( initial_spans } +/// Macros that expand into branches (e.g. `assert!`, `trace!`) tend to generate +/// multiple condition/consequent blocks that have the span of the whole macro +/// invocation, which is unhelpful. Keeping only the first such span seems to +/// give better mappings, so remove the others. +/// +/// (The input spans should be sorted in BCB dominator order, so that the +/// retained "first" span is likely to dominate the others.) +fn remove_unwanted_macro_spans(initial_spans: &mut Vec) { + let mut seen_macro_spans = FxHashSet::default(); + initial_spans.retain(|covspan| { + // Ignore (retain) closure spans and non-macro-expansion spans. + if covspan.is_closure || covspan.visible_macro.is_none() { + return true; + } + + // Retain only the first macro-expanded covspan with this span. + seen_macro_spans.insert(covspan.span) + }); +} + // Generate a set of `CoverageSpan`s from the filtered set of `Statement`s and `Terminator`s of // the `BasicBlock`(s) in the given `BasicCoverageBlockData`. One `CoverageSpan` is generated // for each `Statement` and `Terminator`. (Note that subsequent stages of coverage analysis will From d4d2f1428cda381fe0c0d58700ead551fdf96f6b Mon Sep 17 00:00:00 2001 From: Zalathar Date: Fri, 22 Dec 2023 11:21:52 +1100 Subject: [PATCH 08/23] coverage: Hoist the splitting of visible macro invocations --- .../rustc_mir_transform/src/coverage/spans.rs | 34 --------------- .../src/coverage/spans/from_mir.rs | 36 ++++++++++++++++ tests/coverage/closure.cov-map | 42 ++++++++----------- 3 files changed, 54 insertions(+), 58 deletions(-) diff --git a/compiler/rustc_mir_transform/src/coverage/spans.rs b/compiler/rustc_mir_transform/src/coverage/spans.rs index 576f098e6e4aa..f346aa1810c9d 100644 --- a/compiler/rustc_mir_transform/src/coverage/spans.rs +++ b/compiler/rustc_mir_transform/src/coverage/spans.rs @@ -220,7 +220,6 @@ impl<'a> CoverageSpansGenerator<'a> { // span-processing steps don't make sense yet. if self.some_prev.is_none() { debug!(" initial span"); - self.maybe_push_macro_name_span(); continue; } @@ -232,7 +231,6 @@ impl<'a> CoverageSpansGenerator<'a> { debug!(" same bcb (and neither is a closure), merge with prev={prev:?}"); let prev = self.take_prev(); self.curr_mut().merge_from(&prev); - self.maybe_push_macro_name_span(); // Note that curr.span may now differ from curr_original_span } else if prev.span.hi() <= curr.span.lo() { debug!( @@ -240,7 +238,6 @@ impl<'a> CoverageSpansGenerator<'a> { ); let prev = self.take_prev(); self.refined_spans.push(prev); - self.maybe_push_macro_name_span(); } else if prev.is_closure { // drop any equal or overlapping span (`curr`) and keep `prev` to test again in the // next iter @@ -256,7 +253,6 @@ impl<'a> CoverageSpansGenerator<'a> { self.update_pending_dups(); } else { self.cutoff_prev_at_overlapping_curr(); - self.maybe_push_macro_name_span(); } } @@ -291,36 +287,6 @@ impl<'a> CoverageSpansGenerator<'a> { self.refined_spans } - /// If `curr` is part of a new macro expansion, carve out and push a separate - /// span that ends just after the macro name and its subsequent `!`. - fn maybe_push_macro_name_span(&mut self) { - let curr = self.curr(); - - let Some(visible_macro) = curr.visible_macro else { return }; - - // The split point is relative to `curr_original_span`, - // because `curr.span` may have been merged with preceding spans. - let split_point_after_macro_bang = self.curr_original_span.lo() - + BytePos(visible_macro.as_str().len() as u32) - + BytePos(1); // add 1 for the `!` - debug_assert!(split_point_after_macro_bang <= curr.span.hi()); - if split_point_after_macro_bang > curr.span.hi() { - // Something is wrong with the macro name span; - // return now to avoid emitting malformed mappings (e.g. #117788). - return; - } - - let mut macro_name_cov = curr.clone(); - macro_name_cov.span = macro_name_cov.span.with_hi(split_point_after_macro_bang); - self.curr_mut().span = curr.span.with_lo(split_point_after_macro_bang); - - debug!( - " and curr starts a new macro expansion, so add a new span just for \ - the macro `{visible_macro}!`, new span={macro_name_cov:?}", - ); - self.refined_spans.push(macro_name_cov); - } - #[track_caller] fn curr(&self) -> &CoverageSpan { self.some_curr.as_ref().unwrap_or_else(|| bug!("some_curr is None (curr)")) diff --git a/compiler/rustc_mir_transform/src/coverage/spans/from_mir.rs b/compiler/rustc_mir_transform/src/coverage/spans/from_mir.rs index b28abcf71d57e..637424753d96c 100644 --- a/compiler/rustc_mir_transform/src/coverage/spans/from_mir.rs +++ b/compiler/rustc_mir_transform/src/coverage/spans/from_mir.rs @@ -38,6 +38,7 @@ pub(super) fn mir_to_initial_sorted_coverage_spans( initial_spans.sort_by(|a, b| basic_coverage_blocks.cmp_in_dominator_order(a.bcb, b.bcb)); remove_unwanted_macro_spans(&mut initial_spans); + split_visible_macro_spans(&mut initial_spans); initial_spans.sort_by(|a, b| { // First sort by span start. @@ -79,6 +80,41 @@ fn remove_unwanted_macro_spans(initial_spans: &mut Vec) { }); } +/// When a span corresponds to a macro invocation that is visible from the +/// function body, split it into two parts. The first part covers just the +/// macro name plus `!`, and the second part covers the rest of the macro +/// invocation. This seems to give better results for code that uses macros. +fn split_visible_macro_spans(initial_spans: &mut Vec) { + let mut extra_spans = vec![]; + + initial_spans.retain(|covspan| { + if covspan.is_closure { + return true; + } + + let Some(visible_macro) = covspan.visible_macro else { return true }; + + let split_len = visible_macro.as_str().len() as u32 + 1; + let (before, after) = covspan.span.split_at(split_len); + if !covspan.span.contains(before) || !covspan.span.contains(after) { + // Something is unexpectedly wrong with the split point. + // The debug assertion in `split_at` will have already caught this, + // but in release builds it's safer to do nothing and maybe get a + // bug report for unexpected coverage, rather than risk an ICE. + return true; + } + + assert!(!covspan.is_closure); + extra_spans.push(CoverageSpan::new(before, covspan.visible_macro, covspan.bcb, false)); + extra_spans.push(CoverageSpan::new(after, covspan.visible_macro, covspan.bcb, false)); + false // Discard the original covspan that we just split. + }); + + // The newly-split spans are added at the end, so any previous sorting + // is not preserved. + initial_spans.extend(extra_spans); +} + // Generate a set of `CoverageSpan`s from the filtered set of `Statement`s and `Terminator`s of // the `BasicBlock`(s) in the given `BasicCoverageBlockData`. One `CoverageSpan` is generated // for each `Statement` and `Terminator`. (Note that subsequent stages of coverage analysis will diff --git a/tests/coverage/closure.cov-map b/tests/coverage/closure.cov-map index 522c1e73afe3d..c5ac17600cd2a 100644 --- a/tests/coverage/closure.cov-map +++ b/tests/coverage/closure.cov-map @@ -81,21 +81,18 @@ Number of file 0 mappings: 1 - Code(Zero) at (prev + 171, 13) to (start + 2, 14) Function name: closure::main::{closure#14} -Raw bytes (36): 0x[01, 01, 03, 05, 0a, 01, 05, 01, 05, 05, 03, b2, 01, 0d, 00, 15, 01, 01, 11, 01, 1b, 05, 01, 1e, 00, 25, 0a, 00, 2f, 00, 33, 03, 01, 0d, 00, 0e] +Raw bytes (29): 0x[01, 01, 02, 01, 05, 05, 02, 04, 01, b2, 01, 0d, 02, 1b, 05, 02, 1e, 00, 25, 02, 00, 2f, 00, 33, 07, 01, 0d, 00, 0e] Number of files: 1 - file 0 => global file 1 -Number of expressions: 3 -- expression 0 operands: lhs = Counter(1), rhs = Expression(2, Sub) -- expression 1 operands: lhs = Counter(0), rhs = Counter(1) -- expression 2 operands: lhs = Counter(0), rhs = Counter(1) -Number of file 0 mappings: 5 -- Code(Expression(0, Add)) at (prev + 178, 13) to (start + 0, 21) - = (c1 + (c0 - c1)) -- Code(Counter(0)) at (prev + 1, 17) to (start + 1, 27) -- Code(Counter(1)) at (prev + 1, 30) to (start + 0, 37) -- Code(Expression(2, Sub)) at (prev + 0, 47) to (start + 0, 51) +Number of expressions: 2 +- expression 0 operands: lhs = Counter(0), rhs = Counter(1) +- expression 1 operands: lhs = Counter(1), rhs = Expression(0, Sub) +Number of file 0 mappings: 4 +- Code(Counter(0)) at (prev + 178, 13) to (start + 2, 27) +- Code(Counter(1)) at (prev + 2, 30) to (start + 0, 37) +- Code(Expression(0, Sub)) at (prev + 0, 47) to (start + 0, 51) = (c0 - c1) -- Code(Expression(0, Add)) at (prev + 1, 13) to (start + 0, 14) +- Code(Expression(1, Add)) at (prev + 1, 13) to (start + 0, 14) = (c1 + (c0 - c1)) Function name: closure::main::{closure#15} @@ -118,21 +115,18 @@ Number of file 0 mappings: 6 = (c1 + (c0 - c1)) Function name: closure::main::{closure#16} -Raw bytes (36): 0x[01, 01, 03, 05, 0a, 01, 05, 01, 05, 05, 03, c4, 01, 0d, 00, 15, 01, 01, 11, 01, 1b, 05, 01, 1e, 00, 25, 0a, 00, 2f, 00, 33, 03, 01, 0d, 00, 0e] +Raw bytes (29): 0x[01, 01, 02, 01, 05, 05, 02, 04, 01, c4, 01, 0d, 02, 1b, 05, 02, 1e, 00, 25, 02, 00, 2f, 00, 33, 07, 01, 0d, 00, 0e] Number of files: 1 - file 0 => global file 1 -Number of expressions: 3 -- expression 0 operands: lhs = Counter(1), rhs = Expression(2, Sub) -- expression 1 operands: lhs = Counter(0), rhs = Counter(1) -- expression 2 operands: lhs = Counter(0), rhs = Counter(1) -Number of file 0 mappings: 5 -- Code(Expression(0, Add)) at (prev + 196, 13) to (start + 0, 21) - = (c1 + (c0 - c1)) -- Code(Counter(0)) at (prev + 1, 17) to (start + 1, 27) -- Code(Counter(1)) at (prev + 1, 30) to (start + 0, 37) -- Code(Expression(2, Sub)) at (prev + 0, 47) to (start + 0, 51) +Number of expressions: 2 +- expression 0 operands: lhs = Counter(0), rhs = Counter(1) +- expression 1 operands: lhs = Counter(1), rhs = Expression(0, Sub) +Number of file 0 mappings: 4 +- Code(Counter(0)) at (prev + 196, 13) to (start + 2, 27) +- Code(Counter(1)) at (prev + 2, 30) to (start + 0, 37) +- Code(Expression(0, Sub)) at (prev + 0, 47) to (start + 0, 51) = (c0 - c1) -- Code(Expression(0, Add)) at (prev + 1, 13) to (start + 0, 14) +- Code(Expression(1, Add)) at (prev + 1, 13) to (start + 0, 14) = (c1 + (c0 - c1)) Function name: closure::main::{closure#17} From cd5084388a8e7cbecdea90f071ec0bb5be7c3ee8 Mon Sep 17 00:00:00 2001 From: Zalathar Date: Fri, 22 Dec 2023 14:20:30 +1100 Subject: [PATCH 09/23] coverage: Split out `SpanFromMir` from `CoverageSpan` This draws a clear distinction between the fields/methods that are needed by initial span extraction and preprocessing, and those that are needed by the main "refinement" loop. --- .../rustc_mir_transform/src/coverage/spans.rs | 18 ++---- .../src/coverage/spans/from_mir.rs | 57 +++++++++++++++---- 2 files changed, 51 insertions(+), 24 deletions(-) diff --git a/compiler/rustc_mir_transform/src/coverage/spans.rs b/compiler/rustc_mir_transform/src/coverage/spans.rs index f346aa1810c9d..6604e13398fa8 100644 --- a/compiler/rustc_mir_transform/src/coverage/spans.rs +++ b/compiler/rustc_mir_transform/src/coverage/spans.rs @@ -1,9 +1,9 @@ use rustc_data_structures::graph::WithNumNodes; use rustc_index::IndexVec; use rustc_middle::mir; -use rustc_span::{BytePos, Span, Symbol, DUMMY_SP}; +use rustc_span::{BytePos, Span, DUMMY_SP}; -use super::graph::{BasicCoverageBlock, CoverageGraph, START_BCB}; +use super::graph::{BasicCoverageBlock, CoverageGraph}; use crate::coverage::ExtractedHirInfo; mod from_mir; @@ -69,7 +69,6 @@ impl CoverageSpans { #[derive(Debug, Clone)] struct CoverageSpan { pub span: Span, - pub visible_macro: Option, pub bcb: BasicCoverageBlock, /// List of all the original spans from MIR that have been merged into this /// span. Mainly used to precisely skip over gaps when truncating a span. @@ -78,17 +77,8 @@ struct CoverageSpan { } impl CoverageSpan { - pub fn for_fn_sig(fn_sig_span: Span) -> Self { - Self::new(fn_sig_span, None, START_BCB, false) - } - - pub(super) fn new( - span: Span, - visible_macro: Option, - bcb: BasicCoverageBlock, - is_closure: bool, - ) -> Self { - Self { span, visible_macro, bcb, merged_spans: vec![span], is_closure } + fn new(span: Span, bcb: BasicCoverageBlock, is_closure: bool) -> Self { + Self { span, bcb, merged_spans: vec![span], is_closure } } pub fn merge_from(&mut self, other: &Self) { diff --git a/compiler/rustc_mir_transform/src/coverage/spans/from_mir.rs b/compiler/rustc_mir_transform/src/coverage/spans/from_mir.rs index 637424753d96c..1b6dfccd57495 100644 --- a/compiler/rustc_mir_transform/src/coverage/spans/from_mir.rs +++ b/compiler/rustc_mir_transform/src/coverage/spans/from_mir.rs @@ -6,7 +6,9 @@ use rustc_middle::mir::{ }; use rustc_span::{ExpnKind, MacroKind, Span, Symbol}; -use crate::coverage::graph::{BasicCoverageBlock, BasicCoverageBlockData, CoverageGraph}; +use crate::coverage::graph::{ + BasicCoverageBlock, BasicCoverageBlockData, CoverageGraph, START_BCB, +}; use crate::coverage::spans::CoverageSpan; use crate::coverage::ExtractedHirInfo; @@ -17,7 +19,7 @@ pub(super) fn mir_to_initial_sorted_coverage_spans( ) -> Vec { let &ExtractedHirInfo { is_async_fn, fn_sig_span, body_span, .. } = hir_info; - let mut initial_spans = vec![CoverageSpan::for_fn_sig(fn_sig_span)]; + let mut initial_spans = vec![SpanFromMir::for_fn_sig(fn_sig_span)]; if is_async_fn { // An async function desugars into a function that returns a future, @@ -57,7 +59,7 @@ pub(super) fn mir_to_initial_sorted_coverage_spans( .then_with(|| Ord::cmp(&a.is_closure, &b.is_closure).reverse()) }); - initial_spans + initial_spans.into_iter().map(SpanFromMir::into_coverage_span).collect::>() } /// Macros that expand into branches (e.g. `assert!`, `trace!`) tend to generate @@ -67,7 +69,7 @@ pub(super) fn mir_to_initial_sorted_coverage_spans( /// /// (The input spans should be sorted in BCB dominator order, so that the /// retained "first" span is likely to dominate the others.) -fn remove_unwanted_macro_spans(initial_spans: &mut Vec) { +fn remove_unwanted_macro_spans(initial_spans: &mut Vec) { let mut seen_macro_spans = FxHashSet::default(); initial_spans.retain(|covspan| { // Ignore (retain) closure spans and non-macro-expansion spans. @@ -84,7 +86,7 @@ fn remove_unwanted_macro_spans(initial_spans: &mut Vec) { /// function body, split it into two parts. The first part covers just the /// macro name plus `!`, and the second part covers the rest of the macro /// invocation. This seems to give better results for code that uses macros. -fn split_visible_macro_spans(initial_spans: &mut Vec) { +fn split_visible_macro_spans(initial_spans: &mut Vec) { let mut extra_spans = vec![]; initial_spans.retain(|covspan| { @@ -105,8 +107,8 @@ fn split_visible_macro_spans(initial_spans: &mut Vec) { } assert!(!covspan.is_closure); - extra_spans.push(CoverageSpan::new(before, covspan.visible_macro, covspan.bcb, false)); - extra_spans.push(CoverageSpan::new(after, covspan.visible_macro, covspan.bcb, false)); + extra_spans.push(SpanFromMir::new(before, covspan.visible_macro, covspan.bcb, false)); + extra_spans.push(SpanFromMir::new(after, covspan.visible_macro, covspan.bcb, false)); false // Discard the original covspan that we just split. }); @@ -125,7 +127,7 @@ fn bcb_to_initial_coverage_spans<'a, 'tcx>( body_span: Span, bcb: BasicCoverageBlock, bcb_data: &'a BasicCoverageBlockData, -) -> impl Iterator + Captures<'a> + Captures<'tcx> { +) -> impl Iterator + Captures<'a> + Captures<'tcx> { bcb_data.basic_blocks.iter().flat_map(move |&bb| { let data = &mir_body[bb]; @@ -134,7 +136,7 @@ fn bcb_to_initial_coverage_spans<'a, 'tcx>( let (span, visible_macro) = unexpand_into_body_span_with_visible_macro(expn_span, body_span)?; - Some(CoverageSpan::new(span, visible_macro, bcb, is_closure_or_coroutine(statement))) + Some(SpanFromMir::new(span, visible_macro, bcb, is_closure_or_coroutine(statement))) }); let terminator_span = Some(data.terminator()).into_iter().filter_map(move |terminator| { @@ -142,7 +144,7 @@ fn bcb_to_initial_coverage_spans<'a, 'tcx>( let (span, visible_macro) = unexpand_into_body_span_with_visible_macro(expn_span, body_span)?; - Some(CoverageSpan::new(span, visible_macro, bcb, false)) + Some(SpanFromMir::new(span, visible_macro, bcb, false)) }); statement_spans.chain(terminator_span) @@ -308,3 +310,38 @@ fn unexpand_into_body_span_with_prev( Some((curr, prev)) } + +#[derive(Debug)] +struct SpanFromMir { + /// A span that has been extracted from MIR and then "un-expanded" back to + /// within the current function's `body_span`. After various intermediate + /// processing steps, this span is emitted as part of the final coverage + /// mappings. + /// + /// With the exception of `fn_sig_span`, this should always be contained + /// within `body_span`. + span: Span, + visible_macro: Option, + bcb: BasicCoverageBlock, + is_closure: bool, +} + +impl SpanFromMir { + fn for_fn_sig(fn_sig_span: Span) -> Self { + Self::new(fn_sig_span, None, START_BCB, false) + } + + fn new( + span: Span, + visible_macro: Option, + bcb: BasicCoverageBlock, + is_closure: bool, + ) -> Self { + Self { span, visible_macro, bcb, is_closure } + } + + fn into_coverage_span(self) -> CoverageSpan { + let Self { span, visible_macro: _, bcb, is_closure } = self; + CoverageSpan::new(span, bcb, is_closure) + } +} From 514e026853fcd426528cc23a7768d0e43f05ce7c Mon Sep 17 00:00:00 2001 From: Zalathar Date: Thu, 28 Dec 2023 14:49:16 +1100 Subject: [PATCH 10/23] coverage: Make the remaining fields of `CoverageSpan` non-public The struct itself is already non-public, so having public fields doesn't achieve anything. --- compiler/rustc_mir_transform/src/coverage/spans.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/compiler/rustc_mir_transform/src/coverage/spans.rs b/compiler/rustc_mir_transform/src/coverage/spans.rs index 6604e13398fa8..5983189984d91 100644 --- a/compiler/rustc_mir_transform/src/coverage/spans.rs +++ b/compiler/rustc_mir_transform/src/coverage/spans.rs @@ -68,12 +68,12 @@ impl CoverageSpans { /// `dominates()` the `BasicBlock`s in this `CoverageSpan`. #[derive(Debug, Clone)] struct CoverageSpan { - pub span: Span, - pub bcb: BasicCoverageBlock, + span: Span, + bcb: BasicCoverageBlock, /// List of all the original spans from MIR that have been merged into this /// span. Mainly used to precisely skip over gaps when truncating a span. - pub merged_spans: Vec, - pub is_closure: bool, + merged_spans: Vec, + is_closure: bool, } impl CoverageSpan { From ddfcf86867e4a8f29ea235ee700aa89a8b6ddf21 Mon Sep 17 00:00:00 2001 From: Georg Semmler Date: Thu, 21 Dec 2023 14:02:58 +0100 Subject: [PATCH 11/23] Allow emitting diagnostics from the `#[diagnostic]` namespace without a nightly feature (Using this attribute still requires a nightly feature, this just enables that this feature does not need to be enabled on the child crate as well) --- .../src/traits/error_reporting/on_unimplemented.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/compiler/rustc_trait_selection/src/traits/error_reporting/on_unimplemented.rs b/compiler/rustc_trait_selection/src/traits/error_reporting/on_unimplemented.rs index 52f91d282f0b6..c9e8b30c4c48c 100644 --- a/compiler/rustc_trait_selection/src/traits/error_reporting/on_unimplemented.rs +++ b/compiler/rustc_trait_selection/src/traits/error_reporting/on_unimplemented.rs @@ -521,7 +521,7 @@ impl<'tcx> OnUnimplementedDirective { pub fn of_item(tcx: TyCtxt<'tcx>, item_def_id: DefId) -> Result, ErrorGuaranteed> { if let Some(attr) = tcx.get_attr(item_def_id, sym::rustc_on_unimplemented) { return Self::parse_attribute(attr, false, tcx, item_def_id); - } else if tcx.features().diagnostic_namespace { + } else { tcx.get_attrs_by_path(item_def_id, &[sym::diagnostic, sym::on_unimplemented]) .filter_map(|attr| Self::parse_attribute(attr, true, tcx, item_def_id).transpose()) .try_fold(None, |aggr: Option, directive| { @@ -592,8 +592,6 @@ impl<'tcx> OnUnimplementedDirective { Ok(Some(directive)) } }) - } else { - Ok(None) } } From 2c3aeea1baf4eed29f131f53e852532d8e3833c3 Mon Sep 17 00:00:00 2001 From: Georg Semmler Date: Fri, 22 Dec 2023 14:30:12 +0100 Subject: [PATCH 12/23] Replace some usage of `#[rustc_on_unimplemented]` with `#[diagnostic::on_unimplemented]` This commit replaces those `#[rustc_on_unimplemented]` attributes with their equivalent `#[diagnostic::on_unimplemented]` where this is supported (So no filter or any extended option) --- library/core/src/future/future.rs | 2 +- library/core/src/lib.rs | 1 + library/core/src/marker.rs | 16 +++++++-------- library/core/src/ops/arith.rs | 16 +++++++-------- library/core/src/ops/bit.rs | 20 +++++++++---------- library/core/src/ops/index.rs | 2 +- library/core/src/panic/unwind_safe.rs | 4 ++-- tests/ui/proc-macro/meta-macro-hygiene.stdout | 3 +++ .../nonterminal-token-hygiene.stdout | 3 +++ 9 files changed, 37 insertions(+), 30 deletions(-) diff --git a/library/core/src/future/future.rs b/library/core/src/future/future.rs index 71b9464efd288..af2e422e8a00c 100644 --- a/library/core/src/future/future.rs +++ b/library/core/src/future/future.rs @@ -28,7 +28,7 @@ use crate::task::{Context, Poll}; #[must_use = "futures do nothing unless you `.await` or poll them"] #[stable(feature = "futures_api", since = "1.36.0")] #[lang = "future_trait"] -#[rustc_on_unimplemented( +#[diagnostic::on_unimplemented( label = "`{Self}` is not a future", message = "`{Self}` is not a future", note = "{Self} must be a future or must implement `IntoFuture` to be awaited" diff --git a/library/core/src/lib.rs b/library/core/src/lib.rs index 07720f235989b..9f8ae7a09f094 100644 --- a/library/core/src/lib.rs +++ b/library/core/src/lib.rs @@ -218,6 +218,7 @@ #![feature(const_trait_impl)] #![feature(decl_macro)] #![feature(deprecated_suggestion)] +#![feature(diagnostic_namespace)] #![feature(doc_cfg)] #![feature(doc_cfg_hide)] #![feature(doc_notable_trait)] diff --git a/library/core/src/marker.rs b/library/core/src/marker.rs index 99762bccd18f2..69d54f0640780 100644 --- a/library/core/src/marker.rs +++ b/library/core/src/marker.rs @@ -75,7 +75,7 @@ macro marker_impls { /// [ub]: ../../reference/behavior-considered-undefined.html #[stable(feature = "rust1", since = "1.0.0")] #[cfg_attr(not(test), rustc_diagnostic_item = "Send")] -#[rustc_on_unimplemented( +#[diagnostic::on_unimplemented( message = "`{Self}` cannot be sent between threads safely", label = "`{Self}` cannot be sent between threads safely" )] @@ -134,7 +134,7 @@ unsafe impl Send for &T {} #[doc(alias = "?", alias = "?Sized")] #[stable(feature = "rust1", since = "1.0.0")] #[lang = "sized"] -#[rustc_on_unimplemented( +#[diagnostic::on_unimplemented( message = "the size for values of type `{Self}` cannot be known at compilation time", label = "doesn't have a size known at compile-time" )] @@ -205,7 +205,7 @@ pub trait Unsize { /// [RFC1445]: https://github.com/rust-lang/rfcs/blob/master/text/1445-restrict-constants-in-patterns.md /// [issue 63438]: https://github.com/rust-lang/rust/issues/63438 #[unstable(feature = "structural_match", issue = "31434")] -#[rustc_on_unimplemented(message = "the type `{Self}` does not `#[derive(PartialEq)]`")] +#[diagnostic::on_unimplemented(message = "the type `{Self}` does not `#[derive(PartialEq)]`")] #[lang = "structural_peq"] pub trait StructuralPartialEq { // Empty. @@ -273,7 +273,7 @@ marker_impls! { /// of the two derives (`#[derive(PartialEq)]` and `#[derive(Eq)]`) and check /// that both of them are present as part of structural-match checking. #[unstable(feature = "structural_match", issue = "31434")] -#[rustc_on_unimplemented(message = "the type `{Self}` does not `#[derive(Eq)]`")] +#[diagnostic::on_unimplemented(message = "the type `{Self}` does not `#[derive(Eq)]`")] #[lang = "structural_teq"] pub trait StructuralEq { // Empty. @@ -941,7 +941,7 @@ marker_impls! { /// [Pin]: crate::pin::Pin /// [`pin` module]: crate::pin #[stable(feature = "pin", since = "1.33.0")] -#[rustc_on_unimplemented( +#[diagnostic::on_unimplemented( note = "consider using the `pin!` macro\nconsider using `Box::pin` if you need to access the pinned value outside of the current scope", message = "`{Self}` cannot be unpinned" )] @@ -989,7 +989,7 @@ pub trait Destruct {} /// for any user type. #[unstable(feature = "tuple_trait", issue = "none")] #[lang = "tuple_trait"] -#[rustc_on_unimplemented(message = "`{Self}` is not a tuple")] +#[diagnostic::on_unimplemented(message = "`{Self}` is not a tuple")] #[rustc_deny_explicit_impl(implement_via_object = false)] pub trait Tuple {} @@ -999,7 +999,7 @@ pub trait Tuple {} /// `*const ()` automatically implement this trait. #[unstable(feature = "pointer_like_trait", issue = "none")] #[lang = "pointer_like"] -#[rustc_on_unimplemented( +#[diagnostic::on_unimplemented( message = "`{Self}` needs to have the same ABI as a pointer", label = "`{Self}` needs to be a pointer-like type" )] @@ -1013,7 +1013,7 @@ pub trait PointerLike {} /// are `StructuralPartialEq`. #[lang = "const_param_ty"] #[unstable(feature = "adt_const_params", issue = "95174")] -#[rustc_on_unimplemented(message = "`{Self}` can't be used as a const parameter type")] +#[diagnostic::on_unimplemented(message = "`{Self}` can't be used as a const parameter type")] #[allow(multiple_supertrait_upcastable)] pub trait ConstParamTy: StructuralEq + StructuralPartialEq + Eq {} diff --git a/library/core/src/ops/arith.rs b/library/core/src/ops/arith.rs index 1773fdbf37cc7..bb3cdde66d173 100644 --- a/library/core/src/ops/arith.rs +++ b/library/core/src/ops/arith.rs @@ -307,7 +307,7 @@ sub_impl! { usize u8 u16 u32 u64 u128 isize i8 i16 i32 i64 i128 f32 f64 } /// ``` #[lang = "mul"] #[stable(feature = "rust1", since = "1.0.0")] -#[rustc_on_unimplemented( +#[diagnostic::on_unimplemented( message = "cannot multiply `{Self}` by `{Rhs}`", label = "no implementation for `{Self} * {Rhs}`" )] @@ -441,7 +441,7 @@ mul_impl! { usize u8 u16 u32 u64 u128 isize i8 i16 i32 i64 i128 f32 f64 } /// ``` #[lang = "div"] #[stable(feature = "rust1", since = "1.0.0")] -#[rustc_on_unimplemented( +#[diagnostic::on_unimplemented( message = "cannot divide `{Self}` by `{Rhs}`", label = "no implementation for `{Self} / {Rhs}`" )] @@ -543,7 +543,7 @@ div_impl_float! { f32 f64 } /// ``` #[lang = "rem"] #[stable(feature = "rust1", since = "1.0.0")] -#[rustc_on_unimplemented( +#[diagnostic::on_unimplemented( message = "cannot calculate the remainder of `{Self}` divided by `{Rhs}`", label = "no implementation for `{Self} % {Rhs}`" )] @@ -729,7 +729,7 @@ neg_impl! { isize i8 i16 i32 i64 i128 f32 f64 } /// ``` #[lang = "add_assign"] #[stable(feature = "op_assign_traits", since = "1.8.0")] -#[rustc_on_unimplemented( +#[diagnostic::on_unimplemented( message = "cannot add-assign `{Rhs}` to `{Self}`", label = "no implementation for `{Self} += {Rhs}`" )] @@ -796,7 +796,7 @@ add_assign_impl! { usize u8 u16 u32 u64 u128 isize i8 i16 i32 i64 i128 f32 f64 } /// ``` #[lang = "sub_assign"] #[stable(feature = "op_assign_traits", since = "1.8.0")] -#[rustc_on_unimplemented( +#[diagnostic::on_unimplemented( message = "cannot subtract-assign `{Rhs}` from `{Self}`", label = "no implementation for `{Self} -= {Rhs}`" )] @@ -854,7 +854,7 @@ sub_assign_impl! { usize u8 u16 u32 u64 u128 isize i8 i16 i32 i64 i128 f32 f64 } /// ``` #[lang = "mul_assign"] #[stable(feature = "op_assign_traits", since = "1.8.0")] -#[rustc_on_unimplemented( +#[diagnostic::on_unimplemented( message = "cannot multiply-assign `{Self}` by `{Rhs}`", label = "no implementation for `{Self} *= {Rhs}`" )] @@ -912,7 +912,7 @@ mul_assign_impl! { usize u8 u16 u32 u64 u128 isize i8 i16 i32 i64 i128 f32 f64 } /// ``` #[lang = "div_assign"] #[stable(feature = "op_assign_traits", since = "1.8.0")] -#[rustc_on_unimplemented( +#[diagnostic::on_unimplemented( message = "cannot divide-assign `{Self}` by `{Rhs}`", label = "no implementation for `{Self} /= {Rhs}`" )] @@ -973,7 +973,7 @@ div_assign_impl! { usize u8 u16 u32 u64 u128 isize i8 i16 i32 i64 i128 f32 f64 } /// ``` #[lang = "rem_assign"] #[stable(feature = "op_assign_traits", since = "1.8.0")] -#[rustc_on_unimplemented( +#[diagnostic::on_unimplemented( message = "cannot calculate and assign the remainder of `{Self}` divided by `{Rhs}`", label = "no implementation for `{Self} %= {Rhs}`" )] diff --git a/library/core/src/ops/bit.rs b/library/core/src/ops/bit.rs index c70f4a3da2ed8..6984100e498e8 100644 --- a/library/core/src/ops/bit.rs +++ b/library/core/src/ops/bit.rs @@ -137,7 +137,7 @@ impl Not for ! { #[lang = "bitand"] #[doc(alias = "&")] #[stable(feature = "rust1", since = "1.0.0")] -#[rustc_on_unimplemented( +#[diagnostic::on_unimplemented( message = "no implementation for `{Self} & {Rhs}`", label = "no implementation for `{Self} & {Rhs}`" )] @@ -237,7 +237,7 @@ bitand_impl! { bool usize u8 u16 u32 u64 u128 isize i8 i16 i32 i64 i128 } #[lang = "bitor"] #[doc(alias = "|")] #[stable(feature = "rust1", since = "1.0.0")] -#[rustc_on_unimplemented( +#[diagnostic::on_unimplemented( message = "no implementation for `{Self} | {Rhs}`", label = "no implementation for `{Self} | {Rhs}`" )] @@ -337,7 +337,7 @@ bitor_impl! { bool usize u8 u16 u32 u64 u128 isize i8 i16 i32 i64 i128 } #[lang = "bitxor"] #[doc(alias = "^")] #[stable(feature = "rust1", since = "1.0.0")] -#[rustc_on_unimplemented( +#[diagnostic::on_unimplemented( message = "no implementation for `{Self} ^ {Rhs}`", label = "no implementation for `{Self} ^ {Rhs}`" )] @@ -436,7 +436,7 @@ bitxor_impl! { bool usize u8 u16 u32 u64 u128 isize i8 i16 i32 i64 i128 } #[lang = "shl"] #[doc(alias = "<<")] #[stable(feature = "rust1", since = "1.0.0")] -#[rustc_on_unimplemented( +#[diagnostic::on_unimplemented( message = "no implementation for `{Self} << {Rhs}`", label = "no implementation for `{Self} << {Rhs}`" )] @@ -554,7 +554,7 @@ shl_impl_all! { u8 u16 u32 u64 u128 usize i8 i16 i32 i64 isize i128 } #[lang = "shr"] #[doc(alias = ">>")] #[stable(feature = "rust1", since = "1.0.0")] -#[rustc_on_unimplemented( +#[diagnostic::on_unimplemented( message = "no implementation for `{Self} >> {Rhs}`", label = "no implementation for `{Self} >> {Rhs}`" )] @@ -681,7 +681,7 @@ shr_impl_all! { u8 u16 u32 u64 u128 usize i8 i16 i32 i64 i128 isize } #[lang = "bitand_assign"] #[doc(alias = "&=")] #[stable(feature = "op_assign_traits", since = "1.8.0")] -#[rustc_on_unimplemented( +#[diagnostic::on_unimplemented( message = "no implementation for `{Self} &= {Rhs}`", label = "no implementation for `{Self} &= {Rhs}`" )] @@ -752,7 +752,7 @@ bitand_assign_impl! { bool usize u8 u16 u32 u64 u128 isize i8 i16 i32 i64 i128 } #[lang = "bitor_assign"] #[doc(alias = "|=")] #[stable(feature = "op_assign_traits", since = "1.8.0")] -#[rustc_on_unimplemented( +#[diagnostic::on_unimplemented( message = "no implementation for `{Self} |= {Rhs}`", label = "no implementation for `{Self} |= {Rhs}`" )] @@ -823,7 +823,7 @@ bitor_assign_impl! { bool usize u8 u16 u32 u64 u128 isize i8 i16 i32 i64 i128 } #[lang = "bitxor_assign"] #[doc(alias = "^=")] #[stable(feature = "op_assign_traits", since = "1.8.0")] -#[rustc_on_unimplemented( +#[diagnostic::on_unimplemented( message = "no implementation for `{Self} ^= {Rhs}`", label = "no implementation for `{Self} ^= {Rhs}`" )] @@ -892,7 +892,7 @@ bitxor_assign_impl! { bool usize u8 u16 u32 u64 u128 isize i8 i16 i32 i64 i128 } #[lang = "shl_assign"] #[doc(alias = "<<=")] #[stable(feature = "op_assign_traits", since = "1.8.0")] -#[rustc_on_unimplemented( +#[diagnostic::on_unimplemented( message = "no implementation for `{Self} <<= {Rhs}`", label = "no implementation for `{Self} <<= {Rhs}`" )] @@ -974,7 +974,7 @@ shl_assign_impl_all! { u8 u16 u32 u64 u128 usize i8 i16 i32 i64 i128 isize } #[lang = "shr_assign"] #[doc(alias = ">>=")] #[stable(feature = "op_assign_traits", since = "1.8.0")] -#[rustc_on_unimplemented( +#[diagnostic::on_unimplemented( message = "no implementation for `{Self} >>= {Rhs}`", label = "no implementation for `{Self} >>= {Rhs}`" )] diff --git a/library/core/src/ops/index.rs b/library/core/src/ops/index.rs index 6ceee46372980..37d9a28fb99c0 100644 --- a/library/core/src/ops/index.rs +++ b/library/core/src/ops/index.rs @@ -47,7 +47,7 @@ /// assert_eq!(nucleotide_count[Nucleotide::T], 12); /// ``` #[lang = "index"] -#[rustc_on_unimplemented( +#[diagnostic::on_unimplemented( message = "the type `{Self}` cannot be indexed by `{Idx}`", label = "`{Self}` cannot be indexed by `{Idx}`" )] diff --git a/library/core/src/panic/unwind_safe.rs b/library/core/src/panic/unwind_safe.rs index 6a53909a8f128..37859212c0ee3 100644 --- a/library/core/src/panic/unwind_safe.rs +++ b/library/core/src/panic/unwind_safe.rs @@ -83,7 +83,7 @@ use crate::task::{Context, Poll}; /// implemented for any closed over variables passed to `catch_unwind`. #[stable(feature = "catch_unwind", since = "1.9.0")] #[cfg_attr(not(test), rustc_diagnostic_item = "unwind_safe_trait")] -#[rustc_on_unimplemented( +#[diagnostic::on_unimplemented( message = "the type `{Self}` may not be safely transferred across an unwind boundary", label = "`{Self}` may not be safely transferred across an unwind boundary" )] @@ -99,7 +99,7 @@ pub auto trait UnwindSafe {} /// [`UnwindSafe`] trait, for more information see that documentation. #[stable(feature = "catch_unwind", since = "1.9.0")] #[cfg_attr(not(test), rustc_diagnostic_item = "ref_unwind_safe_trait")] -#[rustc_on_unimplemented( +#[diagnostic::on_unimplemented( message = "the type `{Self}` may contain interior mutability and a reference may not be safely \ transferrable across a catch_unwind boundary", label = "`{Self}` may contain interior mutability and a reference may not be safely \ diff --git a/tests/ui/proc-macro/meta-macro-hygiene.stdout b/tests/ui/proc-macro/meta-macro-hygiene.stdout index 6d10cc604c27f..3672a3590fd7a 100644 --- a/tests/ui/proc-macro/meta-macro-hygiene.stdout +++ b/tests/ui/proc-macro/meta-macro-hygiene.stdout @@ -50,6 +50,9 @@ crate0::{{expn1}}: parent: crate0::{{expn0}}, call_site_ctxt: #0, def_site_ctxt: crate0::{{expn2}}: parent: crate0::{{expn0}}, call_site_ctxt: #0, def_site_ctxt: #0, kind: Macro(Bang, "produce_it") crate0::{{expn3}}: parent: crate0::{{expn2}}, call_site_ctxt: #3, def_site_ctxt: #0, kind: Macro(Bang, "meta_macro::print_def_site") crate0::{{expn4}}: parent: crate0::{{expn3}}, call_site_ctxt: #4, def_site_ctxt: #0, kind: Macro(Bang, "$crate::dummy") +crate1::{{expnNNN}}: parent: crate0::{{expn0}}, call_site_ctxt: #0, def_site_ctxt: #0, kind: Macro(Attr, "diagnostic::on_unimplemented") +crate1::{{expnNNN}}: parent: crate0::{{expn0}}, call_site_ctxt: #0, def_site_ctxt: #0, kind: Macro(Attr, "diagnostic::on_unimplemented") +crate1::{{expnNNN}}: parent: crate0::{{expn0}}, call_site_ctxt: #0, def_site_ctxt: #0, kind: Macro(Attr, "diagnostic::on_unimplemented") crate1::{{expnNNN}}: parent: crate0::{{expn0}}, call_site_ctxt: #0, def_site_ctxt: #0, kind: Macro(Attr, "derive") crate1::{{expnNNN}}: parent: crate0::{{expn0}}, call_site_ctxt: #0, def_site_ctxt: #0, kind: Macro(Attr, "derive") crate1::{{expnNNN}}: parent: crate0::{{expn0}}, call_site_ctxt: #0, def_site_ctxt: #0, kind: Macro(Bang, "include") diff --git a/tests/ui/proc-macro/nonterminal-token-hygiene.stdout b/tests/ui/proc-macro/nonterminal-token-hygiene.stdout index 5c70e780f7489..d3c2c46ac75cf 100644 --- a/tests/ui/proc-macro/nonterminal-token-hygiene.stdout +++ b/tests/ui/proc-macro/nonterminal-token-hygiene.stdout @@ -73,6 +73,9 @@ crate0::{{expn1}}: parent: crate0::{{expn0}}, call_site_ctxt: #0, def_site_ctxt: crate0::{{expn2}}: parent: crate0::{{expn0}}, call_site_ctxt: #0, def_site_ctxt: #0, kind: Macro(Bang, "outer") crate0::{{expn3}}: parent: crate0::{{expn2}}, call_site_ctxt: #3, def_site_ctxt: #3, kind: Macro(Bang, "inner") crate0::{{expn4}}: parent: crate0::{{expn3}}, call_site_ctxt: #5, def_site_ctxt: #0, kind: Macro(Bang, "print_bang") +crate1::{{expnNNN}}: parent: crate0::{{expn0}}, call_site_ctxt: #0, def_site_ctxt: #0, kind: Macro(Attr, "diagnostic::on_unimplemented") +crate1::{{expnNNN}}: parent: crate0::{{expn0}}, call_site_ctxt: #0, def_site_ctxt: #0, kind: Macro(Attr, "diagnostic::on_unimplemented") +crate1::{{expnNNN}}: parent: crate0::{{expn0}}, call_site_ctxt: #0, def_site_ctxt: #0, kind: Macro(Attr, "diagnostic::on_unimplemented") crate1::{{expnNNN}}: parent: crate0::{{expn0}}, call_site_ctxt: #0, def_site_ctxt: #0, kind: Macro(Attr, "derive") crate1::{{expnNNN}}: parent: crate0::{{expn0}}, call_site_ctxt: #0, def_site_ctxt: #0, kind: Macro(Attr, "derive") crate1::{{expnNNN}}: parent: crate0::{{expn0}}, call_site_ctxt: #0, def_site_ctxt: #0, kind: Macro(Bang, "include") From 91e1af38623e05c52591aae2aa4a213553907ea0 Mon Sep 17 00:00:00 2001 From: Georg Semmler Date: Sat, 23 Dec 2023 14:02:01 +0100 Subject: [PATCH 13/23] Add a test that emitting diagnostics does not require the crate to use the corresponding feature. --- .../on_unimplemented/auxiliary/other.rs | 8 ++++++++ .../error_is_shown_in_downstream_crates.rs | 12 ++++++++++++ ...error_is_shown_in_downstream_crates.stderr | 19 +++++++++++++++++++ 3 files changed, 39 insertions(+) create mode 100644 tests/ui/diagnostic_namespace/on_unimplemented/auxiliary/other.rs create mode 100644 tests/ui/diagnostic_namespace/on_unimplemented/error_is_shown_in_downstream_crates.rs create mode 100644 tests/ui/diagnostic_namespace/on_unimplemented/error_is_shown_in_downstream_crates.stderr diff --git a/tests/ui/diagnostic_namespace/on_unimplemented/auxiliary/other.rs b/tests/ui/diagnostic_namespace/on_unimplemented/auxiliary/other.rs new file mode 100644 index 0000000000000..884bab2800a5b --- /dev/null +++ b/tests/ui/diagnostic_namespace/on_unimplemented/auxiliary/other.rs @@ -0,0 +1,8 @@ +#![feature(diagnostic_namespace)] + +#[diagnostic::on_unimplemented( + message = "Message", + note = "Note", + label = "label" +)] +pub trait Foo {} diff --git a/tests/ui/diagnostic_namespace/on_unimplemented/error_is_shown_in_downstream_crates.rs b/tests/ui/diagnostic_namespace/on_unimplemented/error_is_shown_in_downstream_crates.rs new file mode 100644 index 0000000000000..b39375a09f369 --- /dev/null +++ b/tests/ui/diagnostic_namespace/on_unimplemented/error_is_shown_in_downstream_crates.rs @@ -0,0 +1,12 @@ +// aux-build:other.rs + +extern crate other; + +use other::Foo; + +fn take_foo(_: impl Foo) {} + +fn main() { + take_foo(()); + //~^ERROR Message +} diff --git a/tests/ui/diagnostic_namespace/on_unimplemented/error_is_shown_in_downstream_crates.stderr b/tests/ui/diagnostic_namespace/on_unimplemented/error_is_shown_in_downstream_crates.stderr new file mode 100644 index 0000000000000..a9968538d0d38 --- /dev/null +++ b/tests/ui/diagnostic_namespace/on_unimplemented/error_is_shown_in_downstream_crates.stderr @@ -0,0 +1,19 @@ +error[E0277]: Message + --> $DIR/error_is_shown_in_downstream_crates.rs:10:14 + | +LL | take_foo(()); + | -------- ^^ label + | | + | required by a bound introduced by this call + | + = help: the trait `Foo` is not implemented for `()` + = note: Note +note: required by a bound in `take_foo` + --> $DIR/error_is_shown_in_downstream_crates.rs:7:21 + | +LL | fn take_foo(_: impl Foo) {} + | ^^^ required by this bound in `take_foo` + +error: aborting due to 1 previous error + +For more information about this error, try `rustc --explain E0277`. From 97c82385114dfb8aa30b0fd63aa06c201aeb65d9 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Fri, 5 Jan 2024 18:49:25 +0100 Subject: [PATCH 14/23] remove duplicate test --- tests/ui/numbers-arithmetic/overflowing-rsh-6.rs | 9 --------- .../ui/numbers-arithmetic/overflowing-rsh-6.stderr | 14 -------------- 2 files changed, 23 deletions(-) delete mode 100644 tests/ui/numbers-arithmetic/overflowing-rsh-6.rs delete mode 100644 tests/ui/numbers-arithmetic/overflowing-rsh-6.stderr diff --git a/tests/ui/numbers-arithmetic/overflowing-rsh-6.rs b/tests/ui/numbers-arithmetic/overflowing-rsh-6.rs deleted file mode 100644 index f75e779ed158c..0000000000000 --- a/tests/ui/numbers-arithmetic/overflowing-rsh-6.rs +++ /dev/null @@ -1,9 +0,0 @@ -// build-fail -// compile-flags: -C debug-assertions - -#![deny(arithmetic_overflow)] - -fn main() { - let _n = 1i64 >> [64][0]; - //~^ ERROR: this arithmetic operation will overflow -} diff --git a/tests/ui/numbers-arithmetic/overflowing-rsh-6.stderr b/tests/ui/numbers-arithmetic/overflowing-rsh-6.stderr deleted file mode 100644 index 005f7378226c1..0000000000000 --- a/tests/ui/numbers-arithmetic/overflowing-rsh-6.stderr +++ /dev/null @@ -1,14 +0,0 @@ -error: this arithmetic operation will overflow - --> $DIR/overflowing-rsh-6.rs:7:14 - | -LL | let _n = 1i64 >> [64][0]; - | ^^^^^^^^^^^^^^^ attempt to shift right by `64_i32`, which would overflow - | -note: the lint level is defined here - --> $DIR/overflowing-rsh-6.rs:4:9 - | -LL | #![deny(arithmetic_overflow)] - | ^^^^^^^^^^^^^^^^^^^ - -error: aborting due to 1 previous error - From 54967d7a680bc72bea2c1ff6f1edc0d7d2ca6d41 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Le=C3=B3n=20Orell=20Valerian=20Liehr?= Date: Fri, 5 Jan 2024 20:33:16 +0100 Subject: [PATCH 15/23] Ignore a rustdoc test --- tests/rustdoc/synthetic_auto/no-redundancy.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/rustdoc/synthetic_auto/no-redundancy.rs b/tests/rustdoc/synthetic_auto/no-redundancy.rs index ea57d7388b85f..fed9c9c7ba477 100644 --- a/tests/rustdoc/synthetic_auto/no-redundancy.rs +++ b/tests/rustdoc/synthetic_auto/no-redundancy.rs @@ -1,3 +1,6 @@ +// FIXME(fmease, #119216): Reenable this test! +// ignore-test + pub struct Inner { field: T, } From 004bfc5eb2aa6789741055d3e6c479e34eb960cb Mon Sep 17 00:00:00 2001 From: Michael Howell Date: Fri, 5 Jan 2024 12:40:11 -0700 Subject: [PATCH 16/23] Add notes about the serialization format --- src/librustdoc/html/render/mod.rs | 31 ++++++++++++++++++++----- src/librustdoc/html/static/js/search.js | 2 +- 2 files changed, 26 insertions(+), 7 deletions(-) diff --git a/src/librustdoc/html/render/mod.rs b/src/librustdoc/html/render/mod.rs index 8afeec81d8731..bea5ccd7c860d 100644 --- a/src/librustdoc/html/render/mod.rs +++ b/src/librustdoc/html/render/mod.rs @@ -131,6 +131,10 @@ pub(crate) struct RenderType { } impl RenderType { + // Types are rendered as lists of lists, because that's pretty compact. + // The contents of the lists are always integers in self-terminating hex + // form, handled by `RenderTypeId::write_to_string`, so no commas are + // needed to separate the items. pub fn write_to_string(&self, string: &mut String) { fn write_optional_id(id: Option, string: &mut String) { // 0 is a sentinel, everything else is one-indexed @@ -139,6 +143,9 @@ impl RenderType { None => string.push('`'), } } + // Either just the type id, or `{type, generics, bindings?}` + // where generics is a list of types, + // and bindings is a list of `{id, typelist}` pairs. if self.generics.is_some() || self.bindings.is_some() { string.push('{'); write_optional_id(self.id, string); @@ -186,10 +193,19 @@ impl RenderTypeId { RenderTypeId::Index(idx) => (true, (-*idx).try_into().unwrap()), _ => panic!("must convert render types to indexes before serializing"), }; - // zig-zag notation + // zig-zag encoding let value: u32 = (id << 1) | (if sign { 1 } else { 0 }); - // encode - // Documented in https://rust-lang.github.io/rustc-dev-guide/rustdoc-internals/search.html + // Self-terminating hex use capital letters for everything but the + // least significant digit, which is lowercase. For example, decimal 17 + // would be `` Aa `` if zig-zag encoding weren't used. + // + // Zig-zag encoding, however, stores the sign bit as the last bit. + // This means, in the last hexit, 1 is actually `c`, -1 is `b` + // (`a` is the imaginary -0), and, because all the bits are shifted + // by one, `` A` `` is actually 8 and `` Aa `` is -8. + // + // https://rust-lang.github.io/rustc-dev-guide/rustdoc-internals/search.html + // describes the encoding in more detail. let mut shift: u32 = 28; let mut mask: u32 = 0xF0_00_00_00; while shift < 32 { @@ -219,8 +235,9 @@ impl IndexItemFunctionType { string: &mut String, backref_queue: &mut VecDeque<&'a IndexItemFunctionType>, ) { - assert!(backref_queue.len() < 16); - // If we couldn't figure out a type, just write `0`. + assert!(backref_queue.len() <= 16); + // If we couldn't figure out a type, just write 0, + // which is encoded as `` ` `` (see RenderTypeId::write_to_string). let has_missing = self .inputs .iter() @@ -229,13 +246,15 @@ impl IndexItemFunctionType { if has_missing { string.push('`'); } else if let Some(idx) = backref_queue.iter().position(|other| *other == self) { + // The backref queue has 16 items, so backrefs use + // a single hexit, disjoint from the ones used for numbers. string.push( char::try_from('0' as u32 + u32::try_from(idx).unwrap()) .expect("last possible value is '?'"), ); } else { backref_queue.push_front(self); - if backref_queue.len() >= 16 { + if backref_queue.len() > 16 { backref_queue.pop_back(); } string.push('{'); diff --git a/src/librustdoc/html/static/js/search.js b/src/librustdoc/html/static/js/search.js index 6fb92d8fbb123..e0708485fc058 100644 --- a/src/librustdoc/html/static/js/search.js +++ b/src/librustdoc/html/static/js/search.js @@ -2858,7 +2858,7 @@ ${item.displayPath}${name}\ inputs, output, where_clause, }; itemFunctionDecoder.backrefQueue.unshift(ret); - if (itemFunctionDecoder.backrefQueue.length >= 16) { + if (itemFunctionDecoder.backrefQueue.length > 16) { itemFunctionDecoder.backrefQueue.pop(); } return ret; From 274674819c3c37e09f5fe0e3276d3921068dbb95 Mon Sep 17 00:00:00 2001 From: Lukas Markeffsky <@> Date: Fri, 5 Jan 2024 21:50:52 +0100 Subject: [PATCH 17/23] fix OOM when `ty::Instance` is used in query description --- compiler/rustc_middle/src/ty/instance.rs | 12 ++++++++---- .../auxiliary/suggest-constructor-cycle-error.rs | 12 ++++++++++++ .../ui/resolve/suggest-constructor-cycle-error.rs | 10 ++++++++++ .../suggest-constructor-cycle-error.stderr | 15 +++++++++++++++ 4 files changed, 45 insertions(+), 4 deletions(-) create mode 100644 tests/ui/resolve/auxiliary/suggest-constructor-cycle-error.rs create mode 100644 tests/ui/resolve/suggest-constructor-cycle-error.rs create mode 100644 tests/ui/resolve/suggest-constructor-cycle-error.stderr diff --git a/compiler/rustc_middle/src/ty/instance.rs b/compiler/rustc_middle/src/ty/instance.rs index 2ac3cddfa15ad..dd41cb5a61f44 100644 --- a/compiler/rustc_middle/src/ty/instance.rs +++ b/compiler/rustc_middle/src/ty/instance.rs @@ -293,12 +293,16 @@ impl<'tcx> InstanceDef<'tcx> { fn fmt_instance( f: &mut fmt::Formatter<'_>, instance: &Instance<'_>, - type_length: rustc_session::Limit, + type_length: Option, ) -> fmt::Result { ty::tls::with(|tcx| { let args = tcx.lift(instance.args).expect("could not lift for printing"); - let mut cx = FmtPrinter::new_with_limit(tcx, Namespace::ValueNS, type_length); + let mut cx = if let Some(type_length) = type_length { + FmtPrinter::new_with_limit(tcx, Namespace::ValueNS, type_length) + } else { + FmtPrinter::new(tcx, Namespace::ValueNS) + }; cx.print_def_path(instance.def_id(), args)?; let s = cx.into_buffer(); f.write_str(&s) @@ -324,13 +328,13 @@ pub struct ShortInstance<'a, 'tcx>(pub &'a Instance<'tcx>, pub usize); impl<'a, 'tcx> fmt::Display for ShortInstance<'a, 'tcx> { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - fmt_instance(f, self.0, rustc_session::Limit(self.1)) + fmt_instance(f, self.0, Some(rustc_session::Limit(self.1))) } } impl<'tcx> fmt::Display for Instance<'tcx> { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - ty::tls::with(|tcx| fmt_instance(f, self, tcx.type_length_limit())) + fmt_instance(f, self, None) } } diff --git a/tests/ui/resolve/auxiliary/suggest-constructor-cycle-error.rs b/tests/ui/resolve/auxiliary/suggest-constructor-cycle-error.rs new file mode 100644 index 0000000000000..8de68c38bc34d --- /dev/null +++ b/tests/ui/resolve/auxiliary/suggest-constructor-cycle-error.rs @@ -0,0 +1,12 @@ +mod m { + pub struct Uuid(()); + + impl Uuid { + pub fn encode_buffer() -> [u8; LENGTH] { + [] + } + } + const LENGTH: usize = 0; +} + +pub use m::Uuid; diff --git a/tests/ui/resolve/suggest-constructor-cycle-error.rs b/tests/ui/resolve/suggest-constructor-cycle-error.rs new file mode 100644 index 0000000000000..2336cd92f4d33 --- /dev/null +++ b/tests/ui/resolve/suggest-constructor-cycle-error.rs @@ -0,0 +1,10 @@ +// aux-build:suggest-constructor-cycle-error.rs +//~^ cycle detected when getting the resolver for lowering [E0391] + +// Regression test for https://github.com/rust-lang/rust/issues/119625 + +extern crate suggest_constructor_cycle_error as a; + +const CONST_NAME: a::Uuid = a::Uuid(()); + +fn main() {} diff --git a/tests/ui/resolve/suggest-constructor-cycle-error.stderr b/tests/ui/resolve/suggest-constructor-cycle-error.stderr new file mode 100644 index 0000000000000..ae139619c695c --- /dev/null +++ b/tests/ui/resolve/suggest-constructor-cycle-error.stderr @@ -0,0 +1,15 @@ +error[E0391]: cycle detected when getting the resolver for lowering + | + = note: ...which requires normalizing `[u8; suggest_constructor_cycle_error::::m::{impl#0}::encode_buffer::{constant#0}]`... +note: ...which requires resolving instance `suggest_constructor_cycle_error::m::Uuid::encode_buffer::{constant#0}`... + --> $DIR/auxiliary/suggest-constructor-cycle-error.rs:5:40 + | +LL | pub fn encode_buffer() -> [u8; LENGTH] { + | ^^^^^^ + = note: ...which requires calculating the lang items map... + = note: ...which again requires getting the resolver for lowering, completing the cycle + = note: see https://rustc-dev-guide.rust-lang.org/overview.html#queries and https://rustc-dev-guide.rust-lang.org/query.html for more information + +error: aborting due to 1 previous error + +For more information about this error, try `rustc --explain E0391`. From 339fa311ad8140cfb98a1c6080ea7e002b9ce3fa Mon Sep 17 00:00:00 2001 From: Lukas Markeffsky <@> Date: Fri, 5 Jan 2024 21:55:09 +0100 Subject: [PATCH 18/23] fix cycle error for "use constructor" suggestion --- .../rustc_resolve/src/late/diagnostics.rs | 7 ++----- .../suggest-constructor-cycle-error.rs | 2 +- .../suggest-constructor-cycle-error.stderr | 20 +++++++++---------- 3 files changed, 13 insertions(+), 16 deletions(-) diff --git a/compiler/rustc_resolve/src/late/diagnostics.rs b/compiler/rustc_resolve/src/late/diagnostics.rs index 6c38ed6227038..1ccb4820cdd9f 100644 --- a/compiler/rustc_resolve/src/late/diagnostics.rs +++ b/compiler/rustc_resolve/src/late/diagnostics.rs @@ -1755,11 +1755,8 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> { .filter_map(|item| { // Only assoc fns that return `Self` let fn_sig = self.r.tcx.fn_sig(item.def_id).skip_binder(); - let ret_ty = fn_sig.output(); - let ret_ty = self - .r - .tcx - .normalize_erasing_late_bound_regions(ty::ParamEnv::reveal_all(), ret_ty); + // Don't normalize the return type, because that can cause cycle errors. + let ret_ty = fn_sig.output().skip_binder(); let ty::Adt(def, _args) = ret_ty.kind() else { return None; }; diff --git a/tests/ui/resolve/suggest-constructor-cycle-error.rs b/tests/ui/resolve/suggest-constructor-cycle-error.rs index 2336cd92f4d33..e36fffd97d1b4 100644 --- a/tests/ui/resolve/suggest-constructor-cycle-error.rs +++ b/tests/ui/resolve/suggest-constructor-cycle-error.rs @@ -1,10 +1,10 @@ // aux-build:suggest-constructor-cycle-error.rs -//~^ cycle detected when getting the resolver for lowering [E0391] // Regression test for https://github.com/rust-lang/rust/issues/119625 extern crate suggest_constructor_cycle_error as a; const CONST_NAME: a::Uuid = a::Uuid(()); +//~^ ERROR: cannot initialize a tuple struct which contains private fields [E0423] fn main() {} diff --git a/tests/ui/resolve/suggest-constructor-cycle-error.stderr b/tests/ui/resolve/suggest-constructor-cycle-error.stderr index ae139619c695c..c6ec2465a432c 100644 --- a/tests/ui/resolve/suggest-constructor-cycle-error.stderr +++ b/tests/ui/resolve/suggest-constructor-cycle-error.stderr @@ -1,15 +1,15 @@ -error[E0391]: cycle detected when getting the resolver for lowering +error[E0423]: cannot initialize a tuple struct which contains private fields + --> $DIR/suggest-constructor-cycle-error.rs:7:29 | - = note: ...which requires normalizing `[u8; suggest_constructor_cycle_error::::m::{impl#0}::encode_buffer::{constant#0}]`... -note: ...which requires resolving instance `suggest_constructor_cycle_error::m::Uuid::encode_buffer::{constant#0}`... - --> $DIR/auxiliary/suggest-constructor-cycle-error.rs:5:40 +LL | const CONST_NAME: a::Uuid = a::Uuid(()); + | ^^^^^^^ | -LL | pub fn encode_buffer() -> [u8; LENGTH] { - | ^^^^^^ - = note: ...which requires calculating the lang items map... - = note: ...which again requires getting the resolver for lowering, completing the cycle - = note: see https://rustc-dev-guide.rust-lang.org/overview.html#queries and https://rustc-dev-guide.rust-lang.org/query.html for more information +note: constructor is not visible here due to private fields + --> $DIR/auxiliary/suggest-constructor-cycle-error.rs:2:21 + | +LL | pub struct Uuid(()); + | ^^ private field error: aborting due to 1 previous error -For more information about this error, try `rustc --explain E0391`. +For more information about this error, try `rustc --explain E0423`. From 7366bdaea2757b7b45965f4b59a8137adf7d10af Mon Sep 17 00:00:00 2001 From: Camille GILLOT Date: Fri, 29 Dec 2023 20:04:03 +0000 Subject: [PATCH 19/23] Handle ForeignItem as TAIT scope. --- .../src/collect/type_of/opaque.rs | 7 +++++++ .../nested-in-anon-const.rs | 21 +++++++++++++++++++ .../nested-in-anon-const.stderr | 20 ++++++++++++++++++ 3 files changed, 48 insertions(+) create mode 100644 tests/ui/type-alias-impl-trait/nested-in-anon-const.rs create mode 100644 tests/ui/type-alias-impl-trait/nested-in-anon-const.stderr diff --git a/compiler/rustc_hir_analysis/src/collect/type_of/opaque.rs b/compiler/rustc_hir_analysis/src/collect/type_of/opaque.rs index 5a73097b0f6b3..04047e56c60ea 100644 --- a/compiler/rustc_hir_analysis/src/collect/type_of/opaque.rs +++ b/compiler/rustc_hir_analysis/src/collect/type_of/opaque.rs @@ -69,6 +69,7 @@ pub(super) fn find_opaque_ty_constraints_for_tait(tcx: TyCtxt<'_>, def_id: Local Node::Item(it) => locator.visit_item(it), Node::ImplItem(it) => locator.visit_impl_item(it), Node::TraitItem(it) => locator.visit_trait_item(it), + Node::ForeignItem(it) => locator.visit_foreign_item(it), other => bug!("{:?} is not a valid scope for an opaque type item", other), } } @@ -240,6 +241,12 @@ impl<'tcx> intravisit::Visitor<'tcx> for TaitConstraintLocator<'tcx> { self.check(it.owner_id.def_id); intravisit::walk_trait_item(self, it); } + fn visit_foreign_item(&mut self, it: &'tcx hir::ForeignItem<'tcx>) { + trace!(?it.owner_id); + assert_ne!(it.owner_id.def_id, self.def_id); + self.check(it.owner_id.def_id); + intravisit::walk_foreign_item(self, it); + } } pub(super) fn find_opaque_ty_constraints_for_rpit<'tcx>( diff --git a/tests/ui/type-alias-impl-trait/nested-in-anon-const.rs b/tests/ui/type-alias-impl-trait/nested-in-anon-const.rs new file mode 100644 index 0000000000000..e9d53c99d041b --- /dev/null +++ b/tests/ui/type-alias-impl-trait/nested-in-anon-const.rs @@ -0,0 +1,21 @@ +// Regression test for issue #119295. + +#![feature(type_alias_impl_trait)] + +type Bar = T; +type S = [i32; A]; + +extern "C" { + pub fn lint_me( + x: Bar< + S< + { //~ ERROR mismatched types + type B = impl Sized; + //~^ ERROR unconstrained opaque type + }, + >, + >, + ); +} + +fn main() {} diff --git a/tests/ui/type-alias-impl-trait/nested-in-anon-const.stderr b/tests/ui/type-alias-impl-trait/nested-in-anon-const.stderr new file mode 100644 index 0000000000000..aa0c1076117cc --- /dev/null +++ b/tests/ui/type-alias-impl-trait/nested-in-anon-const.stderr @@ -0,0 +1,20 @@ +error[E0308]: mismatched types + --> $DIR/nested-in-anon-const.rs:12:17 + | +LL | / { +LL | | type B = impl Sized; +LL | | +LL | | }, + | |_________________^ expected `usize`, found `()` + +error: unconstrained opaque type + --> $DIR/nested-in-anon-const.rs:13:33 + | +LL | type B = impl Sized; + | ^^^^^^^^^^ + | + = note: `B` must be used in combination with a concrete type within the same item + +error: aborting due to 2 previous errors + +For more information about this error, try `rustc --explain E0308`. From 6dfdeab65a5e08efa811e033417bd7aef5d3c1fa Mon Sep 17 00:00:00 2001 From: Camille GILLOT Date: Sun, 31 Dec 2023 00:22:52 +0000 Subject: [PATCH 20/23] Do not run check on foreign items. --- compiler/rustc_hir_analysis/src/collect/type_of/opaque.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/rustc_hir_analysis/src/collect/type_of/opaque.rs b/compiler/rustc_hir_analysis/src/collect/type_of/opaque.rs index 04047e56c60ea..da7279967dac1 100644 --- a/compiler/rustc_hir_analysis/src/collect/type_of/opaque.rs +++ b/compiler/rustc_hir_analysis/src/collect/type_of/opaque.rs @@ -244,7 +244,7 @@ impl<'tcx> intravisit::Visitor<'tcx> for TaitConstraintLocator<'tcx> { fn visit_foreign_item(&mut self, it: &'tcx hir::ForeignItem<'tcx>) { trace!(?it.owner_id); assert_ne!(it.owner_id.def_id, self.def_id); - self.check(it.owner_id.def_id); + // No need to call `check`, as we do not run borrowck on foreign items. intravisit::walk_foreign_item(self, it); } } From eeaea576008f53e259c0f86cdd37f28bed9034a1 Mon Sep 17 00:00:00 2001 From: Camille GILLOT Date: Fri, 5 Jan 2024 21:54:41 +0000 Subject: [PATCH 21/23] Rebase fallout. --- .../nested-in-anon-const.stderr | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/tests/ui/type-alias-impl-trait/nested-in-anon-const.stderr b/tests/ui/type-alias-impl-trait/nested-in-anon-const.stderr index aa0c1076117cc..d0fe920b35f4a 100644 --- a/tests/ui/type-alias-impl-trait/nested-in-anon-const.stderr +++ b/tests/ui/type-alias-impl-trait/nested-in-anon-const.stderr @@ -1,3 +1,11 @@ +error: unconstrained opaque type + --> $DIR/nested-in-anon-const.rs:13:33 + | +LL | type B = impl Sized; + | ^^^^^^^^^^ + | + = note: `B` must be used in combination with a concrete type within the same item + error[E0308]: mismatched types --> $DIR/nested-in-anon-const.rs:12:17 | @@ -7,14 +15,6 @@ LL | | LL | | }, | |_________________^ expected `usize`, found `()` -error: unconstrained opaque type - --> $DIR/nested-in-anon-const.rs:13:33 - | -LL | type B = impl Sized; - | ^^^^^^^^^^ - | - = note: `B` must be used in combination with a concrete type within the same item - error: aborting due to 2 previous errors For more information about this error, try `rustc --explain E0308`. From 0489fd05d8562a7ccaeb79d0566b45bc463a0853 Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Sat, 6 Jan 2024 01:32:03 +0300 Subject: [PATCH 22/23] library: Fix warnings in rtstartup --- library/rtstartup/rsbegin.rs | 1 + library/rtstartup/rsend.rs | 1 + 2 files changed, 2 insertions(+) diff --git a/library/rtstartup/rsbegin.rs b/library/rtstartup/rsbegin.rs index 1df0c89705380..14bce2bbeee2b 100644 --- a/library/rtstartup/rsbegin.rs +++ b/library/rtstartup/rsbegin.rs @@ -18,6 +18,7 @@ #![crate_type = "rlib"] #![no_core] #![allow(non_camel_case_types)] +#![allow(internal_features)] #[lang = "sized"] trait Sized {} diff --git a/library/rtstartup/rsend.rs b/library/rtstartup/rsend.rs index d5aca80edf9e0..714643c83866f 100644 --- a/library/rtstartup/rsend.rs +++ b/library/rtstartup/rsend.rs @@ -5,6 +5,7 @@ #![feature(auto_traits)] #![crate_type = "rlib"] #![no_core] +#![allow(internal_features)] #[lang = "sized"] trait Sized {} From 0d70e588e6624db9ce001d32e9ce1148cfde8085 Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Sat, 6 Jan 2024 01:40:56 +0300 Subject: [PATCH 23/23] library: Fix a symlink test failing on Windows --- library/std/src/fs/tests.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/library/std/src/fs/tests.rs b/library/std/src/fs/tests.rs index 12afdef2669aa..5917bf8df029e 100644 --- a/library/std/src/fs/tests.rs +++ b/library/std/src/fs/tests.rs @@ -936,8 +936,10 @@ fn read_link() { } // Check that readlink works with non-drive paths on Windows. let link = tmpdir.join("link_unc"); - check!(symlink_dir(r"\\localhost\c$\", &link)); - assert_eq!(check!(fs::read_link(&link)), Path::new(r"\\localhost\c$\")); + if got_symlink_permission(&tmpdir) { + check!(symlink_dir(r"\\localhost\c$\", &link)); + assert_eq!(check!(fs::read_link(&link)), Path::new(r"\\localhost\c$\")); + }; } let link = tmpdir.join("link"); if !got_symlink_permission(&tmpdir) {