transmutability: remove NFA intermediate representation #139990
Conversation
rustbot
added
S-waiting-on-review
jswrenn
force-pushed
the
no-nfas
branch
2 times, most recently
from
0548619 to
0fec408
Compare
Prior to this commit, the transmutability analysis used an intermediate NFA representation of type layout. We then determinized this representation into a DFA, upon which we ran the core transmutability analysis. Unfortunately, determinizing NFAs is expensive. In this commit, we avoid NFAs entirely by observing that Rust `union`s are the only source of nondeterminism and that it is comparatively cheap to compute the DFA union of DFAs. We also implement Graphviz DOT debug formatting of DFAs. Fixes rust-lang/project-safe-transmute#23 Fixes rust-lang/project-safe-transmute#24
| @@ -22,12 +22,12 @@ fn validity() { | |||
| } | |||
|
|
|||
| fn padding() { | |||
| #[repr(align(8))] | |||
| struct Align8; | |||
| #[repr(align(4))] | |||
There was a problem hiding this comment.
This temporary downsizing is tracked here: rust-lang/project-safe-transmute#25
There was a problem hiding this comment.
Could you double check that those test changes are necessary after resolving other comments?
| #[derive(PartialEq, Clone)] | ||
| pub(crate) struct Dfa<R> |
There was a problem hiding this comment.
It seems suspect to have a derived implementation of Clone, since it doesn't preserve the invariant that the states of distinct automatons are disjoint. This is orthogonal to the changes here, so feel free to ignore.
| /// The states in a `Nfa` represent byte offsets. | ||
| #[derive(Hash, Eq, PartialEq, PartialOrd, Ord, Copy, Clone)] | ||
| pub(crate) struct State(u32); | ||
| pub(crate) struct State(pub(crate) u32); |
There was a problem hiding this comment.
Outdated reference to Nfa.
|
|
||
| transitions.entry(src).or_default().byte_transitions.insert(*byte_transition, dst); | ||
|
|
||
| queue.push((a_dst, b_dst)) |
There was a problem hiding this comment.
Make sure that each state is enqueued and processed only once. While it is irrelevant for correctness, it duplicates the work performed otherwise. For example, consider a union between two chain automatons with complete byte edges, the first state pair is enqueued once, the second state pair is enqueued 256 times, the third state pair 256^2 times, and so on.
| Tree::Alt(alts) => { | ||
| // Convert and filter the inhabited alternatives. | ||
| let mut alts = alts.into_iter().map(Self::from_tree).filter_map(Result::ok); | ||
| // If there is not at least one alternative, return `Uninhabited`. |
There was a problem hiding this comment.
Perhaps: "If there are no alternatives, ..."
| Self { transitions, start, accepting } | ||
| } | ||
|
|
||
| /// Compute the union of two `Nfa`s. |
There was a problem hiding this comment.
Outdated reference to Nfa.
| @@ -109,7 +109,7 @@ where | |||
| // representation. If the conversion fails because `src` is uninhabited, | |||
There was a problem hiding this comment.
One line above: outdated reference to NFA.
| @@ -120,7 +120,7 @@ where | |||
| // the `dst` type do not exist, either because it's genuinely | |||
There was a problem hiding this comment.
Three lines above: outdated reference to NFA.
| @@ -22,12 +22,12 @@ fn validity() { | |||
| } | |||
|
|
|||
| fn padding() { | |||
| #[repr(align(8))] | |||
| struct Align8; | |||
| #[repr(align(4))] | |||
There was a problem hiding this comment.
Could you double check that those test changes are necessary after resolving other comments?
Prior to this commit, the transmutability analysis used an intermediate NFA representation of type layout. We then determinized this representation into a DFA, upon which we ran the core transmutability analysis. Unfortunately, determinizing NFAs is expensive. In this commit, we avoid NFAs entirely by observing that Rust
unions are the only source of nondeterminism and that it is comparatively cheap to compute the DFA union of DFAs.We also implement Graphviz DOT debug formatting of DFAs.
Fixes rust-lang/project-safe-transmute#23
Fixes rust-lang/project-safe-transmute#24
r? @compiler-errors