From 449b84cb99203a1eaa735f49958b6ddabc8de779 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Fri, 6 Oct 2023 11:20:02 +1100 Subject: [PATCH] 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 c0141f1f841..ae193ef99e6 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() } }