Rollup merge of #136959 - nnethercote:simplify-SwitchSources, r=tmiasko

Simplify switch sources

`SwitchSources` and the code around it can be simplified.

r? `@tmiasko`

compiler/rustc_middle/src/mir/basic_blocks.rs

@@ -20,7 +20,22 @@ pub struct BasicBlocks<'tcx> {
 // Typically 95%+ of basic blocks have 4 or fewer predecessors.
 type Predecessors = IndexVec<BasicBlock, SmallVec<[BasicBlock; 4]>>;
 
-type SwitchSources = FxHashMap<(BasicBlock, BasicBlock), SmallVec<[Option<u128>; 1]>>;
+/// Each `(target, switch)` entry in the map contains a list of switch values
+/// that lead to a `target` block from a `switch` block.
+///
+/// Note: this type is currently never instantiated, because it's only used for
+/// `BasicBlocks::switch_sources`, which is only called by backwards analyses
+/// that do `SwitchInt` handling, and we don't have any of those, not even in
+/// tests. See #95120 and #94576.
+type SwitchSources = FxHashMap<(BasicBlock, BasicBlock), SmallVec<[SwitchTargetValue; 1]>>;
+
+#[derive(Debug, Clone, Copy)]
+pub enum SwitchTargetValue {
+    // A normal switch value.
+    Normal(u128),
+    // The final "otherwise" fallback value.
+    Otherwise,
+}
 
 #[derive(Clone, Default, Debug)]
 struct Cache {
@@ -70,8 +85,8 @@ impl<'tcx> BasicBlocks<'tcx> {
         })
     }
 
-    /// `switch_sources()[&(target, switch)]` returns a list of switch
-    /// values that lead to a `target` block from a `switch` block.
+    /// Returns info about switch values that lead from one block to another
+    /// block. See `SwitchSources`.
     #[inline]
     pub fn switch_sources(&self) -> &SwitchSources {
         self.cache.switch_sources.get_or_init(|| {
@@ -82,9 +97,15 @@ impl<'tcx> BasicBlocks<'tcx> {
                 }) = &data.terminator
                 {
                     for (value, target) in targets.iter() {
-                        switch_sources.entry((target, bb)).or_default().push(Some(value));
+                        switch_sources
+                            .entry((target, bb))
+                            .or_default()
+                            .push(SwitchTargetValue::Normal(value));
                     }
-                    switch_sources.entry((targets.otherwise(), bb)).or_default().push(None);
+                    switch_sources
+                        .entry((targets.otherwise(), bb))
+                        .or_default()
+                        .push(SwitchTargetValue::Otherwise);
                 }
             }
             switch_sources

compiler/rustc_middle/src/mir/mod.rs

@@ -7,7 +7,7 @@ use std::fmt::{self, Debug, Formatter};
 use std::ops::{Index, IndexMut};
 use std::{iter, mem};
 
-pub use basic_blocks::BasicBlocks;
+pub use basic_blocks::{BasicBlocks, SwitchTargetValue};
 use either::Either;
 use polonius_engine::Atom;
 use rustc_abi::{FieldIdx, VariantIdx};

compiler/rustc_middle/src/mir/syntax.rs

@@ -1015,22 +1015,30 @@ impl TerminatorKind<'_> {
 
 #[derive(Debug, Clone, TyEncodable, TyDecodable, Hash, HashStable, PartialEq)]
 pub struct SwitchTargets {
-    /// Possible values. The locations to branch to in each case
-    /// are found in the corresponding indices from the `targets` vector.
+    /// Possible values. For each value, the location to branch to is found in
+    /// the corresponding element in the `targets` vector.
     pub(super) values: SmallVec<[Pu128; 1]>,
 
-    /// Possible branch sites. The last element of this vector is used
-    /// for the otherwise branch, so targets.len() == values.len() + 1
-    /// should hold.
+    /// Possible branch targets. The last element of this vector is used for
+    /// the "otherwise" branch, so `targets.len() == values.len() + 1` always
+    /// holds.
     //
-    // This invariant is quite non-obvious and also could be improved.
-    // One way to make this invariant is to have something like this instead:
+    // Note: This invariant is non-obvious and easy to violate. This would be a
+    // more rigorous representation:
     //
-    // branches: Vec<(ConstInt, BasicBlock)>,
-    // otherwise: Option<BasicBlock> // exhaustive if None
+    //   normal: SmallVec<[(Pu128, BasicBlock); 1]>,
+    //   otherwise: BasicBlock,
     //
-    // However we’ve decided to keep this as-is until we figure a case
-    // where some other approach seems to be strictly better than other.
+    // But it's important to have the targets in a sliceable type, because
+    // target slices show up elsewhere. E.g. `TerminatorKind::InlineAsm` has a
+    // boxed slice, and `TerminatorKind::FalseEdge` has a single target that
+    // can be converted to a slice with `slice::from_ref`.
+    //
+    // Why does this matter? In functions like `TerminatorKind::successors` we
+    // return `impl Iterator` and a non-slice-of-targets representation here
+    // causes problems because multiple different concrete iterator types would
+    // be involved and we would need a boxed trait object, which requires an
+    // allocation, which is expensive if done frequently.
     pub(super) targets: SmallVec<[BasicBlock; 2]>,
 }