From 13e58755d7c692c48e78faa68989e66473708119 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 30 Sep 2023 22:33:22 +0200 Subject: [PATCH 01/18] add some docs to hooks/mod.rs --- compiler/rustc_middle/src/hooks/mod.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/compiler/rustc_middle/src/hooks/mod.rs b/compiler/rustc_middle/src/hooks/mod.rs index 12aeae1772568..572751d951189 100644 --- a/compiler/rustc_middle/src/hooks/mod.rs +++ b/compiler/rustc_middle/src/hooks/mod.rs @@ -1,3 +1,8 @@ +//! "Hooks" provide a way for `tcx` functionality to be provided by some downstream crate without +//! everything in rustc having to depend on that crate. This is somewhat similar to queries, but +//! queries come with a lot of machinery for caching and incremental compilation, whereas hooks are +//! just plain function pointers without any of the query magic. + use crate::mir; use crate::query::TyCtxtAt; use crate::ty::{Ty, TyCtxt}; From 23efab4811c44e315ffd0322c69cbcbc3125eddf Mon Sep 17 00:00:00 2001 From: Ikko Eltociear Ashimine Date: Thu, 5 Oct 2023 00:03:04 +0900 Subject: [PATCH 02/18] Fix typo in attrs.rs documenation -> documentation --- src/tools/clippy/clippy_lints/src/attrs.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/tools/clippy/clippy_lints/src/attrs.rs b/src/tools/clippy/clippy_lints/src/attrs.rs index 0546807bac4fb..db01ddbde04e3 100644 --- a/src/tools/clippy/clippy_lints/src/attrs.rs +++ b/src/tools/clippy/clippy_lints/src/attrs.rs @@ -183,7 +183,7 @@ declare_clippy_lint! { declare_clippy_lint! { /// ### What it does - /// Checks for empty lines after documenation comments. + /// Checks for empty lines after documentation comments. /// /// ### Why is this bad? /// The documentation comment was most likely meant to be an inner attribute or regular comment. @@ -795,7 +795,7 @@ impl EarlyLintPass for EarlyAttributes { /// Check for empty lines after outer attributes. /// -/// Attributes and documenation comments are both considered outer attributes +/// Attributes and documentation comments are both considered outer attributes /// by the AST. However, the average user likely considers them to be different. /// Checking for empty lines after each of these attributes is split into two different /// lints but can share the same logic. From 80bf6883be9af0bee3e764a3cdc0ac73c0cc239a Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Fri, 6 Oct 2023 10:16:54 +1100 Subject: [PATCH 03/18] Remove an unnecessary `pub(crate)`. --- compiler/rustc_transmute/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/rustc_transmute/src/lib.rs b/compiler/rustc_transmute/src/lib.rs index 6c49e94dc310b..e0109657c39a2 100644 --- a/compiler/rustc_transmute/src/lib.rs +++ b/compiler/rustc_transmute/src/lib.rs @@ -9,7 +9,7 @@ extern crate tracing; pub(crate) use rustc_data_structures::fx::{FxIndexMap as Map, FxIndexSet as Set}; pub mod layout; -pub(crate) mod maybe_transmutable; +mod maybe_transmutable; #[derive(Default)] pub struct Assume { From 409193654d6252a4cfa674588a77668ba653e384 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Fri, 6 Oct 2023 10:17:29 +1100 Subject: [PATCH 04/18] Make the comment order match variant declaration order. --- compiler/rustc_transmute/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/rustc_transmute/src/lib.rs b/compiler/rustc_transmute/src/lib.rs index e0109657c39a2..4b55963201717 100644 --- a/compiler/rustc_transmute/src/lib.rs +++ b/compiler/rustc_transmute/src/lib.rs @@ -19,7 +19,7 @@ pub struct Assume { pub validity: bool, } -/// Either we have an error, transmutation is allowed, or we have an optional +/// Either transmutation is allowed, we have an error, or we have an optional /// Condition that must hold. #[derive(Debug, Hash, Eq, PartialEq, Clone)] pub enum Answer { From 449b84cb99203a1eaa735f49958b6ddabc8de779 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Fri, 6 Oct 2023 11:20:02 +1100 Subject: [PATCH 05/18] Remove `map_layouts`. As per the `FIXME` comment, it's an abstraction that makes the code harder to read. --- .../src/maybe_transmutable/mod.rs | 90 +++++++------------ 1 file changed, 33 insertions(+), 57 deletions(-) diff --git a/compiler/rustc_transmute/src/maybe_transmutable/mod.rs b/compiler/rustc_transmute/src/maybe_transmutable/mod.rs index c0141f1f84149..ae193ef99e68a 100644 --- a/compiler/rustc_transmute/src/maybe_transmutable/mod.rs +++ b/compiler/rustc_transmute/src/maybe_transmutable/mod.rs @@ -32,26 +32,6 @@ where ) -> Self { Self { src, dst, scope, assume, context } } - - // FIXME(bryangarza): Delete this when all usages are removed - pub(crate) fn map_layouts( - self, - f: F, - ) -> Result, Answer<::Ref>> - where - F: FnOnce( - L, - L, - ::Scope, - &C, - ) -> Result<(M, M), Answer<::Ref>>, - { - let Self { src, dst, scope, assume, context } = self; - - let (src, dst) = f(src, dst, scope, &context)?; - - Ok(MaybeTransmutableQuery { src, dst, scope, assume, context }) - } } // FIXME: Nix this cfg, so we can write unit tests independently of rustc @@ -107,42 +87,42 @@ where #[instrument(level = "debug", skip(self), fields(src = ?self.src, dst = ?self.dst))] pub(crate) fn answer(self) -> Answer<::Ref> { let assume_visibility = self.assume.safety; - // FIXME(bryangarza): Refactor this code to get rid of `map_layouts` - let query_or_answer = self.map_layouts(|src, dst, scope, context| { - // Remove all `Def` nodes from `src`, without checking their visibility. - let src = src.prune(&|def| true); - trace!(?src, "pruned src"); + let Self { src, dst, scope, assume, context } = self; - // Remove all `Def` nodes from `dst`, additionally... - let dst = if assume_visibility { - // ...if visibility is assumed, don't check their visibility. - dst.prune(&|def| true) - } else { - // ...otherwise, prune away all unreachable paths through the `Dst` layout. - dst.prune(&|def| context.is_accessible_from(def, scope)) - }; + // Remove all `Def` nodes from `src`, without checking their visibility. + let src = src.prune(&|def| true); - trace!(?dst, "pruned dst"); + trace!(?src, "pruned src"); - // Convert `src` from a tree-based representation to an NFA-based representation. - // If the conversion fails because `src` is uninhabited, conclude that the transmutation - // is acceptable, because instances of the `src` type do not exist. - let src = Nfa::from_tree(src).map_err(|Uninhabited| Answer::Yes)?; + // Remove all `Def` nodes from `dst`, additionally... + let dst = if assume_visibility { + // ...if visibility is assumed, don't check their visibility. + dst.prune(&|def| true) + } else { + // ...otherwise, prune away all unreachable paths through the `Dst` layout. + dst.prune(&|def| context.is_accessible_from(def, scope)) + }; - // Convert `dst` from a tree-based representation to an NFA-based representation. - // If the conversion fails because `src` is uninhabited, conclude that the transmutation - // is unacceptable, because instances of the `dst` type do not exist. - let dst = - Nfa::from_tree(dst).map_err(|Uninhabited| Answer::No(Reason::DstIsPrivate))?; + trace!(?dst, "pruned dst"); - Ok((src, dst)) - }); + // Convert `src` from a tree-based representation to an NFA-based representation. + // If the conversion fails because `src` is uninhabited, conclude that the transmutation + // is acceptable, because instances of the `src` type do not exist. + let src = match Nfa::from_tree(src) { + Ok(src) => src, + Err(Uninhabited) => return Answer::Yes, + }; - match query_or_answer { - Ok(query) => query.answer(), - Err(answer) => answer, - } + // Convert `dst` from a tree-based representation to an NFA-based representation. + // If the conversion fails because `src` is uninhabited, conclude that the transmutation + // is unacceptable, because instances of the `dst` type do not exist. + let dst = match Nfa::from_tree(dst) { + Ok(dst) => dst, + Err(Uninhabited) => return Answer::No(Reason::DstIsPrivate), + }; + + MaybeTransmutableQuery { src, dst, scope, assume, context }.answer() } } @@ -156,14 +136,10 @@ where #[inline(always)] #[instrument(level = "debug", skip(self), fields(src = ?self.src, dst = ?self.dst))] pub(crate) fn answer(self) -> Answer<::Ref> { - // FIXME(bryangarza): Refactor this code to get rid of `map_layouts` - let query_or_answer = self - .map_layouts(|src, dst, scope, context| Ok((Dfa::from_nfa(src), Dfa::from_nfa(dst)))); - - match query_or_answer { - Ok(query) => query.answer(), - Err(answer) => answer, - } + let Self { src, dst, scope, assume, context } = self; + let src = Dfa::from_nfa(src); + let dst = Dfa::from_nfa(dst); + MaybeTransmutableQuery { src, dst, scope, assume, context }.answer() } } From 73420fc13bb23151f1baa7199ffa3a38ab955de4 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Fri, 6 Oct 2023 11:21:33 +1100 Subject: [PATCH 06/18] Fix a comment. It was duplicated from the method above. --- compiler/rustc_transmute/src/maybe_transmutable/mod.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/compiler/rustc_transmute/src/maybe_transmutable/mod.rs b/compiler/rustc_transmute/src/maybe_transmutable/mod.rs index ae193ef99e68a..6484a2968ff15 100644 --- a/compiler/rustc_transmute/src/maybe_transmutable/mod.rs +++ b/compiler/rustc_transmute/src/maybe_transmutable/mod.rs @@ -147,9 +147,7 @@ impl MaybeTransmutableQuery::Ref>, C> where C: QueryContext, { - /// Answers whether a `Nfa` is transmutable into another `Nfa`. - /// - /// This method converts `src` and `dst` to DFAs, then computes an answer using those DFAs. + /// Answers whether a `Dfa` is transmutable into another `Dfa`. pub(crate) fn answer(self) -> Answer<::Ref> { MaybeTransmutableQuery { src: &self.src, From 29ed8e492ab99391e9eaa1247fbafe2759f9c383 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Fri, 6 Oct 2023 11:25:32 +1100 Subject: [PATCH 07/18] Remove the `MaybeTransmutableQuery<&'l Dfa<...>, C>` impl. Because there is also a `MaybeTransmutableQuery, C>` impl, and we don't need both. --- .../src/maybe_transmutable/mod.rs | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/compiler/rustc_transmute/src/maybe_transmutable/mod.rs b/compiler/rustc_transmute/src/maybe_transmutable/mod.rs index 6484a2968ff15..bf3c390c8008e 100644 --- a/compiler/rustc_transmute/src/maybe_transmutable/mod.rs +++ b/compiler/rustc_transmute/src/maybe_transmutable/mod.rs @@ -149,22 +149,6 @@ where { /// Answers whether a `Dfa` is transmutable into another `Dfa`. pub(crate) fn answer(self) -> Answer<::Ref> { - MaybeTransmutableQuery { - src: &self.src, - dst: &self.dst, - scope: self.scope, - assume: self.assume, - context: self.context, - } - .answer() - } -} - -impl<'l, C> MaybeTransmutableQuery<&'l Dfa<::Ref>, C> -where - C: QueryContext, -{ - pub(crate) fn answer(&mut self) -> Answer<::Ref> { self.answer_memo(&mut Map::default(), self.src.start, self.dst.start) } From 442a66d385bc2cdaf53b042437b9997d7b163d6d Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Tue, 3 Oct 2023 17:03:58 +1100 Subject: [PATCH 08/18] Remove unneeded dependency. --- Cargo.lock | 1 - compiler/rustc_query_impl/Cargo.toml | 1 - 2 files changed, 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index f0a3553da2b4a..9e79cf55d20be 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4311,7 +4311,6 @@ dependencies = [ "rustc_errors", "rustc_hir", "rustc_index", - "rustc_macros", "rustc_middle", "rustc_query_system", "rustc_serialize", diff --git a/compiler/rustc_query_impl/Cargo.toml b/compiler/rustc_query_impl/Cargo.toml index a44dd5ede2fd2..a350e8b2e3a43 100644 --- a/compiler/rustc_query_impl/Cargo.toml +++ b/compiler/rustc_query_impl/Cargo.toml @@ -13,7 +13,6 @@ rustc_data_structures = { path = "../rustc_data_structures" } rustc_errors = { path = "../rustc_errors" } rustc_hir = { path = "../rustc_hir" } rustc_index = { path = "../rustc_index" } -rustc_macros = { path = "../rustc_macros" } rustc_middle = { path = "../rustc_middle" } rustc_query_system = { path = "../rustc_query_system" } rustc-rayon-core = { version = "0.5.0", optional = true } From 108e541cc2d73fc4548847a2cfddca5c3b3499e6 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Wed, 4 Oct 2023 15:37:44 +1100 Subject: [PATCH 09/18] Remove unused `FileName::CfgSpec`. --- compiler/rustc_session/src/config.rs | 1 - compiler/rustc_span/src/lib.rs | 7 +------ 2 files changed, 1 insertion(+), 7 deletions(-) diff --git a/compiler/rustc_session/src/config.rs b/compiler/rustc_session/src/config.rs index d29ab02c1a6ac..e120f595d92ef 100644 --- a/compiler/rustc_session/src/config.rs +++ b/compiler/rustc_session/src/config.rs @@ -813,7 +813,6 @@ impl Input { FileName::Anon(_) => None, FileName::MacroExpansion(_) => None, FileName::ProcMacroSourceCode(_) => None, - FileName::CfgSpec(_) => None, FileName::CliCrateAttr(_) => None, FileName::Custom(_) => None, FileName::DocTest(path, _) => Some(path), diff --git a/compiler/rustc_span/src/lib.rs b/compiler/rustc_span/src/lib.rs index 772e09291a139..6a7bca73e7c14 100644 --- a/compiler/rustc_span/src/lib.rs +++ b/compiler/rustc_span/src/lib.rs @@ -280,8 +280,7 @@ impl RealFileName { } /// Differentiates between real files and common virtual files. -#[derive(Debug, Eq, PartialEq, Clone, Ord, PartialOrd, Hash)] -#[derive(Decodable, Encodable)] +#[derive(Debug, Eq, PartialEq, Clone, Ord, PartialOrd, Hash, Decodable, Encodable)] pub enum FileName { Real(RealFileName), /// Call to `quote!`. @@ -292,8 +291,6 @@ pub enum FileName { // FIXME(jseyfried) MacroExpansion(Hash64), ProcMacroSourceCode(Hash64), - /// Strings provided as `--cfg [cfgspec]` stored in a `crate_cfg`. - CfgSpec(Hash64), /// Strings provided as crate attributes in the CLI. CliCrateAttr(Hash64), /// Custom sources for explicit parser calls from plugins and drivers. @@ -339,7 +336,6 @@ impl fmt::Display for FileNameDisplay<'_> { MacroExpansion(_) => write!(fmt, ""), Anon(_) => write!(fmt, ""), ProcMacroSourceCode(_) => write!(fmt, ""), - CfgSpec(_) => write!(fmt, ""), CliCrateAttr(_) => write!(fmt, ""), Custom(ref s) => write!(fmt, "<{s}>"), DocTest(ref path, _) => write!(fmt, "{}", path.display()), @@ -365,7 +361,6 @@ impl FileName { Anon(_) | MacroExpansion(_) | ProcMacroSourceCode(_) - | CfgSpec(_) | CliCrateAttr(_) | Custom(_) | QuoteExpansion(_) From e49a1470baee89823a49aa6ec7baa19aa033b88b Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Wed, 4 Oct 2023 18:27:18 +1100 Subject: [PATCH 10/18] Factor out `insert_or_error`. It appears identically as a closure in two functions. Also change its return type from `bool` to `Option<()>` so it can be used with `?`. --- compiler/rustc_attr/src/builtin.rs | 85 ++++++++---------------------- 1 file changed, 23 insertions(+), 62 deletions(-) diff --git a/compiler/rustc_attr/src/builtin.rs b/compiler/rustc_attr/src/builtin.rs index 6f82d6f932330..f013ff45a4fe5 100644 --- a/compiler/rustc_attr/src/builtin.rs +++ b/compiler/rustc_attr/src/builtin.rs @@ -353,28 +353,28 @@ pub fn find_body_stability( body_stab } +fn insert_or_error(sess: &Session, meta: &MetaItem, item: &mut Option) -> Option<()> { + if item.is_some() { + handle_errors( + &sess.parse_sess, + meta.span, + AttrError::MultipleItem(pprust::path_to_string(&meta.path)), + ); + None + } else if let Some(v) = meta.value_str() { + *item = Some(v); + Some(()) + } else { + sess.emit_err(session_diagnostics::IncorrectMetaItem { span: meta.span }); + None + } +} + /// Read the content of a `stable`/`rustc_const_stable` attribute, and return the feature name and /// its stability information. fn parse_stability(sess: &Session, attr: &Attribute) -> Option<(Symbol, StabilityLevel)> { let meta = attr.meta()?; let MetaItem { kind: MetaItemKind::List(ref metas), .. } = meta else { return None }; - let insert_or_error = |meta: &MetaItem, item: &mut Option| { - if item.is_some() { - handle_errors( - &sess.parse_sess, - meta.span, - AttrError::MultipleItem(pprust::path_to_string(&meta.path)), - ); - return false; - } - if let Some(v) = meta.value_str() { - *item = Some(v); - true - } else { - sess.emit_err(session_diagnostics::IncorrectMetaItem { span: meta.span }); - false - } - }; let mut feature = None; let mut since = None; @@ -389,16 +389,8 @@ fn parse_stability(sess: &Session, attr: &Attribute) -> Option<(Symbol, Stabilit }; match mi.name_or_empty() { - sym::feature => { - if !insert_or_error(mi, &mut feature) { - return None; - } - } - sym::since => { - if !insert_or_error(mi, &mut since) { - return None; - } - } + sym::feature => insert_or_error(sess, mi, &mut feature)?, + sym::since => insert_or_error(sess, mi, &mut since)?, _ => { handle_errors( &sess.parse_sess, @@ -438,23 +430,6 @@ fn parse_stability(sess: &Session, attr: &Attribute) -> Option<(Symbol, Stabilit fn parse_unstability(sess: &Session, attr: &Attribute) -> Option<(Symbol, StabilityLevel)> { let meta = attr.meta()?; let MetaItem { kind: MetaItemKind::List(ref metas), .. } = meta else { return None }; - let insert_or_error = |meta: &MetaItem, item: &mut Option| { - if item.is_some() { - handle_errors( - &sess.parse_sess, - meta.span, - AttrError::MultipleItem(pprust::path_to_string(&meta.path)), - ); - return false; - } - if let Some(v) = meta.value_str() { - *item = Some(v); - true - } else { - sess.emit_err(session_diagnostics::IncorrectMetaItem { span: meta.span }); - false - } - }; let mut feature = None; let mut reason = None; @@ -473,20 +448,10 @@ fn parse_unstability(sess: &Session, attr: &Attribute) -> Option<(Symbol, Stabil }; match mi.name_or_empty() { - sym::feature => { - if !insert_or_error(mi, &mut feature) { - return None; - } - } - sym::reason => { - if !insert_or_error(mi, &mut reason) { - return None; - } - } + sym::feature => insert_or_error(sess, mi, &mut feature)?, + sym::reason => insert_or_error(sess, mi, &mut reason)?, sym::issue => { - if !insert_or_error(mi, &mut issue) { - return None; - } + insert_or_error(sess, mi, &mut issue)?; // These unwraps are safe because `insert_or_error` ensures the meta item // is a name/value pair string literal. @@ -515,11 +480,7 @@ fn parse_unstability(sess: &Session, attr: &Attribute) -> Option<(Symbol, Stabil } is_soft = true; } - sym::implied_by => { - if !insert_or_error(mi, &mut implied_by) { - return None; - } - } + sym::implied_by => insert_or_error(sess, mi, &mut implied_by)?, _ => { handle_errors( &sess.parse_sess, From 4b51a3eb52c5ae292ba54c2044be695630fccc3e Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Fri, 6 Oct 2023 14:57:25 +1100 Subject: [PATCH 11/18] Remove unneeded dependency. Also sort them. --- Cargo.lock | 1 - compiler/rustc_smir/Cargo.toml | 7 +++---- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 9e79cf55d20be..54ca110ac59ce 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4416,7 +4416,6 @@ dependencies = [ "rustc_hir", "rustc_interface", "rustc_middle", - "rustc_session", "rustc_span", "rustc_target", "stable_mir", diff --git a/compiler/rustc_smir/Cargo.toml b/compiler/rustc_smir/Cargo.toml index 4c29f743708ac..3e0d6baab6aab 100644 --- a/compiler/rustc_smir/Cargo.toml +++ b/compiler/rustc_smir/Cargo.toml @@ -4,14 +4,13 @@ version = "0.0.0" edition = "2021" [dependencies] +rustc_driver = { path = "../rustc_driver" } rustc_hir = { path = "../rustc_hir" } +rustc_interface = { path = "../rustc_interface" } rustc_middle = { path = "../rustc_middle" } rustc_span = { path = "../rustc_span" } rustc_target = { path = "../rustc_target" } -rustc_driver = { path = "../rustc_driver" } -rustc_interface = { path = "../rustc_interface" } -rustc_session = {path = "../rustc_session" } -tracing = "0.1" stable_mir = {path = "../stable_mir" } +tracing = "0.1" [features] From 093b435b786dfd816aba790f0097943258a1b366 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Fri, 6 Oct 2023 14:58:01 +1100 Subject: [PATCH 12/18] Remove unneeded features. --- compiler/rustc_smir/src/lib.rs | 4 ---- 1 file changed, 4 deletions(-) diff --git a/compiler/rustc_smir/src/lib.rs b/compiler/rustc_smir/src/lib.rs index b6c36678db52f..d10f46fad9e28 100644 --- a/compiler/rustc_smir/src/lib.rs +++ b/compiler/rustc_smir/src/lib.rs @@ -10,10 +10,6 @@ html_root_url = "https://doc.rust-lang.org/nightly/nightly-rustc/", test(attr(allow(unused_variables), deny(warnings))) )] -#![feature(rustc_private)] -#![feature(ptr_metadata)] -#![feature(type_alias_impl_trait)] // Used to define opaque types. -#![feature(intra_doc_pointers)] pub mod rustc_internal; From e7dabc9f877d76e0a3432100d83b14b97978533d Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Fri, 6 Oct 2023 14:59:11 +1100 Subject: [PATCH 13/18] Remove unnecessary `pub`. --- compiler/rustc_smir/src/rustc_internal/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/rustc_smir/src/rustc_internal/mod.rs b/compiler/rustc_smir/src/rustc_internal/mod.rs index 1a9dea99f643f..36eb2247253a0 100644 --- a/compiler/rustc_smir/src/rustc_internal/mod.rs +++ b/compiler/rustc_smir/src/rustc_internal/mod.rs @@ -11,7 +11,7 @@ use rustc_driver::{Callbacks, Compilation, RunCompiler}; use rustc_interface::{interface, Queries}; use rustc_middle::mir::interpret::AllocId; use rustc_middle::ty::TyCtxt; -pub use rustc_span::def_id::{CrateNum, DefId}; +use rustc_span::def_id::{CrateNum, DefId}; use rustc_span::Span; use stable_mir::CompilerError; From b80e653ca1278a2c4fa411b938be1ccc7ed204fb Mon Sep 17 00:00:00 2001 From: Scott McMurray Date: Sun, 6 Aug 2023 14:54:55 -0700 Subject: [PATCH 14/18] Attempt to describe the intent behind the `From` trait further --- library/core/src/convert/mod.rs | 34 +++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/library/core/src/convert/mod.rs b/library/core/src/convert/mod.rs index ff5a4c913b7ed..db9f7237bd6c3 100644 --- a/library/core/src/convert/mod.rs +++ b/library/core/src/convert/mod.rs @@ -478,6 +478,40 @@ pub trait Into: Sized { /// - `From for U` implies [`Into`]` for T` /// - `From` is reflexive, which means that `From for T` is implemented /// +/// # When to implement `From` +/// +/// While there's no technical restrictions on which conversions can be done using +/// a `From` implementation, the general expectation is that the conversions +/// should typically be restricted as follows: +/// +/// * The conversion is *lossless*: it cannot fail and it's possible to recover +/// the original value. For example, `i32: From` exists, where the original +/// value can be recovered using `u16: TryFrom`. And `String: From<&str>` +/// exists, where you can get something equivalent to the original value via +/// `Deref`. But `From` cannot be used to convert from `u32` to `u16`, since +/// that cannot succeed in a lossless way. +/// +/// * The conversion is *value-preserving*: the conceptual kind and meaning of +/// the resulting value is the same, even though the Rust type and technical +/// representation might be different. For example `-1_i8 as u8` is *lossless*, +/// since `as` casting back can recover the original value, but that conversion +/// is *not* available via `From` because `-1` and `255` are different conceptual +/// values (despite being identical bit patterns technically). But +/// `f32: From` *is* available because `1_i16` and `1.0_f32` are conceptually +/// the same real number (despite having very different bit patterns technically). +/// `String: From` is available because they're both *text*, but +/// `String: From` is *not* available, since `1` (a number) and `"1"` +/// (text) are too different. (Converting values to text is instead covered +/// by the [`Display`](crate::fmt::Display) trait.) +/// +/// * The conversion is *obvious*: it's the only reasonable conversion between +/// the two types. Otherwise it's better to have it be a named method or +/// constructor, like how [`str::as_bytes`] is a method and how integers have +/// methods like [`u32::from_ne_bytes`], [`u32::from_le_bytes`], and +/// [`u32::from_be_bytes`], none of which are `From` implementations. Whereas +/// there's only one reasonable way to wrap an [`Ipv6Addr`](crate::net::Ipv6Addr) +/// into an [`IpAddr`](crate::net::IpAddr), thus `IpAddr: From` exists. +/// /// # Examples /// /// [`String`] implements `From<&str>`: From 44f92c1f805434866b9744a6c8953ecdd8cc36f9 Mon Sep 17 00:00:00 2001 From: scottmcm Date: Fri, 6 Oct 2023 05:31:54 +0000 Subject: [PATCH 15/18] Don't mention "recover the original" in `From` docs Co-authored-by: Josh Triplett --- library/core/src/convert/mod.rs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/library/core/src/convert/mod.rs b/library/core/src/convert/mod.rs index db9f7237bd6c3..aed5552ef91f0 100644 --- a/library/core/src/convert/mod.rs +++ b/library/core/src/convert/mod.rs @@ -484,8 +484,11 @@ pub trait Into: Sized { /// a `From` implementation, the general expectation is that the conversions /// should typically be restricted as follows: /// -/// * The conversion is *lossless*: it cannot fail and it's possible to recover -/// the original value. For example, `i32: From` exists, where the original +/// * The conversion is *infallible*: if the conversion can fail, use `TryFrom` +/// instead; don't provide a `From` impl that panics. +/// +/// * The conversion is *lossless*: semantically, it should not lose or discard +/// information. For example, `i32: From` exists, where the original /// value can be recovered using `u16: TryFrom`. And `String: From<&str>` /// exists, where you can get something equivalent to the original value via /// `Deref`. But `From` cannot be used to convert from `u32` to `u16`, since From 1651f1f4b8f97a51f1699101cfe03ea129ec7a07 Mon Sep 17 00:00:00 2001 From: Scott McMurray Date: Thu, 5 Oct 2023 23:03:02 -0700 Subject: [PATCH 16/18] Elaborate some caveats to lossless --- library/core/src/convert/mod.rs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/library/core/src/convert/mod.rs b/library/core/src/convert/mod.rs index aed5552ef91f0..c4b867a480914 100644 --- a/library/core/src/convert/mod.rs +++ b/library/core/src/convert/mod.rs @@ -484,7 +484,7 @@ pub trait Into: Sized { /// a `From` implementation, the general expectation is that the conversions /// should typically be restricted as follows: /// -/// * The conversion is *infallible*: if the conversion can fail, use `TryFrom` +/// * The conversion is *infallible*: if the conversion can fail, use [`TryFrom`] /// instead; don't provide a `From` impl that panics. /// /// * The conversion is *lossless*: semantically, it should not lose or discard @@ -492,7 +492,10 @@ pub trait Into: Sized { /// value can be recovered using `u16: TryFrom`. And `String: From<&str>` /// exists, where you can get something equivalent to the original value via /// `Deref`. But `From` cannot be used to convert from `u32` to `u16`, since -/// that cannot succeed in a lossless way. +/// that cannot succeed in a lossless way. (There's some wiggle room here for +/// information not considered semantically relevant. For example, +/// `Box<[T]>: From>` exists even though it might not preserve capacity, +/// like how two vectors can be equal despite differing capacities.) /// /// * The conversion is *value-preserving*: the conceptual kind and meaning of /// the resulting value is the same, even though the Rust type and technical From 5432d13bb04cecf213c2753b25cfb1e366cb8026 Mon Sep 17 00:00:00 2001 From: Scott McMurray Date: Fri, 6 Oct 2023 01:41:48 -0700 Subject: [PATCH 17/18] Reuse existing `Some`s in `Option::(x)or` LLVM still has trouble re-using discriminants sometimes when rebuilding a two-variant enum, so when we have the correct variant already built, just use it. That's simpler in LLVM *and* in MIR, so might as well: --- library/core/src/option.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/library/core/src/option.rs b/library/core/src/option.rs index 9899014e85a75..bdaeea666221c 100644 --- a/library/core/src/option.rs +++ b/library/core/src/option.rs @@ -1475,7 +1475,7 @@ impl Option { #[stable(feature = "rust1", since = "1.0.0")] pub fn or(self, optb: Option) -> Option { match self { - Some(x) => Some(x), + x @ Some(_) => x, None => optb, } } @@ -1500,7 +1500,7 @@ impl Option { F: FnOnce() -> Option, { match self { - Some(x) => Some(x), + x @ Some(_) => x, None => f(), } } @@ -1530,8 +1530,8 @@ impl Option { #[stable(feature = "option_xor", since = "1.37.0")] pub fn xor(self, optb: Option) -> Option { match (self, optb) { - (Some(a), None) => Some(a), - (None, Some(b)) => Some(b), + (a @ Some(_), None) => a, + (None, b @ Some(_)) => b, _ => None, } } From c95015c2955e8507f93a1106fa3f7eaafc25308b Mon Sep 17 00:00:00 2001 From: Peter Hall Date: Fri, 6 Oct 2023 12:20:39 +0100 Subject: [PATCH 18/18] Minor doc clarification in Once::call_once --- library/std/src/sync/once.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/std/src/sync/once.rs b/library/std/src/sync/once.rs index 8c46080e43b38..2bb4f3f9e0383 100644 --- a/library/std/src/sync/once.rs +++ b/library/std/src/sync/once.rs @@ -125,7 +125,7 @@ impl Once { /// /// # Panics /// - /// The closure `f` will only be executed once if this is called + /// The closure `f` will only be executed once even if this is called /// concurrently amongst many threads. If that closure panics, however, then /// it will *poison* this [`Once`] instance, causing all future invocations of /// `call_once` to also panic.