From 0d8a0c56fee71dac218eb949817669ab8bb00c5e Mon Sep 17 00:00:00 2001 From: Konrad Borowski Date: Tue, 9 Aug 2022 07:07:29 +0000 Subject: [PATCH 01/17] Rename LinesAnyMap to LinesMap lines_any method was replaced with lines method, so it makes sense to rename this structure to match new name. Co-authored-by: Ian Jackson --- library/core/src/str/iter.rs | 4 ++-- library/core/src/str/mod.rs | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/library/core/src/str/iter.rs b/library/core/src/str/iter.rs index 24083ee6af44f..660a3b091b3d1 100644 --- a/library/core/src/str/iter.rs +++ b/library/core/src/str/iter.rs @@ -13,7 +13,7 @@ use super::from_utf8_unchecked; use super::pattern::Pattern; use super::pattern::{DoubleEndedSearcher, ReverseSearcher, Searcher}; use super::validations::{next_code_point, next_code_point_reverse}; -use super::LinesAnyMap; +use super::LinesMap; use super::{BytesIsNotEmpty, UnsafeBytesToStr}; use super::{CharEscapeDebugContinue, CharEscapeDefault, CharEscapeUnicode}; use super::{IsAsciiWhitespace, IsNotEmpty, IsWhitespace}; @@ -1091,7 +1091,7 @@ generate_pattern_iterators! { #[stable(feature = "rust1", since = "1.0.0")] #[must_use = "iterators are lazy and do nothing unless consumed"] #[derive(Clone, Debug)] -pub struct Lines<'a>(pub(super) Map, LinesAnyMap>); +pub struct Lines<'a>(pub(super) Map, LinesMap>); #[stable(feature = "rust1", since = "1.0.0")] impl<'a> Iterator for Lines<'a> { diff --git a/library/core/src/str/mod.rs b/library/core/src/str/mod.rs index fbc0fc397a5df..434c1598ee489 100644 --- a/library/core/src/str/mod.rs +++ b/library/core/src/str/mod.rs @@ -997,7 +997,7 @@ impl str { #[stable(feature = "rust1", since = "1.0.0")] #[inline] pub fn lines(&self) -> Lines<'_> { - Lines(self.split_terminator('\n').map(LinesAnyMap)) + Lines(self.split_terminator('\n').map(LinesMap)) } /// An iterator over the lines of a string. @@ -2590,7 +2590,7 @@ impl Default for &mut str { impl_fn_for_zst! { /// A nameable, cloneable fn type #[derive(Clone)] - struct LinesAnyMap impl<'a> Fn = |line: &'a str| -> &'a str { + struct LinesMap impl<'a> Fn = |line: &'a str| -> &'a str { let l = line.len(); if l > 0 && line.as_bytes()[l - 1] == b'\r' { &line[0 .. l - 1] } else { line } From cef81dcd0a9503ff8a8f915c43688a78c1c11d83 Mon Sep 17 00:00:00 2001 From: Konrad Borowski Date: Tue, 9 Aug 2022 07:12:15 +0000 Subject: [PATCH 02/17] Fix handling of trailing bare CR in str::lines Previously "bare\r" was split into ["bare"] even though the documentation said that only LF and CRLF count as newlines. This fix is a behavioural change, even though it brings the behaviour into line with the documentation, and into line with that of `std::io::BufRead::lines()`. This is an alternative to #91051, which proposes to document rather than fix the behaviour. Fixes #94435. Co-authored-by: Ian Jackson --- library/alloc/tests/str.rs | 26 +++++++++++++++++++------- library/core/src/str/iter.rs | 2 +- library/core/src/str/mod.rs | 8 ++++---- 3 files changed, 24 insertions(+), 12 deletions(-) diff --git a/library/alloc/tests/str.rs b/library/alloc/tests/str.rs index e30329aa1cb6c..aa767d5691abd 100644 --- a/library/alloc/tests/str.rs +++ b/library/alloc/tests/str.rs @@ -1499,13 +1499,25 @@ fn test_split_whitespace() { #[test] fn test_lines() { - let data = "\nMäry häd ä little lämb\n\r\nLittle lämb\n"; - let lines: Vec<&str> = data.lines().collect(); - assert_eq!(lines, ["", "Märy häd ä little lämb", "", "Little lämb"]); - - let data = "\r\nMäry häd ä little lämb\n\nLittle lämb"; // no trailing \n - let lines: Vec<&str> = data.lines().collect(); - assert_eq!(lines, ["", "Märy häd ä little lämb", "", "Little lämb"]); + fn t(data: &str, expected: &[&str]) { + let lines: Vec<&str> = data.lines().collect(); + assert_eq!(lines, expected); + } + t("", &[]); + t("\n", &[""]); + t("\n2nd", &["", "2nd"]); + t("\r\n", &[""]); + t("bare\r", &["bare\r"]); + t("bare\rcr", &["bare\rcr"]); + t("Text\n\r", &["Text", "\r"]); + t( + "\nMäry häd ä little lämb\n\r\nLittle lämb\n", + &["", "Märy häd ä little lämb", "", "Little lämb"], + ); + t( + "\r\nMäry häd ä little lämb\n\nLittle lämb", + &["", "Märy häd ä little lämb", "", "Little lämb"], + ); } #[test] diff --git a/library/core/src/str/iter.rs b/library/core/src/str/iter.rs index 660a3b091b3d1..579f5806b1cd6 100644 --- a/library/core/src/str/iter.rs +++ b/library/core/src/str/iter.rs @@ -1091,7 +1091,7 @@ generate_pattern_iterators! { #[stable(feature = "rust1", since = "1.0.0")] #[must_use = "iterators are lazy and do nothing unless consumed"] #[derive(Clone, Debug)] -pub struct Lines<'a>(pub(super) Map, LinesMap>); +pub struct Lines<'a>(pub(super) Map, LinesMap>); #[stable(feature = "rust1", since = "1.0.0")] impl<'a> Iterator for Lines<'a> { diff --git a/library/core/src/str/mod.rs b/library/core/src/str/mod.rs index 434c1598ee489..49478a72f0e4d 100644 --- a/library/core/src/str/mod.rs +++ b/library/core/src/str/mod.rs @@ -997,7 +997,7 @@ impl str { #[stable(feature = "rust1", since = "1.0.0")] #[inline] pub fn lines(&self) -> Lines<'_> { - Lines(self.split_terminator('\n').map(LinesMap)) + Lines(self.split_inclusive('\n').map(LinesMap)) } /// An iterator over the lines of a string. @@ -2591,9 +2591,9 @@ impl_fn_for_zst! { /// A nameable, cloneable fn type #[derive(Clone)] struct LinesMap impl<'a> Fn = |line: &'a str| -> &'a str { - let l = line.len(); - if l > 0 && line.as_bytes()[l - 1] == b'\r' { &line[0 .. l - 1] } - else { line } + let Some(line) = line.strip_suffix('\n') else { return line }; + let Some(line) = line.strip_suffix('\r') else { return line }; + line }; #[derive(Clone)] From 2d927cc1941cce6f320640836941ca480e958ead Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Mon, 16 Jan 2023 19:27:18 -0800 Subject: [PATCH 03/17] Explain the "no-error" io::Error case Fundamentally, querying the OS for error codes is a process that is deeply subject to the whims of chance and fortune. We can account for OS, but not for every combination of platform APIs. A compiled binary may not recognize new errors introduced years later. We should clarify a few especially odd situations, and what they mean: We can effectively promise nothing. This allows removing mention of ErrorKind::Uncategorized. That error variant is hidden quite deliberately, so we should not explicitly mention it. --- library/std/src/io/error.rs | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/library/std/src/io/error.rs b/library/std/src/io/error.rs index 3cabf24492eaf..27aa245851617 100644 --- a/library/std/src/io/error.rs +++ b/library/std/src/io/error.rs @@ -359,7 +359,7 @@ pub enum ErrorKind { // "Unusual" error kinds which do not correspond simply to (sets // of) OS error codes, should be added just above this comment. - // `Other` and `Uncategorised` should remain at the end: + // `Other` and `Uncategorized` should remain at the end: // /// A custom error that does not fall under any other I/O error kind. /// @@ -565,6 +565,12 @@ impl Error { /// other standard library functions may call platform functions that may /// (or may not) reset the error value even if they succeed. /// + /// If this is used in a case where no error has yet occurred in a program, + /// e.g. right after the beginning of `fn main`, + /// then in principle any possible Error may be returned. + /// The error code may have been set by a previous program (e.g. `execve`) + /// or the OS may have initialized it to an arbitrary, even random, value. + /// /// # Examples /// /// ``` @@ -871,6 +877,13 @@ impl Error { /// Returns the corresponding [`ErrorKind`] for this error. /// + /// In some cases, the ErrorKind variant may not make much sense, + /// either because the situation does not actually involve an error, or + /// because of a new error code the standard library has not been taught. + /// See [`last_os_error`] for more details. + /// + /// [`last_os_error`]: Error::last_os_error + /// /// # Examples /// /// ``` @@ -881,7 +894,8 @@ impl Error { /// } /// /// fn main() { - /// // Will print "Uncategorized". + /// // As no error has occurred, this may print anything! + /// // It likely prints a placeholder for unidentified (non-)errors. /// print_error(Error::last_os_error()); /// // Will print "AddrInUse". /// print_error(Error::new(ErrorKind::AddrInUse, "oh no!")); From 102c8fa29028e3fd501fe297a5d7edc86697f3d9 Mon Sep 17 00:00:00 2001 From: clubby789 Date: Sun, 12 Mar 2023 16:55:32 +0000 Subject: [PATCH 04/17] Render source page layout with Askama Co-authored-by: Michael Howell --- src/librustdoc/html/highlight.rs | 59 ++++++++--------------- src/librustdoc/html/sources.rs | 58 +++++++++++----------- src/librustdoc/html/templates/source.html | 19 ++++++++ 3 files changed, 67 insertions(+), 69 deletions(-) create mode 100644 src/librustdoc/html/templates/source.html diff --git a/src/librustdoc/html/highlight.rs b/src/librustdoc/html/highlight.rs index 2c9fc4e3ca378..c099d0e4f3f47 100644 --- a/src/librustdoc/html/highlight.rs +++ b/src/librustdoc/html/highlight.rs @@ -65,23 +65,6 @@ pub(crate) fn render_item_decl_with_highlighting(src: &str, out: &mut Buffer) { write!(out, ""); } -/// Highlights `src` as a source code page, returning the HTML output. -pub(crate) fn render_source_with_highlighting( - src: &str, - out: &mut Buffer, - line_numbers: Buffer, - href_context: HrefContext<'_, '_>, - decoration_info: DecorationInfo, - extra: Option<&str>, -) { - write_header(out, "", Some(line_numbers), Tooltip::None); - if let Some(extra) = extra { - out.push_str(extra); - } - write_code(out, src, Some(href_context), Some(decoration_info)); - write_footer(out, None); -} - fn write_header(out: &mut Buffer, class: &str, extra_content: Option, tooltip: Tooltip) { write!( out, @@ -143,8 +126,8 @@ fn can_merge(class1: Option, class2: Option, text: &str) -> bool { /// This type is used as a conveniency to prevent having to pass all its fields as arguments into /// the various functions (which became its methods). -struct TokenHandler<'a, 'tcx> { - out: &'a mut Buffer, +struct TokenHandler<'a, 'tcx, F: Write> { + out: &'a mut F, /// It contains the closing tag and the associated `Class`. closing_tags: Vec<(&'static str, Class)>, /// This is used because we don't automatically generate the closing tag on `ExitSpan` in @@ -159,7 +142,7 @@ struct TokenHandler<'a, 'tcx> { href_context: Option>, } -impl<'a, 'tcx> TokenHandler<'a, 'tcx> { +impl<'a, 'tcx, F: Write> TokenHandler<'a, 'tcx, F> { fn handle_exit_span(&mut self) { // We can't get the last `closing_tags` element using `pop()` because `closing_tags` is // being used in `write_pending_elems`. @@ -211,7 +194,7 @@ impl<'a, 'tcx> TokenHandler<'a, 'tcx> { } } -impl<'a, 'tcx> Drop for TokenHandler<'a, 'tcx> { +impl<'a, 'tcx, F: Write> Drop for TokenHandler<'a, 'tcx, F> { /// When leaving, we need to flush all pending data to not have missing content. fn drop(&mut self) { if self.pending_exit_span.is_some() { @@ -233,8 +216,8 @@ impl<'a, 'tcx> Drop for TokenHandler<'a, 'tcx> { /// item definition. /// /// More explanations about spans and how we use them here are provided in the -fn write_code( - out: &mut Buffer, +pub(super) fn write_code( + out: &mut impl Write, src: &str, href_context: Option>, decoration_info: Option, @@ -883,7 +866,7 @@ impl<'src> Classifier<'src> { /// Called when we start processing a span of text that should be highlighted. /// The `Class` argument specifies how it should be highlighted. fn enter_span( - out: &mut Buffer, + out: &mut impl Write, klass: Class, href_context: &Option>, ) -> &'static str { @@ -894,8 +877,8 @@ fn enter_span( } /// Called at the end of a span of highlighted text. -fn exit_span(out: &mut Buffer, closing_tag: &str) { - out.write_str(closing_tag); +fn exit_span(out: &mut impl Write, closing_tag: &str) { + out.write_str(closing_tag).unwrap(); } /// Called for a span of text. If the text should be highlighted differently @@ -915,7 +898,7 @@ fn exit_span(out: &mut Buffer, closing_tag: &str) { /// will then try to find this `span` in the `span_correspondance_map`. If found, it'll then /// generate a link for this element (which corresponds to where its definition is located). fn string( - out: &mut Buffer, + out: &mut impl Write, text: T, klass: Option, href_context: &Option>, @@ -923,7 +906,7 @@ fn string( ) { if let Some(closing_tag) = string_without_closing_tag(out, text, klass, href_context, open_tag) { - out.write_str(closing_tag); + out.write_str(closing_tag).unwrap(); } } @@ -937,7 +920,7 @@ fn string( /// in `span_map.rs::collect_spans_and_sources`. If it cannot retrieve the information, then it's /// the same as the second point (`klass` is `Some` but doesn't have a [`rustc_span::Span`]). fn string_without_closing_tag( - out: &mut Buffer, + out: &mut impl Write, text: T, klass: Option, href_context: &Option>, @@ -945,16 +928,16 @@ fn string_without_closing_tag( ) -> Option<&'static str> { let Some(klass) = klass else { - write!(out, "{}", text); + write!(out, "{}", text).unwrap(); return None; }; let Some(def_span) = klass.get_span() else { if !open_tag { - write!(out, "{}", text); + write!(out, "{}", text).unwrap(); return None; } - write!(out, "{}", klass.as_html(), text); + write!(out, "{}", klass.as_html(), text).unwrap(); return Some(""); }; @@ -1009,28 +992,28 @@ fn string_without_closing_tag( if !open_tag { // We're already inside an element which has the same klass, no need to give it // again. - write!(out, "{}", href, text_s); + write!(out, "{}", href, text_s).unwrap(); } else { let klass_s = klass.as_html(); if klass_s.is_empty() { - write!(out, "{}", href, text_s); + write!(out, "{}", href, text_s).unwrap(); } else { - write!(out, "{}", klass_s, href, text_s); + write!(out, "{}", klass_s, href, text_s).unwrap(); } } return Some(""); } } if !open_tag { - write!(out, "{}", text_s); + write!(out, "{}", text_s).unwrap(); return None; } let klass_s = klass.as_html(); if klass_s.is_empty() { - write!(out, "{}", text_s); + out.write_str(&text_s).unwrap(); Some("") } else { - write!(out, "{}", klass_s, text_s); + write!(out, "{}", klass_s, text_s).unwrap(); Some("") } } diff --git a/src/librustdoc/html/sources.rs b/src/librustdoc/html/sources.rs index 2c90bf4fadc01..5161e8fe74d70 100644 --- a/src/librustdoc/html/sources.rs +++ b/src/librustdoc/html/sources.rs @@ -1,12 +1,14 @@ use crate::clean; use crate::docfs::PathError; use crate::error::Error; +use crate::html::format; use crate::html::format::Buffer; use crate::html::highlight; use crate::html::layout; use crate::html::render::Context; use crate::visit::DocVisitor; +use askama::Template; use rustc_data_structures::fx::{FxHashMap, FxHashSet}; use rustc_hir::def_id::LOCAL_CRATE; use rustc_middle::ty::TyCtxt; @@ -16,6 +18,7 @@ use rustc_span::source_map::FileName; use std::cell::RefCell; use std::ffi::OsStr; use std::fs; +use std::ops::RangeInclusive; use std::path::{Component, Path, PathBuf}; use std::rc::Rc; @@ -299,39 +302,32 @@ pub(crate) fn print_src( decoration_info: highlight::DecorationInfo, source_context: SourceContext, ) { + #[derive(Template)] + #[template(path = "source.html")] + struct Source { + embedded: bool, + needs_expansion: bool, + lines: RangeInclusive, + code_html: Code, + } let lines = s.lines().count(); - let mut line_numbers = Buffer::empty_from(buf); - let extra; - line_numbers.write_str("
");
+    let (embedded, needs_expansion, lines) = match source_context {
+        SourceContext::Standalone => (false, false, 1..=lines),
+        SourceContext::Embedded { offset, needs_expansion } => {
+            (true, needs_expansion, (1 + offset)..=(lines + offset))
+        }
+    };
     let current_href = context
         .href_from_span(clean::Span::new(file_span), false)
         .expect("only local crates should have sources emitted");
-    match source_context {
-        SourceContext::Standalone => {
-            extra = None;
-            for line in 1..=lines {
-                writeln!(line_numbers, "{line}")
-            }
-        }
-        SourceContext::Embedded { offset, needs_expansion } => {
-            extra = if needs_expansion {
-                Some(r#""#)
-            } else {
-                None
-            };
-            for line_number in 1..=lines {
-                let line = line_number + offset;
-                writeln!(line_numbers, "{line}")
-            }
-        }
-    }
-    line_numbers.write_str("
"); - highlight::render_source_with_highlighting( - s, - buf, - line_numbers, - highlight::HrefContext { context, file_span, root_path, current_href }, - decoration_info, - extra, - ); + let code = format::display_fn(move |fmt| { + highlight::write_code( + fmt, + s, + Some(highlight::HrefContext { context, file_span, root_path, current_href }), + Some(decoration_info), + ); + Ok(()) + }); + Source { embedded, needs_expansion, lines, code_html: code }.render_into(buf).unwrap(); } diff --git a/src/librustdoc/html/templates/source.html b/src/librustdoc/html/templates/source.html new file mode 100644 index 0000000000000..968b55ac158dc --- /dev/null +++ b/src/librustdoc/html/templates/source.html @@ -0,0 +1,19 @@ +
{# #} + 
+        {% for line in lines.clone() %}
+            {% if embedded %}
+                {{line}}
+            {%~ else %}
+                {{line}}
+            {%~ endif %}
+        {% endfor %}
+
{# #} + 
 {# #}
+        
+            {% if needs_expansion %}
+                
+            {% endif %}
+            {{code_html|safe}}
+         {# #}
+
{# #} +
From 663953857556bfaa049a3492bb4ec531ec0127b9 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Fri, 17 Mar 2023 20:48:34 +0000 Subject: [PATCH 05/17] Remove VecMap --- compiler/rustc_borrowck/src/lib.rs | 3 +- compiler/rustc_borrowck/src/nll.rs | 6 +- .../src/region_infer/opaque_types.rs | 7 +- compiler/rustc_borrowck/src/type_check/mod.rs | 3 +- compiler/rustc_data_structures/src/lib.rs | 1 - compiler/rustc_data_structures/src/vec_map.rs | 192 ------------------ .../src/vec_map/tests.rs | 48 ----- .../rustc_hir_analysis/src/collect/type_of.rs | 2 +- .../src/infer/canonical/query_response.rs | 4 +- .../rustc_infer/src/infer/opaque_types.rs | 4 +- compiler/rustc_middle/src/mir/query.rs | 4 +- .../rustc_middle/src/ty/typeck_results.rs | 5 +- 12 files changed, 16 insertions(+), 263 deletions(-) delete mode 100644 compiler/rustc_data_structures/src/vec_map.rs delete mode 100644 compiler/rustc_data_structures/src/vec_map/tests.rs diff --git a/compiler/rustc_borrowck/src/lib.rs b/compiler/rustc_borrowck/src/lib.rs index 5e77f6b190a69..2f2f7d0aaa459 100644 --- a/compiler/rustc_borrowck/src/lib.rs +++ b/compiler/rustc_borrowck/src/lib.rs @@ -19,7 +19,6 @@ extern crate tracing; use rustc_data_structures::fx::{FxIndexMap, FxIndexSet}; use rustc_data_structures::graph::dominators::Dominators; -use rustc_data_structures::vec_map::VecMap; use rustc_errors::{Diagnostic, DiagnosticBuilder, DiagnosticMessage, SubdiagnosticMessage}; use rustc_hir as hir; use rustc_hir::def_id::LocalDefId; @@ -141,7 +140,7 @@ fn mir_borrowck(tcx: TyCtxt<'_>, def: ty::WithOptConstParam) -> &Bor debug!("Skipping borrowck because of injected body"); // Let's make up a borrowck result! Fun times! let result = BorrowCheckResult { - concrete_opaque_types: VecMap::new(), + concrete_opaque_types: FxIndexMap::default(), closure_requirements: None, used_mut_upvars: SmallVec::new(), tainted_by_errors: None, diff --git a/compiler/rustc_borrowck/src/nll.rs b/compiler/rustc_borrowck/src/nll.rs index 96228338a4c22..f0068fc9226be 100644 --- a/compiler/rustc_borrowck/src/nll.rs +++ b/compiler/rustc_borrowck/src/nll.rs @@ -2,7 +2,7 @@ #![deny(rustc::diagnostic_outside_of_impl)] //! The entry point of the NLL borrow checker. -use rustc_data_structures::vec_map::VecMap; +use rustc_data_structures::fx::FxIndexMap; use rustc_hir::def_id::LocalDefId; use rustc_index::vec::IndexVec; use rustc_middle::mir::{create_dump_file, dump_enabled, dump_mir, PassWhere}; @@ -44,7 +44,7 @@ pub type PoloniusOutput = Output; /// closure requirements to propagate, and any generated errors. pub(crate) struct NllOutput<'tcx> { pub regioncx: RegionInferenceContext<'tcx>, - pub opaque_type_values: VecMap>, + pub opaque_type_values: FxIndexMap>, pub polonius_input: Option>, pub polonius_output: Option>, pub opt_closure_req: Option>, @@ -377,7 +377,7 @@ pub(super) fn dump_annotation<'tcx>( body: &Body<'tcx>, regioncx: &RegionInferenceContext<'tcx>, closure_region_requirements: &Option>, - opaque_type_values: &VecMap>, + opaque_type_values: &FxIndexMap>, errors: &mut crate::error::BorrowckErrors<'tcx>, ) { let tcx = infcx.tcx; diff --git a/compiler/rustc_borrowck/src/region_infer/opaque_types.rs b/compiler/rustc_borrowck/src/region_infer/opaque_types.rs index 748c8b9e4420c..9deac71809c2a 100644 --- a/compiler/rustc_borrowck/src/region_infer/opaque_types.rs +++ b/compiler/rustc_borrowck/src/region_infer/opaque_types.rs @@ -1,5 +1,4 @@ use rustc_data_structures::fx::{FxIndexMap, FxIndexSet}; -use rustc_data_structures::vec_map::VecMap; use rustc_errors::ErrorGuaranteed; use rustc_hir::def_id::LocalDefId; use rustc_hir::OpaqueTyOrigin; @@ -61,9 +60,9 @@ impl<'tcx> RegionInferenceContext<'tcx> { pub(crate) fn infer_opaque_types( &self, infcx: &InferCtxt<'tcx>, - opaque_ty_decls: VecMap, (OpaqueHiddenType<'tcx>, OpaqueTyOrigin)>, - ) -> VecMap> { - let mut result: VecMap> = VecMap::new(); + opaque_ty_decls: FxIndexMap, (OpaqueHiddenType<'tcx>, OpaqueTyOrigin)>, + ) -> FxIndexMap> { + let mut result: FxIndexMap> = FxIndexMap::default(); let member_constraints: FxIndexMap<_, _> = self .member_constraints diff --git a/compiler/rustc_borrowck/src/type_check/mod.rs b/compiler/rustc_borrowck/src/type_check/mod.rs index 53fef4d75bf67..f67dae9beb925 100644 --- a/compiler/rustc_borrowck/src/type_check/mod.rs +++ b/compiler/rustc_borrowck/src/type_check/mod.rs @@ -10,7 +10,6 @@ use either::Either; use hir::OpaqueTyOrigin; use rustc_data_structures::frozen::Frozen; use rustc_data_structures::fx::{FxIndexMap, FxIndexSet}; -use rustc_data_structures::vec_map::VecMap; use rustc_hir as hir; use rustc_hir::def::DefKind; use rustc_hir::def_id::LocalDefId; @@ -894,7 +893,7 @@ pub(crate) struct MirTypeckResults<'tcx> { pub(crate) constraints: MirTypeckRegionConstraints<'tcx>, pub(crate) universal_region_relations: Frozen>, pub(crate) opaque_type_values: - VecMap, (OpaqueHiddenType<'tcx>, OpaqueTyOrigin)>, + FxIndexMap, (OpaqueHiddenType<'tcx>, OpaqueTyOrigin)>, } /// A collection of region constraints that must be satisfied for the diff --git a/compiler/rustc_data_structures/src/lib.rs b/compiler/rustc_data_structures/src/lib.rs index c595bf830a3dc..0339fb925d458 100644 --- a/compiler/rustc_data_structures/src/lib.rs +++ b/compiler/rustc_data_structures/src/lib.rs @@ -79,7 +79,6 @@ pub mod sync; pub mod tiny_list; pub mod transitive_relation; pub mod vec_linked_list; -pub mod vec_map; pub mod work_queue; pub use atomic_ref::AtomicRef; pub mod frozen; diff --git a/compiler/rustc_data_structures/src/vec_map.rs b/compiler/rustc_data_structures/src/vec_map.rs deleted file mode 100644 index d1a99bcaeb754..0000000000000 --- a/compiler/rustc_data_structures/src/vec_map.rs +++ /dev/null @@ -1,192 +0,0 @@ -use std::borrow::Borrow; -use std::fmt::Debug; -use std::slice::Iter; -use std::vec::IntoIter; - -use crate::stable_hasher::{HashStable, StableHasher}; - -/// A map type implemented as a vector of pairs `K` (key) and `V` (value). -/// It currently provides a subset of all the map operations, the rest could be added as needed. -#[derive(Clone, Encodable, Decodable, Debug)] -pub struct VecMap(Vec<(K, V)>); - -impl VecMap -where - K: Debug + PartialEq, - V: Debug, -{ - pub fn new() -> Self { - VecMap(Default::default()) - } - - /// Sets the value of the entry, and returns the entry's old value. - pub fn insert(&mut self, k: K, v: V) -> Option { - if let Some(elem) = self.0.iter_mut().find(|(key, _)| *key == k) { - Some(std::mem::replace(&mut elem.1, v)) - } else { - self.0.push((k, v)); - None - } - } - - /// Removes the entry from the map and returns the removed value - pub fn remove(&mut self, k: &K) -> Option { - self.0.iter().position(|(k2, _)| k2 == k).map(|pos| self.0.remove(pos).1) - } - - /// Gets a reference to the value in the entry. - pub fn get(&self, k: &Q) -> Option<&V> - where - K: Borrow, - Q: Eq, - { - self.0.iter().find(|(key, _)| k == key.borrow()).map(|elem| &elem.1) - } - - /// Gets a mutable reference to the value in the entry. - pub fn get_mut(&mut self, k: &Q) -> Option<&mut V> - where - K: Borrow, - Q: Eq, - { - self.0.iter_mut().find(|(key, _)| k == key.borrow()).map(|elem| &mut elem.1) - } - - /// Returns the any value corresponding to the supplied predicate filter. - /// - /// The supplied predicate will be applied to each (key, value) pair and it will return a - /// reference to the values where the predicate returns `true`. - pub fn any_value_matching(&self, mut predicate: impl FnMut(&(K, V)) -> bool) -> Option<&V> { - self.0.iter().find(|kv| predicate(kv)).map(|elem| &elem.1) - } - - /// Returns the value corresponding to the supplied predicate filter. It crashes if there's - /// more than one matching element. - /// - /// The supplied predicate will be applied to each (key, value) pair and it will return a - /// reference to the value where the predicate returns `true`. - pub fn get_value_matching(&self, mut predicate: impl FnMut(&(K, V)) -> bool) -> Option<&V> { - let mut filter = self.0.iter().filter(|kv| predicate(kv)); - let (_, value) = filter.next()?; - // This should return just one element, otherwise it's a bug - assert!( - filter.next().is_none(), - "Collection {self:#?} should have just one matching element" - ); - Some(value) - } - - /// Returns `true` if the map contains a value for the specified key. - /// - /// The key may be any borrowed form of the map's key type, - /// [`Eq`] on the borrowed form *must* match those for - /// the key type. - pub fn contains_key(&self, k: &Q) -> bool - where - K: Borrow, - Q: Eq, - { - self.get(k).is_some() - } - - /// Returns `true` if the map contains no elements. - pub fn is_empty(&self) -> bool { - self.0.is_empty() - } - - pub fn iter(&self) -> Iter<'_, (K, V)> { - self.into_iter() - } - - pub fn iter_mut(&mut self) -> impl Iterator { - self.into_iter() - } - - pub fn retain(&mut self, f: impl Fn(&(K, V)) -> bool) { - self.0.retain(f) - } -} - -impl Default for VecMap { - #[inline] - fn default() -> Self { - Self(Default::default()) - } -} - -impl From> for VecMap { - fn from(vec: Vec<(K, V)>) -> Self { - Self(vec) - } -} - -impl Into> for VecMap { - fn into(self) -> Vec<(K, V)> { - self.0 - } -} - -impl FromIterator<(K, V)> for VecMap { - fn from_iter>(iter: I) -> Self { - Self(iter.into_iter().collect()) - } -} - -impl<'a, K, V> IntoIterator for &'a VecMap { - type Item = &'a (K, V); - type IntoIter = Iter<'a, (K, V)>; - - #[inline] - fn into_iter(self) -> Self::IntoIter { - self.0.iter() - } -} - -impl<'a, K: 'a, V: 'a> IntoIterator for &'a mut VecMap { - type Item = (&'a K, &'a mut V); - type IntoIter = impl Iterator; - - #[inline] - fn into_iter(self) -> Self::IntoIter { - self.0.iter_mut().map(|(k, v)| (&*k, v)) - } -} - -impl IntoIterator for VecMap { - type Item = (K, V); - type IntoIter = IntoIter<(K, V)>; - - #[inline] - fn into_iter(self) -> Self::IntoIter { - self.0.into_iter() - } -} - -impl Extend<(K, V)> for VecMap { - fn extend>(&mut self, iter: I) { - for (k, v) in iter { - self.insert(k, v); - } - } - - fn extend_one(&mut self, (k, v): (K, V)) { - self.insert(k, v); - } - - fn extend_reserve(&mut self, additional: usize) { - self.0.extend_reserve(additional); - } -} - -impl HashStable for VecMap -where - K: HashStable + Eq, - V: HashStable, -{ - fn hash_stable(&self, hcx: &mut CTX, hasher: &mut StableHasher) { - self.0.hash_stable(hcx, hasher) - } -} - -#[cfg(test)] -mod tests; diff --git a/compiler/rustc_data_structures/src/vec_map/tests.rs b/compiler/rustc_data_structures/src/vec_map/tests.rs deleted file mode 100644 index 458b60077dc75..0000000000000 --- a/compiler/rustc_data_structures/src/vec_map/tests.rs +++ /dev/null @@ -1,48 +0,0 @@ -use super::*; - -impl VecMap { - fn into_vec(self) -> Vec<(K, V)> { - self.0.into() - } -} - -#[test] -fn test_from_iterator() { - assert_eq!( - std::iter::empty().collect::>().into_vec(), - Vec::<(i32, bool)>::new() - ); - assert_eq!(std::iter::once((42, true)).collect::>().into_vec(), vec![(42, true)]); - assert_eq!( - [(1, true), (2, false)].into_iter().collect::>().into_vec(), - vec![(1, true), (2, false)] - ); -} - -#[test] -fn test_into_iterator_owned() { - assert_eq!(VecMap::new().into_iter().collect::>(), Vec::<(i32, bool)>::new()); - assert_eq!(VecMap::from(vec![(1, true)]).into_iter().collect::>(), vec![(1, true)]); - assert_eq!( - VecMap::from(vec![(1, true), (2, false)]).into_iter().collect::>(), - vec![(1, true), (2, false)] - ); -} - -#[test] -fn test_insert() { - let mut v = VecMap::new(); - assert_eq!(v.insert(1, true), None); - assert_eq!(v.insert(2, false), None); - assert_eq!(v.clone().into_vec(), vec![(1, true), (2, false)]); - assert_eq!(v.insert(1, false), Some(true)); - assert_eq!(v.into_vec(), vec![(1, false), (2, false)]); -} - -#[test] -fn test_get() { - let v = [(1, true), (2, false)].into_iter().collect::>(); - assert_eq!(v.get(&1), Some(&true)); - assert_eq!(v.get(&2), Some(&false)); - assert_eq!(v.get(&3), None); -} diff --git a/compiler/rustc_hir_analysis/src/collect/type_of.rs b/compiler/rustc_hir_analysis/src/collect/type_of.rs index fe44fabf57df9..82689b14fc8c2 100644 --- a/compiler/rustc_hir_analysis/src/collect/type_of.rs +++ b/compiler/rustc_hir_analysis/src/collect/type_of.rs @@ -765,7 +765,7 @@ fn find_opaque_ty_constraints_for_rpit( // Use borrowck to get the type with unerased regions. let concrete_opaque_types = &self.tcx.mir_borrowck(def_id).concrete_opaque_types; debug!(?concrete_opaque_types); - for &(def_id, concrete_type) in concrete_opaque_types { + for (&def_id, &concrete_type) in concrete_opaque_types { if def_id != self.def_id { // Ignore constraints for other opaque types. continue; diff --git a/compiler/rustc_infer/src/infer/canonical/query_response.rs b/compiler/rustc_infer/src/infer/canonical/query_response.rs index 156a7e68ed1a0..268896b671adf 100644 --- a/compiler/rustc_infer/src/infer/canonical/query_response.rs +++ b/compiler/rustc_infer/src/infer/canonical/query_response.rs @@ -159,9 +159,7 @@ impl<'tcx> InferCtxt<'tcx> { .opaque_type_storage .opaque_types .iter() - .map(|&(k, ref v)| { - (self.tcx.mk_opaque(k.def_id.to_def_id(), k.substs), v.hidden_type.ty) - }) + .map(|(k, v)| (self.tcx.mk_opaque(k.def_id.to_def_id(), k.substs), v.hidden_type.ty)) .collect() } diff --git a/compiler/rustc_infer/src/infer/opaque_types.rs b/compiler/rustc_infer/src/infer/opaque_types.rs index 49f823a47b83d..3a0a0494a7ed3 100644 --- a/compiler/rustc_infer/src/infer/opaque_types.rs +++ b/compiler/rustc_infer/src/infer/opaque_types.rs @@ -5,8 +5,8 @@ use crate::infer::{DefiningAnchor, InferCtxt, InferOk}; use crate::traits; use hir::def_id::{DefId, LocalDefId}; use hir::OpaqueTyOrigin; +use rustc_data_structures::fx::FxIndexMap; use rustc_data_structures::sync::Lrc; -use rustc_data_structures::vec_map::VecMap; use rustc_hir as hir; use rustc_middle::traits::ObligationCause; use rustc_middle::ty::error::{ExpectedFound, TypeError}; @@ -21,7 +21,7 @@ use std::ops::ControlFlow; mod table; -pub type OpaqueTypeMap<'tcx> = VecMap, OpaqueTypeDecl<'tcx>>; +pub type OpaqueTypeMap<'tcx> = FxIndexMap, OpaqueTypeDecl<'tcx>>; pub use table::{OpaqueTypeStorage, OpaqueTypeTable}; /// Information about the opaque types whose values we diff --git a/compiler/rustc_middle/src/mir/query.rs b/compiler/rustc_middle/src/mir/query.rs index d85d68870d7d8..786c2e9cd943e 100644 --- a/compiler/rustc_middle/src/mir/query.rs +++ b/compiler/rustc_middle/src/mir/query.rs @@ -2,8 +2,8 @@ use crate::mir::{Body, ConstantKind, Promoted}; use crate::ty::{self, OpaqueHiddenType, Ty, TyCtxt}; +use rustc_data_structures::fx::FxIndexMap; use rustc_data_structures::unord::UnordSet; -use rustc_data_structures::vec_map::VecMap; use rustc_errors::ErrorGuaranteed; use rustc_hir as hir; use rustc_hir::def_id::{DefId, LocalDefId}; @@ -227,7 +227,7 @@ pub struct BorrowCheckResult<'tcx> { /// All the opaque types that are restricted to concrete types /// by this function. Unlike the value in `TypeckResults`, this has /// unerased regions. - pub concrete_opaque_types: VecMap>, + pub concrete_opaque_types: FxIndexMap>, pub closure_requirements: Option>, pub used_mut_upvars: SmallVec<[Field; 8]>, pub tainted_by_errors: Option, diff --git a/compiler/rustc_middle/src/ty/typeck_results.rs b/compiler/rustc_middle/src/ty/typeck_results.rs index 586958247fcdc..2b0fb4dc2b7d6 100644 --- a/compiler/rustc_middle/src/ty/typeck_results.rs +++ b/compiler/rustc_middle/src/ty/typeck_results.rs @@ -8,10 +8,9 @@ use crate::{ }, }; use rustc_data_structures::{ - fx::FxHashMap, + fx::{FxHashMap, FxIndexMap}, sync::Lrc, unord::{UnordItems, UnordSet}, - vec_map::VecMap, }; use rustc_errors::ErrorGuaranteed; use rustc_hir as hir; @@ -155,7 +154,7 @@ pub struct TypeckResults<'tcx> { /// by this function. We also store the /// type here, so that mir-borrowck can use it as a hint for figuring out hidden types, /// even if they are only set in dead code (which doesn't show up in MIR). - pub concrete_opaque_types: VecMap>, + pub concrete_opaque_types: FxIndexMap>, /// Tracks the minimum captures required for a closure; /// see `MinCaptureInformationMap` for more details. From 0f32fd8484d3282648dc18985b7d29abad2d6b70 Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Fri, 17 Mar 2023 14:24:11 -0700 Subject: [PATCH 06/17] Remove irrelevant docs on error kinds --- library/std/src/io/error.rs | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/library/std/src/io/error.rs b/library/std/src/io/error.rs index 27aa245851617..6dd7859fcb40b 100644 --- a/library/std/src/io/error.rs +++ b/library/std/src/io/error.rs @@ -565,12 +565,6 @@ impl Error { /// other standard library functions may call platform functions that may /// (or may not) reset the error value even if they succeed. /// - /// If this is used in a case where no error has yet occurred in a program, - /// e.g. right after the beginning of `fn main`, - /// then in principle any possible Error may be returned. - /// The error code may have been set by a previous program (e.g. `execve`) - /// or the OS may have initialized it to an arbitrary, even random, value. - /// /// # Examples /// /// ``` @@ -877,9 +871,9 @@ impl Error { /// Returns the corresponding [`ErrorKind`] for this error. /// - /// In some cases, the ErrorKind variant may not make much sense, - /// either because the situation does not actually involve an error, or - /// because of a new error code the standard library has not been taught. + /// This may be a value set by Rust code constructing custom `io::Error`s, + /// or if this `io::Error` was sourced from the operating system, + /// it will be a value inferred from the system's error encoding. /// See [`last_os_error`] for more details. /// /// [`last_os_error`]: Error::last_os_error @@ -894,7 +888,7 @@ impl Error { /// } /// /// fn main() { - /// // As no error has occurred, this may print anything! + /// // As no error has (visibly) occurred, this may print anything! /// // It likely prints a placeholder for unidentified (non-)errors. /// print_error(Error::last_os_error()); /// // Will print "AddrInUse". From 27e9ee9baeb8149e3f3d56572061c008109cd134 Mon Sep 17 00:00:00 2001 From: Andre Bogus Date: Wed, 15 Mar 2023 16:29:52 +0100 Subject: [PATCH 07/17] move Option::as_slice to intrinsic --- compiler/rustc_hir/src/lang_items.rs | 1 + .../rustc_hir_analysis/src/check/intrinsic.rs | 15 +++ .../src/lower_intrinsics.rs | 30 +++++ compiler/rustc_span/src/symbol.rs | 1 + library/core/src/intrinsics.rs | 6 + library/core/src/option.rs | 108 ++++++------------ ...insics.option_payload.LowerIntrinsics.diff | 54 +++++++++ tests/mir-opt/lower_intrinsics.rs | 9 ++ 8 files changed, 152 insertions(+), 72 deletions(-) create mode 100644 tests/mir-opt/lower_intrinsics.option_payload.LowerIntrinsics.diff diff --git a/compiler/rustc_hir/src/lang_items.rs b/compiler/rustc_hir/src/lang_items.rs index 72ff317d45d6e..0863d65d8f9c8 100644 --- a/compiler/rustc_hir/src/lang_items.rs +++ b/compiler/rustc_hir/src/lang_items.rs @@ -301,6 +301,7 @@ language_item_table! { Context, sym::Context, context, Target::Struct, GenericRequirement::None; FuturePoll, sym::poll, future_poll_fn, Target::Method(MethodKind::Trait { body: false }), GenericRequirement::None; + Option, sym::Option, option_type, Target::Enum, GenericRequirement::None; OptionSome, sym::Some, option_some_variant, Target::Variant, GenericRequirement::None; OptionNone, sym::None, option_none_variant, Target::Variant, GenericRequirement::None; diff --git a/compiler/rustc_hir_analysis/src/check/intrinsic.rs b/compiler/rustc_hir_analysis/src/check/intrinsic.rs index 8c364a4f3b2b8..2f9fc7adb5212 100644 --- a/compiler/rustc_hir_analysis/src/check/intrinsic.rs +++ b/compiler/rustc_hir_analysis/src/check/intrinsic.rs @@ -222,6 +222,21 @@ pub fn check_intrinsic_type(tcx: TyCtxt<'_>, it: &hir::ForeignItem<'_>) { ], tcx.mk_ptr(ty::TypeAndMut { ty: param(0), mutbl: hir::Mutability::Not }), ), + sym::option_payload_ptr => { + let option_def_id = tcx.require_lang_item(hir::LangItem::Option, None); + let p0 = param(0); + ( + 1, + vec![tcx.mk_ptr(ty::TypeAndMut { + ty: tcx.mk_adt( + tcx.adt_def(option_def_id), + tcx.mk_substs_from_iter([ty::GenericArg::from(p0)].into_iter()), + ), + mutbl: hir::Mutability::Not, + })], + tcx.mk_ptr(ty::TypeAndMut { ty: p0, mutbl: hir::Mutability::Not }), + ) + } sym::ptr_mask => ( 1, vec![ diff --git a/compiler/rustc_mir_transform/src/lower_intrinsics.rs b/compiler/rustc_mir_transform/src/lower_intrinsics.rs index 5d7382305ae14..46eab1184bdad 100644 --- a/compiler/rustc_mir_transform/src/lower_intrinsics.rs +++ b/compiler/rustc_mir_transform/src/lower_intrinsics.rs @@ -6,6 +6,7 @@ use rustc_middle::ty::subst::SubstsRef; use rustc_middle::ty::{self, Ty, TyCtxt}; use rustc_span::symbol::{sym, Symbol}; use rustc_span::Span; +use rustc_target::abi::VariantIdx; pub struct LowerIntrinsics; @@ -191,6 +192,35 @@ impl<'tcx> MirPass<'tcx> for LowerIntrinsics { terminator.kind = TerminatorKind::Goto { target }; } } + sym::option_payload_ptr => { + if let (Some(target), Some(arg)) = (*target, args[0].place()) { + let ty::RawPtr(ty::TypeAndMut { ty: dest_ty, .. }) = + destination.ty(local_decls, tcx).ty.kind() + else { bug!(); }; + + block.statements.push(Statement { + source_info: terminator.source_info, + kind: StatementKind::Assign(Box::new(( + *destination, + Rvalue::AddressOf( + Mutability::Not, + arg.project_deeper( + &[ + PlaceElem::Deref, + PlaceElem::Downcast( + Some(sym::Some), + VariantIdx::from_u32(1), + ), + PlaceElem::Field(Field::from_u32(0), *dest_ty), + ], + tcx, + ), + ), + ))), + }); + terminator.kind = TerminatorKind::Goto { target }; + } + } _ if intrinsic_name.as_str().starts_with("simd_shuffle") => { validate_simd_shuffle(tcx, args, terminator.source_info.span); } diff --git a/compiler/rustc_span/src/symbol.rs b/compiler/rustc_span/src/symbol.rs index 0154c719ef608..139fb48e90e06 100644 --- a/compiler/rustc_span/src/symbol.rs +++ b/compiler/rustc_span/src/symbol.rs @@ -1043,6 +1043,7 @@ symbols! { optin_builtin_traits, option, option_env, + option_payload_ptr, options, or, or_patterns, diff --git a/library/core/src/intrinsics.rs b/library/core/src/intrinsics.rs index ee8846675ce25..7482b8b0862e2 100644 --- a/library/core/src/intrinsics.rs +++ b/library/core/src/intrinsics.rs @@ -2214,6 +2214,12 @@ extern "rust-intrinsic" { where G: FnOnce, F: FnOnce; + + #[cfg(not(bootstrap))] + /// This method creates a pointer to any `Some` value. If the argument is + /// `None`, an invalid within-bounds pointer (that is still acceptable for + /// constructing an empty slice) is returned. + pub fn option_payload_ptr(arg: *const Option) -> *const T; } // Some functions are defined here because they accidentally got made diff --git a/library/core/src/option.rs b/library/core/src/option.rs index 0f2475a8bdea6..cba597e66aa13 100644 --- a/library/core/src/option.rs +++ b/library/core/src/option.rs @@ -559,6 +559,7 @@ use crate::{ /// The `Option` type. See [the module level documentation](self) for more. #[derive(Copy, PartialOrd, Eq, Ord, Debug, Hash)] #[rustc_diagnostic_item = "Option"] +#[cfg_attr(not(bootstrap), lang = "Option")] #[stable(feature = "rust1", since = "1.0.0")] pub enum Option { /// No value. @@ -735,48 +736,6 @@ impl Option { } } - /// This is a guess at how many bytes into the option the payload can be found. - /// - /// For niche-optimized types it's correct because it's pigeon-holed to only - /// one possible place. For other types, it's usually correct today, but - /// tweaks to the layout algorithm (particularly expansions of - /// `-Z randomize-layout`) might make it incorrect at any point. - /// - /// It's guaranteed to be a multiple of alignment (so will always give a - /// correctly-aligned location) and to be within the allocated object, so - /// is valid to use with `offset` and to use for a zero-sized read. - /// - /// FIXME: This is a horrible hack, but allows a nice optimization. It should - /// be replaced with `offset_of!` once that works on enum variants. - const SOME_BYTE_OFFSET_GUESS: isize = { - let some_uninit = Some(mem::MaybeUninit::::uninit()); - let payload_ref = some_uninit.as_ref().unwrap(); - // SAFETY: `as_ref` gives an address inside the existing `Option`, - // so both pointers are derived from the same thing and the result - // cannot overflow an `isize`. - let offset = unsafe { <*const _>::byte_offset_from(payload_ref, &some_uninit) }; - - // The offset is into the object, so it's guaranteed to be non-negative. - assert!(offset >= 0); - - // The payload and the overall option are aligned, - // so the offset will be a multiple of the alignment too. - assert!((offset as usize) % mem::align_of::() == 0); - - let max_offset = mem::size_of::() - mem::size_of::(); - if offset as usize <= max_offset { - // There's enough space after this offset for a `T` to exist without - // overflowing the bounds of the object, so let's try it. - offset - } else { - // The offset guess is definitely wrong, so use the address - // of the original option since we have it already. - // This also correctly handles the case of layout-optimized enums - // where `max_offset == 0` and thus this is the only possibility. - 0 - } - }; - /// Returns a slice of the contained value, if any. If this is `None`, an /// empty slice is returned. This can be useful to have a single type of /// iterator over an `Option` or slice. @@ -809,28 +768,29 @@ impl Option { #[must_use] #[unstable(feature = "option_as_slice", issue = "108545")] pub fn as_slice(&self) -> &[T] { - let payload_ptr: *const T = - // The goal here is that both arms here are calculating exactly - // the same pointer, and thus it'll be folded away when the guessed - // offset is correct, but if the guess is wrong for some reason - // it'll at least still be sound, just no longer optimal. - if let Some(payload) = self { - payload - } else { - let self_ptr: *const Self = self; - // SAFETY: `SOME_BYTE_OFFSET_GUESS` guarantees that its value is - // such that this will be in-bounds of the object. - unsafe { self_ptr.byte_offset(Self::SOME_BYTE_OFFSET_GUESS).cast() } - }; - let len = usize::from(self.is_some()); + #[cfg(bootstrap)] + match self { + Some(value) => slice::from_ref(value), + None => &[], + } + #[cfg(not(bootstrap))] // SAFETY: When the `Option` is `Some`, we're using the actual pointer // to the payload, with a length of 1, so this is equivalent to // `slice::from_ref`, and thus is safe. // When the `Option` is `None`, the length used is 0, so to be safe it // just needs to be aligned, which it is because `&self` is aligned and // the offset used is a multiple of alignment. - unsafe { slice::from_raw_parts(payload_ptr, len) } + // + // In the new version, the intrinsic always returns a pointer to an + // in-bounds and correctly aligned position for a `T` (even if in the + // `None` case it's just padding). + unsafe { + slice::from_raw_parts( + crate::intrinsics::option_payload_ptr(crate::ptr::from_ref(self)), + usize::from(self.is_some()), + ) + } } /// Returns a mutable slice of the contained value, if any. If this is @@ -875,28 +835,32 @@ impl Option { #[must_use] #[unstable(feature = "option_as_slice", issue = "108545")] pub fn as_mut_slice(&mut self) -> &mut [T] { - let payload_ptr: *mut T = - // The goal here is that both arms here are calculating exactly - // the same pointer, and thus it'll be folded away when the guessed - // offset is correct, but if the guess is wrong for some reason - // it'll at least still be sound, just no longer optimal. - if let Some(payload) = self { - payload - } else { - let self_ptr: *mut Self = self; - // SAFETY: `SOME_BYTE_OFFSET_GUESS` guarantees that its value is - // such that this will be in-bounds of the object. - unsafe { self_ptr.byte_offset(Self::SOME_BYTE_OFFSET_GUESS).cast() } - }; - let len = usize::from(self.is_some()); + #[cfg(bootstrap)] + match self { + Some(value) => slice::from_mut(value), + None => &mut [], + } + #[cfg(not(bootstrap))] // SAFETY: When the `Option` is `Some`, we're using the actual pointer // to the payload, with a length of 1, so this is equivalent to // `slice::from_mut`, and thus is safe. // When the `Option` is `None`, the length used is 0, so to be safe it // just needs to be aligned, which it is because `&self` is aligned and // the offset used is a multiple of alignment. - unsafe { slice::from_raw_parts_mut(payload_ptr, len) } + // + // In the new version, the intrinsic creates a `*const T` from a + // mutable reference so it is safe to cast back to a mutable pointer + // here. As with `as_slice`, the intrinsic always returns a pointer to + // an in-bounds and correctly aligned position for a `T` (even if in + // the `None` case it's just padding). + unsafe { + slice::from_raw_parts_mut( + crate::intrinsics::option_payload_ptr(crate::ptr::from_mut(self).cast_const()) + .cast_mut(), + usize::from(self.is_some()), + ) + } } ///////////////////////////////////////////////////////////////////////// diff --git a/tests/mir-opt/lower_intrinsics.option_payload.LowerIntrinsics.diff b/tests/mir-opt/lower_intrinsics.option_payload.LowerIntrinsics.diff new file mode 100644 index 0000000000000..e535141e772f8 --- /dev/null +++ b/tests/mir-opt/lower_intrinsics.option_payload.LowerIntrinsics.diff @@ -0,0 +1,54 @@ +- // MIR for `option_payload` before LowerIntrinsics ++ // MIR for `option_payload` after LowerIntrinsics + + fn option_payload(_1: &Option, _2: &Option) -> () { + debug o => _1; // in scope 0 at $DIR/lower_intrinsics.rs:+0:23: +0:24 + debug p => _2; // in scope 0 at $DIR/lower_intrinsics.rs:+0:42: +0:43 + let mut _0: (); // return place in scope 0 at $DIR/lower_intrinsics.rs:+0:62: +0:62 + let mut _4: *const std::option::Option; // in scope 0 at $DIR/lower_intrinsics.rs:+2:55: +2:56 + let mut _6: *const std::option::Option; // in scope 0 at $DIR/lower_intrinsics.rs:+3:55: +3:56 + scope 1 { + let _3: *const usize; // in scope 1 at $DIR/lower_intrinsics.rs:+2:13: +2:15 + scope 2 { + debug _x => _3; // in scope 2 at $DIR/lower_intrinsics.rs:+2:13: +2:15 + let _5: *const std::string::String; // in scope 2 at $DIR/lower_intrinsics.rs:+3:13: +3:15 + scope 3 { + debug _y => _5; // in scope 3 at $DIR/lower_intrinsics.rs:+3:13: +3:15 + } + } + } + + bb0: { + StorageLive(_3); // scope 1 at $DIR/lower_intrinsics.rs:+2:13: +2:15 + StorageLive(_4); // scope 1 at $DIR/lower_intrinsics.rs:+2:55: +2:56 + _4 = &raw const (*_1); // scope 1 at $DIR/lower_intrinsics.rs:+2:55: +2:56 +- _3 = option_payload_ptr::(move _4) -> bb1; // scope 1 at $DIR/lower_intrinsics.rs:+2:18: +2:57 +- // mir::Constant +- // + span: $DIR/lower_intrinsics.rs:99:18: 99:54 +- // + literal: Const { ty: unsafe extern "rust-intrinsic" fn(*const Option) -> *const usize {option_payload_ptr::}, val: Value() } ++ _3 = &raw const (((*_4) as Some).0: usize); // scope 1 at $DIR/lower_intrinsics.rs:+2:18: +2:57 ++ goto -> bb1; // scope 1 at $DIR/lower_intrinsics.rs:+2:18: +2:57 + } + + bb1: { + StorageDead(_4); // scope 1 at $DIR/lower_intrinsics.rs:+2:56: +2:57 + StorageLive(_5); // scope 2 at $DIR/lower_intrinsics.rs:+3:13: +3:15 + StorageLive(_6); // scope 2 at $DIR/lower_intrinsics.rs:+3:55: +3:56 + _6 = &raw const (*_2); // scope 2 at $DIR/lower_intrinsics.rs:+3:55: +3:56 +- _5 = option_payload_ptr::(move _6) -> bb2; // scope 2 at $DIR/lower_intrinsics.rs:+3:18: +3:57 +- // mir::Constant +- // + span: $DIR/lower_intrinsics.rs:100:18: 100:54 +- // + literal: Const { ty: unsafe extern "rust-intrinsic" fn(*const Option) -> *const String {option_payload_ptr::}, val: Value() } ++ _5 = &raw const (((*_6) as Some).0: std::string::String); // scope 2 at $DIR/lower_intrinsics.rs:+3:18: +3:57 ++ goto -> bb2; // scope 2 at $DIR/lower_intrinsics.rs:+3:18: +3:57 + } + + bb2: { + StorageDead(_6); // scope 2 at $DIR/lower_intrinsics.rs:+3:56: +3:57 + _0 = const (); // scope 1 at $DIR/lower_intrinsics.rs:+1:5: +4:6 + StorageDead(_5); // scope 2 at $DIR/lower_intrinsics.rs:+4:5: +4:6 + StorageDead(_3); // scope 1 at $DIR/lower_intrinsics.rs:+4:5: +4:6 + return; // scope 0 at $DIR/lower_intrinsics.rs:+5:2: +5:2 + } + } + diff --git a/tests/mir-opt/lower_intrinsics.rs b/tests/mir-opt/lower_intrinsics.rs index a0a1df4e5ca86..f07e2816f4f41 100644 --- a/tests/mir-opt/lower_intrinsics.rs +++ b/tests/mir-opt/lower_intrinsics.rs @@ -91,3 +91,12 @@ pub fn read_via_copy_uninhabited(r: &Never) -> Never { } pub enum Never {} + +// EMIT_MIR lower_intrinsics.option_payload.LowerIntrinsics.diff +#[cfg(not(bootstrap))] +pub fn option_payload(o: &Option, p: &Option) { + unsafe { + let _x = core::intrinsics::option_payload_ptr(o); + let _y = core::intrinsics::option_payload_ptr(p); + } +} From 2ec7f6c8d54e821303ba5a3da31858dbe009e283 Mon Sep 17 00:00:00 2001 From: ozkanonur Date: Sat, 18 Mar 2023 10:23:30 +0300 Subject: [PATCH 08/17] refactor `fn bootstrap::builder::Builder::compiler_for` - check compiler stage before forcing for stage2. - check if download_rustc is not set before forcing for stage1. Signed-off-by: ozkanonur --- src/bootstrap/builder.rs | 4 ++-- src/bootstrap/lib.rs | 13 +++++++------ 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/src/bootstrap/builder.rs b/src/bootstrap/builder.rs index bb07ca1908e1c..c68452db0aff6 100644 --- a/src/bootstrap/builder.rs +++ b/src/bootstrap/builder.rs @@ -919,9 +919,9 @@ impl<'a> Builder<'a> { host: TargetSelection, target: TargetSelection, ) -> Compiler { - if self.build.force_use_stage2() { + if self.build.force_use_stage2(stage) { self.compiler(2, self.config.build) - } else if self.build.force_use_stage1(Compiler { stage, host }, target) { + } else if self.build.force_use_stage1(stage, target) { self.compiler(1, self.config.build) } else { self.compiler(stage, host) diff --git a/src/bootstrap/lib.rs b/src/bootstrap/lib.rs index 22ddf87221595..e4b4192cf0882 100644 --- a/src/bootstrap/lib.rs +++ b/src/bootstrap/lib.rs @@ -1198,19 +1198,20 @@ impl Build { /// /// When all of these conditions are met the build will lift artifacts from /// the previous stage forward. - fn force_use_stage1(&self, compiler: Compiler, target: TargetSelection) -> bool { + fn force_use_stage1(&self, stage: u32, target: TargetSelection) -> bool { !self.config.full_bootstrap - && compiler.stage >= 2 + && !self.config.download_rustc() + && stage >= 2 && (self.hosts.iter().any(|h| *h == target) || target == self.build) } /// Checks whether the `compiler` compiling for `target` should be forced to /// use a stage2 compiler instead. /// - /// When we download the pre-compiled version of rustc it should be forced to - /// use a stage2 compiler. - fn force_use_stage2(&self) -> bool { - self.config.download_rustc() + /// When we download the pre-compiled version of rustc and compiler stage is >= 2, + /// it should be forced to use a stage2 compiler. + fn force_use_stage2(&self, stage: u32) -> bool { + self.config.download_rustc() && stage >= 2 } /// Given `num` in the form "a.b.c" return a "release string" which From 572c56cb7e4c31e15d36fabe4e15cff6aa8ce405 Mon Sep 17 00:00:00 2001 From: Eric Huss Date: Thu, 19 Jan 2023 08:57:26 -0800 Subject: [PATCH 09/17] Update links for custom discriminants. --- compiler/rustc_error_codes/src/error_codes/E0080.md | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/compiler/rustc_error_codes/src/error_codes/E0080.md b/compiler/rustc_error_codes/src/error_codes/E0080.md index 7b1bbde614094..71d6c6fe2ef2c 100644 --- a/compiler/rustc_error_codes/src/error_codes/E0080.md +++ b/compiler/rustc_error_codes/src/error_codes/E0080.md @@ -15,9 +15,8 @@ or causing an integer overflow are two ways to induce this error. Ensure that the expressions given can be evaluated as the desired integer type. -See the [Custom Discriminants][custom-discriminants] section of the Reference -for more information about setting custom integer types on fieldless enums -using the [`repr` attribute][repr-attribute]. +See the [Discriminants] section of the Reference for more information about +setting custom integer types on enums using the [`repr` attribute][repr-attribute]. -[custom-discriminants]: https://doc.rust-lang.org/reference/items/enumerations.html#custom-discriminant-values-for-field-less-enumerations -[repr-attribute]: https://doc.rust-lang.org/reference/type-layout.html#reprc-enums +[discriminants]: https://doc.rust-lang.org/reference/items/enumerations.html#discriminants +[repr-attribute]: https://doc.rust-lang.org/reference/type-layout.html#representations From 20dc53208557db5672a1bcf97b3e582a95913f2c Mon Sep 17 00:00:00 2001 From: Mu42 Date: Mon, 20 Mar 2023 15:32:21 +0800 Subject: [PATCH 10/17] Remove Ty::is_region_ptr --- .../src/diagnostics/mutability_errors.rs | 13 ++++--------- compiler/rustc_codegen_cranelift/src/vtable.rs | 4 +--- compiler/rustc_codegen_ssa/src/mir/block.rs | 4 ++-- compiler/rustc_hir_typeck/src/demand.rs | 2 +- compiler/rustc_hir_typeck/src/method/suggest.rs | 2 +- compiler/rustc_middle/src/ty/sty.rs | 7 +------ compiler/rustc_mir_build/src/thir/cx/expr.rs | 2 +- compiler/rustc_ty_utils/src/abi.rs | 2 +- 8 files changed, 12 insertions(+), 24 deletions(-) diff --git a/compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs b/compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs index bad08451adf08..a8c216407f931 100644 --- a/compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs +++ b/compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs @@ -120,9 +120,7 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> { && !self.upvars.is_empty() { item_msg = access_place_desc; - debug_assert!( - self.body.local_decls[ty::CAPTURE_STRUCT_LOCAL].ty.is_region_ptr() - ); + debug_assert!(self.body.local_decls[ty::CAPTURE_STRUCT_LOCAL].ty.is_ref()); debug_assert!(is_closure_or_generator( Place::ty_from( the_place_err.local, @@ -470,11 +468,8 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> { { let local_decl = &self.body.local_decls[local]; - let (pointer_sigil, pointer_desc) = if local_decl.ty.is_region_ptr() { - ("&", "reference") - } else { - ("*const", "pointer") - }; + let (pointer_sigil, pointer_desc) = + if local_decl.ty.is_ref() { ("&", "reference") } else { ("*const", "pointer") }; match self.local_names[local] { Some(name) if !local_decl.from_compiler_desugaring() => { @@ -1258,7 +1253,7 @@ fn suggest_ampmut<'tcx>( ( suggestability, highlight_span, - if local_decl.ty.is_region_ptr() { + if local_decl.ty.is_ref() { format!("&mut {}", ty_mut.ty) } else { format!("*mut {}", ty_mut.ty) diff --git a/compiler/rustc_codegen_cranelift/src/vtable.rs b/compiler/rustc_codegen_cranelift/src/vtable.rs index b7bfd8fd39526..94806e0d798ed 100644 --- a/compiler/rustc_codegen_cranelift/src/vtable.rs +++ b/compiler/rustc_codegen_cranelift/src/vtable.rs @@ -48,9 +48,7 @@ pub(crate) fn get_ptr_and_method_ref<'tcx>( ) -> (Pointer, Value) { let (ptr, vtable) = 'block: { if let Abi::Scalar(_) = arg.layout().abi { - 'descend_newtypes: while !arg.layout().ty.is_unsafe_ptr() - && !arg.layout().ty.is_region_ptr() - { + 'descend_newtypes: while !arg.layout().ty.is_unsafe_ptr() && !arg.layout().ty.is_ref() { for i in 0..arg.layout().fields.count() { let field = arg.value_field(fx, mir::Field::new(i)); if !field.layout().is_zst() { diff --git a/compiler/rustc_codegen_ssa/src/mir/block.rs b/compiler/rustc_codegen_ssa/src/mir/block.rs index f9aa2aecf65bc..6bad5c3eb2822 100644 --- a/compiler/rustc_codegen_ssa/src/mir/block.rs +++ b/compiler/rustc_codegen_ssa/src/mir/block.rs @@ -917,7 +917,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { // // This is also relevant for `Pin<&mut Self>`, where we need to peel the `Pin`. 'descend_newtypes: while !op.layout.ty.is_unsafe_ptr() - && !op.layout.ty.is_region_ptr() + && !op.layout.ty.is_ref() { for i in 0..op.layout.fields.count() { let field = op.extract_field(bx, i); @@ -959,7 +959,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { Immediate(_) => { // See comment above explaining why we peel these newtypes 'descend_newtypes: while !op.layout.ty.is_unsafe_ptr() - && !op.layout.ty.is_region_ptr() + && !op.layout.ty.is_ref() { for i in 0..op.layout.fields.count() { let field = op.extract_field(bx, i); diff --git a/compiler/rustc_hir_typeck/src/demand.rs b/compiler/rustc_hir_typeck/src/demand.rs index f65f16e317d49..23845d39a981f 100644 --- a/compiler/rustc_hir_typeck/src/demand.rs +++ b/compiler/rustc_hir_typeck/src/demand.rs @@ -1487,7 +1487,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { { let deref_kind = if checked_ty.is_box() { "unboxing the value" - } else if checked_ty.is_region_ptr() { + } else if checked_ty.is_ref() { "dereferencing the borrow" } else { "dereferencing the type" diff --git a/compiler/rustc_hir_typeck/src/method/suggest.rs b/compiler/rustc_hir_typeck/src/method/suggest.rs index 50f2b71250c01..b219be4ae1992 100644 --- a/compiler/rustc_hir_typeck/src/method/suggest.rs +++ b/compiler/rustc_hir_typeck/src/method/suggest.rs @@ -1182,7 +1182,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { .inputs() .skip_binder() .get(0) - .filter(|ty| ty.is_region_ptr() && !rcvr_ty.is_region_ptr()) + .filter(|ty| ty.is_ref() && !rcvr_ty.is_ref()) .copied() .unwrap_or(rcvr_ty), }; diff --git a/compiler/rustc_middle/src/ty/sty.rs b/compiler/rustc_middle/src/ty/sty.rs index 4c606b939b25e..2e691ab250d83 100644 --- a/compiler/rustc_middle/src/ty/sty.rs +++ b/compiler/rustc_middle/src/ty/sty.rs @@ -1913,11 +1913,6 @@ impl<'tcx> Ty<'tcx> { } } - #[inline] - pub fn is_region_ptr(self) -> bool { - matches!(self.kind(), Ref(..)) - } - #[inline] pub fn is_mutable_ptr(self) -> bool { matches!( @@ -1944,7 +1939,7 @@ impl<'tcx> Ty<'tcx> { /// Tests if this is any kind of primitive pointer type (reference, raw pointer, fn pointer). #[inline] pub fn is_any_ptr(self) -> bool { - self.is_region_ptr() || self.is_unsafe_ptr() || self.is_fn_ptr() + self.is_ref() || self.is_unsafe_ptr() || self.is_fn_ptr() } #[inline] diff --git a/compiler/rustc_mir_build/src/thir/cx/expr.rs b/compiler/rustc_mir_build/src/thir/cx/expr.rs index cecb8a61aa2f4..6fd9b9dbb5755 100644 --- a/compiler/rustc_mir_build/src/thir/cx/expr.rs +++ b/compiler/rustc_mir_build/src/thir/cx/expr.rs @@ -185,7 +185,7 @@ impl<'tcx> Cx<'tcx> { if self.typeck_results().is_coercion_cast(source.hir_id) { // Convert the lexpr to a vexpr. ExprKind::Use { source: self.mirror_expr(source) } - } else if self.typeck_results().expr_ty(source).is_region_ptr() { + } else if self.typeck_results().expr_ty(source).is_ref() { // Special cased so that we can type check that the element // type of the source matches the pointed to type of the // destination. diff --git a/compiler/rustc_ty_utils/src/abi.rs b/compiler/rustc_ty_utils/src/abi.rs index 35c9f95eb03b1..ee5a7909ba3dc 100644 --- a/compiler/rustc_ty_utils/src/abi.rs +++ b/compiler/rustc_ty_utils/src/abi.rs @@ -539,7 +539,7 @@ fn make_thin_self_ptr<'tcx>( // get a built-in pointer type let mut fat_pointer_layout = layout; 'descend_newtypes: while !fat_pointer_layout.ty.is_unsafe_ptr() - && !fat_pointer_layout.ty.is_region_ptr() + && !fat_pointer_layout.ty.is_ref() { for i in 0..fat_pointer_layout.fields.count() { let field_layout = fat_pointer_layout.field(cx, i); From 9da1da94efd45cf28e9e72c81cacbdbcb1aeabd1 Mon Sep 17 00:00:00 2001 From: Andy Wang Date: Mon, 20 Mar 2023 12:21:19 +0100 Subject: [PATCH 11/17] Allow optional RET type annotation --- library/core/src/intrinsics/mir.rs | 3 ++- .../building/custom/composite_return.rs | 21 +++++++++++++++++++ .../composite_return.tuple.built.after.mir | 11 ++++++++++ 3 files changed, 34 insertions(+), 1 deletion(-) create mode 100644 tests/mir-opt/building/custom/composite_return.rs create mode 100644 tests/mir-opt/building/custom/composite_return.tuple.built.after.mir diff --git a/library/core/src/intrinsics/mir.rs b/library/core/src/intrinsics/mir.rs index d2d9771bdce2e..f391474b8436b 100644 --- a/library/core/src/intrinsics/mir.rs +++ b/library/core/src/intrinsics/mir.rs @@ -342,6 +342,7 @@ define!( #[rustc_macro_transparency = "transparent"] pub macro mir { ( + $(type RET = $ret_ty:ty ;)? $(let $local_decl:ident $(: $local_decl_ty:ty)? ;)* { @@ -362,7 +363,7 @@ pub macro mir { { // Now all locals #[allow(non_snake_case)] - let RET; + let RET $(: $ret_ty)?; $( let $local_decl $(: $local_decl_ty)? ; )* diff --git a/tests/mir-opt/building/custom/composite_return.rs b/tests/mir-opt/building/custom/composite_return.rs new file mode 100644 index 0000000000000..701d6b1ab7131 --- /dev/null +++ b/tests/mir-opt/building/custom/composite_return.rs @@ -0,0 +1,21 @@ +#![feature(custom_mir, core_intrinsics)] + +extern crate core; +use core::intrinsics::mir::*; + +// EMIT_MIR composite_return.tuple.built.after.mir +#[custom_mir(dialect = "runtime", phase = "optimized")] +fn tuple() -> (i32, bool) { + mir!( + type RET = (i32, bool); + { + RET.0 = 1; + RET.1 = true; + Return() + } + ) +} + +fn main() { + assert_eq!(tuple(), (1, true)); +} diff --git a/tests/mir-opt/building/custom/composite_return.tuple.built.after.mir b/tests/mir-opt/building/custom/composite_return.tuple.built.after.mir new file mode 100644 index 0000000000000..d159c1a655eb5 --- /dev/null +++ b/tests/mir-opt/building/custom/composite_return.tuple.built.after.mir @@ -0,0 +1,11 @@ +// MIR for `tuple` after built + +fn tuple() -> (i32, bool) { + let mut _0: (i32, bool); // return place in scope 0 at $DIR/composite_return.rs:+0:15: +0:26 + + bb0: { + (_0.0: i32) = const 1_i32; // scope 0 at $DIR/composite_return.rs:+4:13: +4:22 + (_0.1: bool) = const true; // scope 0 at $DIR/composite_return.rs:+5:13: +5:25 + return; // scope 0 at $DIR/composite_return.rs:+6:13: +6:21 + } +} From 9dc275bb54ab088ac85a08dc807984afd57a78c7 Mon Sep 17 00:00:00 2001 From: Andy Wang Date: Mon, 20 Mar 2023 15:23:27 +0100 Subject: [PATCH 12/17] Add documentation for `type RET = ...` --- library/core/src/intrinsics/mir.rs | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/library/core/src/intrinsics/mir.rs b/library/core/src/intrinsics/mir.rs index f391474b8436b..77e207ecc6c7f 100644 --- a/library/core/src/intrinsics/mir.rs +++ b/library/core/src/intrinsics/mir.rs @@ -49,6 +49,8 @@ //! //! The input to the [`mir!`] macro is: //! +//! - An optional return type annotation in the form of `type RET = ...;`. This may be required +//! if the compiler cannot infer the type of RET. //! - A possibly empty list of local declarations. Locals can also be declared inline on //! assignments via `let`. Type inference generally works. Shadowing does not. //! - A list of basic blocks. The first of these is the start block and is where execution begins. @@ -124,6 +126,18 @@ //! } //! ) //! } +//! +//! #[custom_mir(dialect = "runtime", phase = "optimized")] +//! fn annotated_return_type() -> (i32, bool) { +//! mir!( +//! type RET = (i32, bool); +//! { +//! RET.0 = 1; +//! RET.1 = true; +//! Return() +//! } +//! ) +//! } //! ``` //! //! We can also set off compilation failures that happen in sufficiently late stages of the From d3a55419391ab2d5b75b8c8107b37459ca9b2b83 Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Sat, 18 Mar 2023 19:18:51 +0400 Subject: [PATCH 13/17] rustdoc: Cleanup parent module tracking for doc links Keep ids of the documented items themselves, not their parent modules. Parent modules can be retreived from those ids when necessary. --- compiler/rustc_resolve/src/rustdoc.rs | 18 +-- src/librustdoc/clean/inline.rs | 75 ++++------- src/librustdoc/clean/mod.rs | 26 ++-- src/librustdoc/clean/types/tests.rs | 2 +- src/librustdoc/clean/utils.rs | 4 +- .../passes/collect_intra_doc_links.rs | 126 ++++++------------ src/librustdoc/passes/collect_trait_impls.rs | 6 +- src/librustdoc/passes/propagate_doc_cfg.rs | 3 +- .../intra-doc/auxiliary/inner-crate-doc.rs | 1 + .../intra-doc/import-inline-merge-module.rs | 10 ++ 10 files changed, 103 insertions(+), 168 deletions(-) create mode 100644 tests/rustdoc-ui/intra-doc/auxiliary/inner-crate-doc.rs create mode 100644 tests/rustdoc-ui/intra-doc/import-inline-merge-module.rs diff --git a/compiler/rustc_resolve/src/rustdoc.rs b/compiler/rustc_resolve/src/rustdoc.rs index b8853c1744c92..0e40f794f1860 100644 --- a/compiler/rustc_resolve/src/rustdoc.rs +++ b/compiler/rustc_resolve/src/rustdoc.rs @@ -26,11 +26,13 @@ pub enum DocFragmentKind { #[derive(Clone, PartialEq, Eq, Debug)] pub struct DocFragment { pub span: Span, - /// The module this doc-comment came from. - /// - /// This allows distinguishing between the original documentation and a pub re-export. - /// If it is `None`, the item was not re-exported. - pub parent_module: Option, + /// The item this doc-comment came from. + /// Used to determine the scope in which doc links in this fragment are resolved. + /// Typically filled for reexport docs when they are merged into the docs of the + /// original reexported item. + /// If the id is not filled, which happens for the original reexported item, then + /// it has to be taken from somewhere else during doc link resolution. + pub item_id: Option, pub doc: Symbol, pub kind: DocFragmentKind, pub indent: usize, @@ -186,7 +188,7 @@ pub fn attrs_to_doc_fragments<'a>( ) -> (Vec, ast::AttrVec) { let mut doc_fragments = Vec::new(); let mut other_attrs = ast::AttrVec::new(); - for (attr, parent_module) in attrs { + for (attr, item_id) in attrs { if let Some((doc_str, comment_kind)) = attr.doc_str_and_comment_kind() { let doc = beautify_doc_string(doc_str, comment_kind); let kind = if attr.is_doc_comment() { @@ -194,7 +196,7 @@ pub fn attrs_to_doc_fragments<'a>( } else { DocFragmentKind::RawDoc }; - let fragment = DocFragment { span: attr.span, doc, kind, parent_module, indent: 0 }; + let fragment = DocFragment { span: attr.span, doc, kind, item_id, indent: 0 }; doc_fragments.push(fragment); } else if !doc_only { other_attrs.push(attr.clone()); @@ -216,7 +218,7 @@ pub fn prepare_to_doc_link_resolution( ) -> FxHashMap, String> { let mut res = FxHashMap::default(); for fragment in doc_fragments { - let out_str = res.entry(fragment.parent_module).or_default(); + let out_str = res.entry(fragment.item_id).or_default(); add_doc_fragment(out_str, fragment); } res diff --git a/src/librustdoc/clean/inline.rs b/src/librustdoc/clean/inline.rs index 148243683cbbf..768f8bb7bc899 100644 --- a/src/librustdoc/clean/inline.rs +++ b/src/librustdoc/clean/inline.rs @@ -36,15 +36,11 @@ use crate::formats::item_type::ItemType; /// /// The returned value is `None` if the definition could not be inlined, /// and `Some` of a vector of items if it was successfully expanded. -/// -/// `parent_module` refers to the parent of the *re-export*, not the original item. pub(crate) fn try_inline( cx: &mut DocContext<'_>, - parent_module: DefId, - import_def_id: Option, res: Res, name: Symbol, - attrs: Option<&[ast::Attribute]>, + attrs: Option<(&[ast::Attribute], Option)>, visited: &mut DefIdSet, ) -> Option> { let did = res.opt_def_id()?; @@ -55,38 +51,17 @@ pub(crate) fn try_inline( debug!("attrs={:?}", attrs); - let attrs_without_docs = attrs.map(|attrs| { - attrs.into_iter().filter(|a| a.doc_str().is_none()).cloned().collect::>() + let attrs_without_docs = attrs.map(|(attrs, def_id)| { + (attrs.into_iter().filter(|a| a.doc_str().is_none()).cloned().collect::>(), def_id) }); - // We need this ugly code because: - // - // ``` - // attrs_without_docs.map(|a| a.as_slice()) - // ``` - // - // will fail because it returns a temporary slice and: - // - // ``` - // attrs_without_docs.map(|s| { - // vec = s.as_slice(); - // vec - // }) - // ``` - // - // will fail because we're moving an uninitialized variable into a closure. - let vec; - let attrs_without_docs = match attrs_without_docs { - Some(s) => { - vec = s; - Some(vec.as_slice()) - } - None => None, - }; + let attrs_without_docs = + attrs_without_docs.as_ref().map(|(attrs, def_id)| (&attrs[..], *def_id)); + let import_def_id = attrs.and_then(|(_, def_id)| def_id); let kind = match res { Res::Def(DefKind::Trait, did) => { record_extern_fqn(cx, did, ItemType::Trait); - build_impls(cx, Some(parent_module), did, attrs_without_docs, &mut ret); + build_impls(cx, did, attrs_without_docs, &mut ret); clean::TraitItem(Box::new(build_external_trait(cx, did))) } Res::Def(DefKind::Fn, did) => { @@ -95,27 +70,27 @@ pub(crate) fn try_inline( } Res::Def(DefKind::Struct, did) => { record_extern_fqn(cx, did, ItemType::Struct); - build_impls(cx, Some(parent_module), did, attrs_without_docs, &mut ret); + build_impls(cx, did, attrs_without_docs, &mut ret); clean::StructItem(build_struct(cx, did)) } Res::Def(DefKind::Union, did) => { record_extern_fqn(cx, did, ItemType::Union); - build_impls(cx, Some(parent_module), did, attrs_without_docs, &mut ret); + build_impls(cx, did, attrs_without_docs, &mut ret); clean::UnionItem(build_union(cx, did)) } Res::Def(DefKind::TyAlias, did) => { record_extern_fqn(cx, did, ItemType::Typedef); - build_impls(cx, Some(parent_module), did, attrs_without_docs, &mut ret); + build_impls(cx, did, attrs_without_docs, &mut ret); clean::TypedefItem(build_type_alias(cx, did)) } Res::Def(DefKind::Enum, did) => { record_extern_fqn(cx, did, ItemType::Enum); - build_impls(cx, Some(parent_module), did, attrs_without_docs, &mut ret); + build_impls(cx, did, attrs_without_docs, &mut ret); clean::EnumItem(build_enum(cx, did)) } Res::Def(DefKind::ForeignTy, did) => { record_extern_fqn(cx, did, ItemType::ForeignType); - build_impls(cx, Some(parent_module), did, attrs_without_docs, &mut ret); + build_impls(cx, did, attrs_without_docs, &mut ret); clean::ForeignTypeItem } // Never inline enum variants but leave them shown as re-exports. @@ -149,7 +124,7 @@ pub(crate) fn try_inline( _ => return None, }; - let (attrs, cfg) = merge_attrs(cx, Some(parent_module), load_attrs(cx, did), attrs); + let (attrs, cfg) = merge_attrs(cx, load_attrs(cx, did), attrs); cx.inlined.insert(did.into()); let mut item = clean::Item::from_def_id_and_attrs_and_parts(did, Some(name), kind, Box::new(attrs), cfg); @@ -316,9 +291,8 @@ fn build_type_alias(cx: &mut DocContext<'_>, did: DefId) -> Box /// Builds all inherent implementations of an ADT (struct/union/enum) or Trait item/path/reexport. pub(crate) fn build_impls( cx: &mut DocContext<'_>, - parent_module: Option, did: DefId, - attrs: Option<&[ast::Attribute]>, + attrs: Option<(&[ast::Attribute], Option)>, ret: &mut Vec, ) { let _prof_timer = cx.tcx.sess.prof.generic_activity("build_inherent_impls"); @@ -326,7 +300,7 @@ pub(crate) fn build_impls( // for each implementation of an item represented by `did`, build the clean::Item for that impl for &did in tcx.inherent_impls(did).iter() { - build_impl(cx, parent_module, did, attrs, ret); + build_impl(cx, did, attrs, ret); } // This pretty much exists expressly for `dyn Error` traits that exist in the `alloc` crate. @@ -340,28 +314,26 @@ pub(crate) fn build_impls( let type_ = if tcx.is_trait(did) { TraitSimplifiedType(did) } else { AdtSimplifiedType(did) }; for &did in tcx.incoherent_impls(type_) { - build_impl(cx, parent_module, did, attrs, ret); + build_impl(cx, did, attrs, ret); } } } -/// `parent_module` refers to the parent of the re-export, not the original item pub(crate) fn merge_attrs( cx: &mut DocContext<'_>, - parent_module: Option, old_attrs: &[ast::Attribute], - new_attrs: Option<&[ast::Attribute]>, + new_attrs: Option<(&[ast::Attribute], Option)>, ) -> (clean::Attributes, Option>) { // NOTE: If we have additional attributes (from a re-export), // always insert them first. This ensure that re-export // doc comments show up before the original doc comments // when we render them. - if let Some(inner) = new_attrs { + if let Some((inner, item_id)) = new_attrs { let mut both = inner.to_vec(); both.extend_from_slice(old_attrs); ( - if let Some(new_id) = parent_module { - Attributes::from_ast_with_additional(old_attrs, (inner, new_id)) + if let Some(item_id) = item_id { + Attributes::from_ast_with_additional(old_attrs, (inner, item_id)) } else { Attributes::from_ast(&both) }, @@ -375,9 +347,8 @@ pub(crate) fn merge_attrs( /// Inline an `impl`, inherent or of a trait. The `did` must be for an `impl`. pub(crate) fn build_impl( cx: &mut DocContext<'_>, - parent_module: Option, did: DefId, - attrs: Option<&[ast::Attribute]>, + attrs: Option<(&[ast::Attribute], Option)>, ret: &mut Vec, ) { if !cx.inlined.insert(did.into()) { @@ -539,7 +510,7 @@ pub(crate) fn build_impl( record_extern_trait(cx, did); } - let (merged_attrs, cfg) = merge_attrs(cx, parent_module, load_attrs(cx, did), attrs); + let (merged_attrs, cfg) = merge_attrs(cx, load_attrs(cx, did), attrs); trace!("merged_attrs={:?}", merged_attrs); trace!( @@ -635,7 +606,7 @@ fn build_module_items( cfg: None, inline_stmt_id: None, }); - } else if let Some(i) = try_inline(cx, did, None, res, item.ident.name, None, visited) { + } else if let Some(i) = try_inline(cx, res, item.ident.name, None, visited) { items.extend(i) } } diff --git a/src/librustdoc/clean/mod.rs b/src/librustdoc/clean/mod.rs index e3e5454ef5443..2e1f456f50e2f 100644 --- a/src/librustdoc/clean/mod.rs +++ b/src/librustdoc/clean/mod.rs @@ -2388,12 +2388,12 @@ fn clean_maybe_renamed_item<'tcx>( target_attrs.extend_from_slice(inline::load_attrs(cx, def_id)); } - let import_parent = import_id.map(|import_id| cx.tcx.local_parent(import_id).to_def_id()); - let (attrs, cfg) = merge_attrs(cx, import_parent, &target_attrs, Some(&import_attrs)); + let import_id = import_id.map(|def_id| def_id.to_def_id()); + let (attrs, cfg) = merge_attrs(cx, &target_attrs, Some((&import_attrs, import_id))); let mut item = Item::from_def_id_and_attrs_and_parts(def_id, Some(name), kind, Box::new(attrs), cfg); - item.inline_stmt_id = import_id.map(|def_id| def_id.to_def_id()); + item.inline_stmt_id = import_id; vec![item] }) } @@ -2478,18 +2478,12 @@ fn clean_extern_crate<'tcx>( let krate_owner_def_id = krate.owner_id.to_def_id(); if please_inline { - let mut visited = DefIdSet::default(); - - let res = Res::Def(DefKind::Mod, crate_def_id); - if let Some(items) = inline::try_inline( cx, - cx.tcx.parent_module(krate.hir_id()).to_def_id(), - Some(krate_owner_def_id), - res, + Res::Def(DefKind::Mod, crate_def_id), name, - Some(attrs), - &mut visited, + Some((attrs, Some(krate_owner_def_id))), + &mut Default::default(), ) { return items; } @@ -2613,17 +2607,13 @@ fn clean_use_statement_inner<'tcx>( denied = true; } if !denied { - let mut visited = DefIdSet::default(); let import_def_id = import.owner_id.to_def_id(); - if let Some(mut items) = inline::try_inline( cx, - cx.tcx.parent_module(import.hir_id()).to_def_id(), - Some(import_def_id), path.res, name, - Some(attrs), - &mut visited, + Some((attrs, Some(import_def_id))), + &mut Default::default(), ) { items.push(Item::from_def_id_and_parts( import_def_id, diff --git a/src/librustdoc/clean/types/tests.rs b/src/librustdoc/clean/types/tests.rs index 20627c2cfc164..8f2331785f50d 100644 --- a/src/librustdoc/clean/types/tests.rs +++ b/src/librustdoc/clean/types/tests.rs @@ -10,7 +10,7 @@ use rustc_span::symbol::Symbol; fn create_doc_fragment(s: &str) -> Vec { vec![DocFragment { span: DUMMY_SP, - parent_module: None, + item_id: None, doc: Symbol::intern(s), kind: DocFragmentKind::SugaredDoc, indent: 0, diff --git a/src/librustdoc/clean/utils.rs b/src/librustdoc/clean/utils.rs index cafb00df51ed2..cca50df0db213 100644 --- a/src/librustdoc/clean/utils.rs +++ b/src/librustdoc/clean/utils.rs @@ -195,12 +195,12 @@ pub(crate) fn build_deref_target_impls( if let Some(prim) = target.primitive_type() { let _prof_timer = cx.tcx.sess.prof.generic_activity("build_primitive_inherent_impls"); for did in prim.impls(tcx).filter(|did| !did.is_local()) { - inline::build_impl(cx, None, did, None, ret); + inline::build_impl(cx, did, None, ret); } } else if let Type::Path { path } = target { let did = path.def_id(); if !did.is_local() { - inline::build_impls(cx, None, did, None, ret); + inline::build_impls(cx, did, None, ret); } } } diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs index 6ed7b98999977..769cc7ddfae61 100644 --- a/src/librustdoc/passes/collect_intra_doc_links.rs +++ b/src/librustdoc/passes/collect_intra_doc_links.rs @@ -28,7 +28,7 @@ use std::mem; use std::ops::Range; use crate::clean::{self, utils::find_nearest_parent_module}; -use crate::clean::{Crate, Item, ItemId, ItemLink, PrimitiveType}; +use crate::clean::{Crate, Item, ItemLink, PrimitiveType}; use crate::core::DocContext; use crate::html::markdown::{markdown_links, MarkdownLink}; use crate::lint::{BROKEN_INTRA_DOC_LINKS, PRIVATE_INTRA_DOC_LINKS}; @@ -42,8 +42,7 @@ pub(crate) const COLLECT_INTRA_DOC_LINKS: Pass = Pass { }; fn collect_intra_doc_links(krate: Crate, cx: &mut DocContext<'_>) -> Crate { - let mut collector = - LinkCollector { cx, mod_ids: Vec::new(), visited_links: FxHashMap::default() }; + let mut collector = LinkCollector { cx, visited_links: FxHashMap::default() }; collector.visit_crate(&krate); krate } @@ -149,7 +148,7 @@ impl TryFrom for Res { #[derive(Debug)] struct UnresolvedPath<'a> { /// Item on which the link is resolved, used for resolving `Self`. - item_id: ItemId, + item_id: DefId, /// The scope the link was resolved in. module_id: DefId, /// If part of the link resolved, this has the `Res`. @@ -225,7 +224,7 @@ impl UrlFragment { #[derive(Clone, Debug, Hash, PartialEq, Eq)] struct ResolutionInfo { - item_id: ItemId, + item_id: DefId, module_id: DefId, dis: Option, path_str: Box, @@ -242,11 +241,6 @@ struct DiagnosticInfo<'a> { struct LinkCollector<'a, 'tcx> { cx: &'a mut DocContext<'tcx>, - /// A stack of modules used to decide what scope to resolve in. - /// - /// The last module will be used if the parent scope of the current item is - /// unknown. - mod_ids: Vec, /// Cache the resolved links so we can avoid resolving (and emitting errors for) the same link. /// The link will be `None` if it could not be resolved (i.e. the error was cached). visited_links: FxHashMap)>>, @@ -262,7 +256,7 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { fn variant_field<'path>( &self, path_str: &'path str, - item_id: ItemId, + item_id: DefId, module_id: DefId, ) -> Result<(Res, DefId), UnresolvedPath<'path>> { let tcx = self.cx.tcx; @@ -333,35 +327,33 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { }) } - fn resolve_self_ty(&self, path_str: &str, ns: Namespace, item_id: ItemId) -> Option { + fn resolve_self_ty(&self, path_str: &str, ns: Namespace, item_id: DefId) -> Option { if ns != TypeNS || path_str != "Self" { return None; } let tcx = self.cx.tcx; - item_id - .as_def_id() - .map(|def_id| match tcx.def_kind(def_id) { - def_kind @ (DefKind::AssocFn - | DefKind::AssocConst - | DefKind::AssocTy - | DefKind::Variant - | DefKind::Field) => { - let parent_def_id = tcx.parent(def_id); - if def_kind == DefKind::Field && tcx.def_kind(parent_def_id) == DefKind::Variant - { - tcx.parent(parent_def_id) - } else { - parent_def_id - } + let self_id = match tcx.def_kind(item_id) { + def_kind @ (DefKind::AssocFn + | DefKind::AssocConst + | DefKind::AssocTy + | DefKind::Variant + | DefKind::Field) => { + let parent_def_id = tcx.parent(item_id); + if def_kind == DefKind::Field && tcx.def_kind(parent_def_id) == DefKind::Variant { + tcx.parent(parent_def_id) + } else { + parent_def_id } - _ => def_id, - }) - .and_then(|self_id| match tcx.def_kind(self_id) { - DefKind::Impl { .. } => self.def_id_to_res(self_id), - DefKind::Use => None, - def_kind => Some(Res::Def(def_kind, self_id)), - }) + } + _ => item_id, + }; + + match tcx.def_kind(self_id) { + DefKind::Impl { .. } => self.def_id_to_res(self_id), + DefKind::Use => None, + def_kind => Some(Res::Def(def_kind, self_id)), + } } /// Convenience wrapper around `doc_link_resolutions`. @@ -373,7 +365,7 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { &self, path_str: &str, ns: Namespace, - item_id: ItemId, + item_id: DefId, module_id: DefId, ) -> Option { if let res @ Some(..) = self.resolve_self_ty(path_str, ns, item_id) { @@ -400,7 +392,7 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { &mut self, path_str: &'path str, ns: Namespace, - item_id: ItemId, + item_id: DefId, module_id: DefId, ) -> Result<(Res, Option), UnresolvedPath<'path>> { if let Some(res) = self.resolve_path(path_str, ns, item_id, module_id) { @@ -779,48 +771,31 @@ fn is_derive_trait_collision(ns: &PerNS DocVisitor for LinkCollector<'a, 'tcx> { fn visit_item(&mut self, item: &Item) { - let parent_node = - item.item_id.as_def_id().and_then(|did| find_nearest_parent_module(self.cx.tcx, did)); - if parent_node.is_some() { - trace!("got parent node for {:?} {:?}, id {:?}", item.type_(), item.name, item.item_id); - } - - let inner_docs = item.inner_docs(self.cx.tcx); - - if item.is_mod() && inner_docs { - self.mod_ids.push(item.item_id.expect_def_id()); - } - // We want to resolve in the lexical scope of the documentation. // In the presence of re-exports, this is not the same as the module of the item. // Rather than merging all documentation into one, resolve it one attribute at a time // so we know which module it came from. - for (parent_module, doc) in prepare_to_doc_link_resolution(&item.attrs.doc_strings) { + for (item_id, doc) in prepare_to_doc_link_resolution(&item.attrs.doc_strings) { if !may_have_doc_links(&doc) { continue; } debug!("combined_docs={}", doc); // NOTE: if there are links that start in one crate and end in another, this will not resolve them. // This is a degenerate case and it's not supported by rustdoc. - let parent_node = parent_module.or(parent_node); + let item_id = item_id.unwrap_or_else(|| item.item_id.expect_def_id()); + let module_id = match self.cx.tcx.def_kind(item_id) { + DefKind::Mod if item.inner_docs(self.cx.tcx) => item_id, + _ => find_nearest_parent_module(self.cx.tcx, item_id).unwrap(), + }; for md_link in preprocessed_markdown_links(&doc) { - let link = self.resolve_link(item, &doc, parent_node, &md_link); + let link = self.resolve_link(item, item_id, module_id, &doc, &md_link); if let Some(link) = link { self.cx.cache.intra_doc_links.entry(item.item_id).or_default().push(link); } } } - if item.is_mod() { - if !inner_docs { - self.mod_ids.push(item.item_id.expect_def_id()); - } - - self.visit_item_recur(item); - self.mod_ids.pop(); - } else { - self.visit_item_recur(item) - } + self.visit_item_recur(item) } } @@ -952,8 +927,9 @@ impl LinkCollector<'_, '_> { fn resolve_link( &mut self, item: &Item, + item_id: DefId, + module_id: DefId, dox: &str, - parent_node: Option, link: &PreprocessedMarkdownLink, ) -> Option { let PreprocessedMarkdownLink(pp_link, ori_link) = link; @@ -970,25 +946,9 @@ impl LinkCollector<'_, '_> { pp_link.as_ref().map_err(|err| err.report(self.cx, diag_info.clone())).ok()?; let disambiguator = *disambiguator; - // In order to correctly resolve intra-doc links we need to - // pick a base AST node to work from. If the documentation for - // this module came from an inner comment (//!) then we anchor - // our name resolution *inside* the module. If, on the other - // hand it was an outer comment (///) then we anchor the name - // resolution in the parent module on the basis that the names - // used are more likely to be intended to be parent names. For - // this, we set base_node to None for inner comments since - // we've already pushed this node onto the resolution stack but - // for outer comments we explicitly try and resolve against the - // parent_node first. - let inner_docs = item.inner_docs(self.cx.tcx); - let base_node = - if item.is_mod() && inner_docs { self.mod_ids.last().copied() } else { parent_node }; - let module_id = base_node.expect("doc link without parent module"); - let (mut res, fragment) = self.resolve_with_disambiguator_cached( ResolutionInfo { - item_id: item.item_id, + item_id, module_id, dis: disambiguator, path_str: path_str.clone(), @@ -1229,11 +1189,11 @@ impl LinkCollector<'_, '_> { let disambiguator = key.dis; let path_str = &key.path_str; let item_id = key.item_id; - let base_node = key.module_id; + let module_id = key.module_id; match disambiguator.map(Disambiguator::ns) { Some(expected_ns) => { - match self.resolve(path_str, expected_ns, item_id, base_node) { + match self.resolve(path_str, expected_ns, item_id, module_id) { Ok(res) => Some(res), Err(err) => { // We only looked in one namespace. Try to give a better error if possible. @@ -1243,7 +1203,7 @@ impl LinkCollector<'_, '_> { for other_ns in [TypeNS, ValueNS, MacroNS] { if other_ns != expected_ns { if let Ok(res) = - self.resolve(path_str, other_ns, item_id, base_node) + self.resolve(path_str, other_ns, item_id, module_id) { err = ResolutionFailure::WrongNamespace { res: full_res(self.cx.tcx, res), @@ -1260,7 +1220,7 @@ impl LinkCollector<'_, '_> { None => { // Try everything! let mut candidate = |ns| { - self.resolve(path_str, ns, item_id, base_node) + self.resolve(path_str, ns, item_id, module_id) .map_err(ResolutionFailure::NotResolved) }; diff --git a/src/librustdoc/passes/collect_trait_impls.rs b/src/librustdoc/passes/collect_trait_impls.rs index d32e8185d3f96..8d204ddb79e39 100644 --- a/src/librustdoc/passes/collect_trait_impls.rs +++ b/src/librustdoc/passes/collect_trait_impls.rs @@ -49,7 +49,7 @@ pub(crate) fn collect_trait_impls(mut krate: Crate, cx: &mut DocContext<'_>) -> let _prof_timer = cx.tcx.sess.prof.generic_activity("build_extern_trait_impls"); for &cnum in cx.tcx.crates(()) { for &impl_def_id in cx.tcx.trait_impls_in_crate(cnum) { - inline::build_impl(cx, None, impl_def_id, None, &mut new_items_external); + inline::build_impl(cx, impl_def_id, None, &mut new_items_external); } } } @@ -75,7 +75,7 @@ pub(crate) fn collect_trait_impls(mut krate: Crate, cx: &mut DocContext<'_>) -> ); parent = cx.tcx.opt_parent(did); } - inline::build_impl(cx, None, impl_def_id, Some(&attr_buf), &mut new_items_local); + inline::build_impl(cx, impl_def_id, Some((&attr_buf, None)), &mut new_items_local); attr_buf.clear(); } } @@ -84,7 +84,7 @@ pub(crate) fn collect_trait_impls(mut krate: Crate, cx: &mut DocContext<'_>) -> for def_id in PrimitiveType::all_impls(cx.tcx) { // Try to inline primitive impls from other crates. if !def_id.is_local() { - inline::build_impl(cx, None, def_id, None, &mut new_items_external); + inline::build_impl(cx, def_id, None, &mut new_items_external); } } for (prim, did) in PrimitiveType::primitive_locations(cx.tcx) { diff --git a/src/librustdoc/passes/propagate_doc_cfg.rs b/src/librustdoc/passes/propagate_doc_cfg.rs index f35643af63738..8a33e51b3beb1 100644 --- a/src/librustdoc/passes/propagate_doc_cfg.rs +++ b/src/librustdoc/passes/propagate_doc_cfg.rs @@ -57,7 +57,8 @@ impl<'a, 'tcx> CfgPropagator<'a, 'tcx> { next_def_id = parent_def_id; } - let (_, cfg) = merge_attrs(self.cx, None, item.attrs.other_attrs.as_slice(), Some(&attrs)); + let (_, cfg) = + merge_attrs(self.cx, item.attrs.other_attrs.as_slice(), Some((&attrs, None))); item.cfg = cfg; } } diff --git a/tests/rustdoc-ui/intra-doc/auxiliary/inner-crate-doc.rs b/tests/rustdoc-ui/intra-doc/auxiliary/inner-crate-doc.rs new file mode 100644 index 0000000000000..15bf51e6f8e2b --- /dev/null +++ b/tests/rustdoc-ui/intra-doc/auxiliary/inner-crate-doc.rs @@ -0,0 +1 @@ +//! Inner doc comment diff --git a/tests/rustdoc-ui/intra-doc/import-inline-merge-module.rs b/tests/rustdoc-ui/intra-doc/import-inline-merge-module.rs new file mode 100644 index 0000000000000..4d6a325664578 --- /dev/null +++ b/tests/rustdoc-ui/intra-doc/import-inline-merge-module.rs @@ -0,0 +1,10 @@ +// Test for issue #108501. +// Module parent scope doesn't hijack import's parent scope for the import's doc links. + +// check-pass +// aux-build: inner-crate-doc.rs +// compile-flags: --extern inner_crate_doc --edition 2018 + +/// Import doc comment [inner_crate_doc] +#[doc(inline)] +pub use inner_crate_doc; From 0f45d855c33aea5e04ea9a1e154bee68fbaa61da Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Tue, 21 Mar 2023 17:37:37 +0400 Subject: [PATCH 14/17] rustdoc: Factor out some doc link resolution code into a separate function --- .../passes/collect_intra_doc_links.rs | 51 ++++++++++--------- 1 file changed, 27 insertions(+), 24 deletions(-) diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs index 769cc7ddfae61..789523c561e57 100644 --- a/src/librustdoc/passes/collect_intra_doc_links.rs +++ b/src/librustdoc/passes/collect_intra_doc_links.rs @@ -771,30 +771,7 @@ fn is_derive_trait_collision(ns: &PerNS DocVisitor for LinkCollector<'a, 'tcx> { fn visit_item(&mut self, item: &Item) { - // We want to resolve in the lexical scope of the documentation. - // In the presence of re-exports, this is not the same as the module of the item. - // Rather than merging all documentation into one, resolve it one attribute at a time - // so we know which module it came from. - for (item_id, doc) in prepare_to_doc_link_resolution(&item.attrs.doc_strings) { - if !may_have_doc_links(&doc) { - continue; - } - debug!("combined_docs={}", doc); - // NOTE: if there are links that start in one crate and end in another, this will not resolve them. - // This is a degenerate case and it's not supported by rustdoc. - let item_id = item_id.unwrap_or_else(|| item.item_id.expect_def_id()); - let module_id = match self.cx.tcx.def_kind(item_id) { - DefKind::Mod if item.inner_docs(self.cx.tcx) => item_id, - _ => find_nearest_parent_module(self.cx.tcx, item_id).unwrap(), - }; - for md_link in preprocessed_markdown_links(&doc) { - let link = self.resolve_link(item, item_id, module_id, &doc, &md_link); - if let Some(link) = link { - self.cx.cache.intra_doc_links.entry(item.item_id).or_default().push(link); - } - } - } - + self.resolve_links(item); self.visit_item_recur(item) } } @@ -921,6 +898,32 @@ fn preprocessed_markdown_links(s: &str) -> Vec { } impl LinkCollector<'_, '_> { + fn resolve_links(&mut self, item: &Item) { + // We want to resolve in the lexical scope of the documentation. + // In the presence of re-exports, this is not the same as the module of the item. + // Rather than merging all documentation into one, resolve it one attribute at a time + // so we know which module it came from. + for (item_id, doc) in prepare_to_doc_link_resolution(&item.attrs.doc_strings) { + if !may_have_doc_links(&doc) { + continue; + } + debug!("combined_docs={}", doc); + // NOTE: if there are links that start in one crate and end in another, this will not resolve them. + // This is a degenerate case and it's not supported by rustdoc. + let item_id = item_id.unwrap_or_else(|| item.item_id.expect_def_id()); + let module_id = match self.cx.tcx.def_kind(item_id) { + DefKind::Mod if item.inner_docs(self.cx.tcx) => item_id, + _ => find_nearest_parent_module(self.cx.tcx, item_id).unwrap(), + }; + for md_link in preprocessed_markdown_links(&doc) { + let link = self.resolve_link(item, item_id, module_id, &doc, &md_link); + if let Some(link) = link { + self.cx.cache.intra_doc_links.entry(item.item_id).or_default().push(link); + } + } + } + } + /// This is the entry point for resolving an intra-doc link. /// /// FIXME(jynelson): this is way too many arguments From dfbf61029fec121026eb7a77731cfd52e88a3f4c Mon Sep 17 00:00:00 2001 From: Ian Douglas Scott Date: Sun, 19 Mar 2023 19:00:17 -0700 Subject: [PATCH 15/17] Set LLVM `LLVM_UNREACHABLE_OPTIMIZE` to `OFF` This option was added to LLVM in https://reviews.llvm.org/D121750?id=416339. It makes `llvm_unreachable` in builds without assertions compile to an `LLVM_BUILTIN_TRAP` instead of `LLVM_BUILTIN_UNREACHABLE` (which causes undefined behavior and is equivalent to `std::hint::unreachable_unchecked`). Having compiler bugs triggering undefined behavior generally seems undesirable and inconsistent with Rust's goals. There is a check in `src/tools/tidy/src/style.rs` to reject code using `llvm_unreachable`. But it is used a lot within LLVM itself. For instance, this changes a failure I get compiling `libcore` for m68k from a `SIGSEGV` to `SIGILL`, which seems better though it still doesn't provide a useful message without switching to an LLVM build with asserts. It may be best not to do this if it noticeably degrades compiler performance, but worthwhile if it doesn't do so in any significant way. I haven't looked into what benchmarks there are for Rustc. That should be considered before merging. --- src/bootstrap/download-ci-llvm-stamp | 2 +- src/bootstrap/native.rs | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/bootstrap/download-ci-llvm-stamp b/src/bootstrap/download-ci-llvm-stamp index 94630e40f3c4c..36f9aaa595d0f 100644 --- a/src/bootstrap/download-ci-llvm-stamp +++ b/src/bootstrap/download-ci-llvm-stamp @@ -1,4 +1,4 @@ Change this file to make users of the `download-ci-llvm` configuration download a new version of LLVM from CI, even if the LLVM submodule hasn’t changed. -Last change is for: https://github.com/rust-lang/rust/pull/104748 +Last change is for: https://github.com/rust-lang/rust/pull/109373 diff --git a/src/bootstrap/native.rs b/src/bootstrap/native.rs index 41ee509655326..6f09c8307fca4 100644 --- a/src/bootstrap/native.rs +++ b/src/bootstrap/native.rs @@ -309,6 +309,7 @@ impl Step for Llvm { cfg.out_dir(&out_dir) .profile(profile) .define("LLVM_ENABLE_ASSERTIONS", assertions) + .define("LLVM_UNREACHABLE_OPTIMIZE", "OFF") .define("LLVM_ENABLE_PLUGINS", plugins) .define("LLVM_TARGETS_TO_BUILD", llvm_targets) .define("LLVM_EXPERIMENTAL_TARGETS_TO_BUILD", llvm_exp_targets) From 4212c1b0679da78295d3efc985ddbb62fa83076b Mon Sep 17 00:00:00 2001 From: clubby789 Date: Tue, 21 Mar 2023 17:51:21 +0000 Subject: [PATCH 16/17] Add `safe` to number rendering --- src/librustdoc/html/templates/source.html | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/librustdoc/html/templates/source.html b/src/librustdoc/html/templates/source.html index 968b55ac158dc..a224ff12f448e 100644 --- a/src/librustdoc/html/templates/source.html +++ b/src/librustdoc/html/templates/source.html @@ -2,9 +2,9 @@ 
         {% for line in lines.clone() %}
             {% if embedded %}
-                {{line}}
+                {{line|safe}}
             {%~ else %}
-                {{line}}
+                {{line|safe}}
             {%~ endif %}
         {% endfor %}
{# #} From df034b06b72f97a39844ce84a648e4de7fa2c58d Mon Sep 17 00:00:00 2001 From: Trevor Gross Date: Fri, 10 Mar 2023 13:23:46 -0500 Subject: [PATCH 17/17] Change text -> rust,ignore highlighting in sanitizer.md Marked ignore due to difficulty getting doctests to pass cross-platform --- src/doc/unstable-book/src/compiler-flags/sanitizer.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/doc/unstable-book/src/compiler-flags/sanitizer.md b/src/doc/unstable-book/src/compiler-flags/sanitizer.md index 262cef3454ad3..f71aceff4550c 100644 --- a/src/doc/unstable-book/src/compiler-flags/sanitizer.md +++ b/src/doc/unstable-book/src/compiler-flags/sanitizer.md @@ -213,7 +213,7 @@ See the [Clang ControlFlowIntegrity documentation][clang-cfi] for more details. ## Example -```text +```rust,ignore #![feature(naked_functions)] use std::arch::asm; @@ -238,7 +238,7 @@ pub extern "C" fn add_two(x: i32) { nop nop nop - lea rax, [rdi+2] + lea eax, [edi+2] ret ", options(noreturn)