Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rust: Implement ConditionalCompletionSplitting #17657

Merged
merged 2 commits into from
Oct 8, 2024

Conversation

hvitved
Copy link
Contributor

@hvitved hvitved commented Oct 3, 2024

This PR implements ConditionalCompletionSplitting, similar to how its done for other languages (C#, Ruby, and Swift). The idea is to record information about the Boolean value of sub expressions in order to (a) have all expressions be post-order in the CFG, and (b) provide more accurate information for a future guards library.

Examples

Logical negation

Before this PR, logical negation expressions were in fact post-order, but the dominance information was lost. For example, this program

    fn test_if_not_operator(a: bool) -> bool {
        if !a {
            true
        } else {
            false
        }
    }

has the following CFGs before and after

Before
flowchart TD
1["enter test_if_not_operator"]
10["BlockExpr"]
11["false"]
2["exit test_if_not_operator"]
3["exit test_if_not_operator (normal)"]
4["BlockExpr"]
5["IfExpr"]
6["! ..."]
7["a"]
8["BlockExpr"]
9["true"]

1 --> 7
3 --> 2
4 --> 3
5 --> 4
6 -- false --> 11
6 -- true --> 9
7 -- false, true --> 6
8 --> 5
9 --> 8
10 --> 5
11 --> 10
Loading
After
flowchart TD
1["enter test_if_not_operator"]
10["true"]
11["BlockExpr"]
12["false"]
2["exit test_if_not_operator"]
3["exit test_if_not_operator (normal)"]
4["BlockExpr"]
5["IfExpr"]
6["[boolean(false)] ! ..."]
7["[boolean(true)] ! ..."]
8["a"]
9["BlockExpr"]

1 --> 8
3 --> 2
4 --> 3
5 --> 4
6 -- false --> 12
7 -- true --> 10
8 -- false --> 7
8 -- true --> 6
9 --> 5
10 --> 9
11 --> 5
12 --> 11
Loading

Logical conjunction

    fn test_and_operator(a: bool, b: bool, c: bool) -> bool {
        let d = a && b && c;
        d
    }
Before
flowchart TD
1["enter test_and_operator"]
10["b"]
11["c"]
12["d"]
2["exit test_and_operator"]
3["exit test_and_operator (normal)"]
4["BlockExpr"]
5["LetStmt"]
6["d"]
7["a"]
8["... && ..."]
9["... && ..."]

1 --> 5
3 --> 2
4 --> 3
5 --> 9
6 -- match, no-match --> 12
7 -- false --> 6
7 -- true --> 10
8 --> 7
9 --> 8
10 -- false --> 6
10 -- true --> 11
11 --> 6
12 --> 4
Loading
After
flowchart TD
1["enter test_and_operator"]
10["... && ..."]
11["b"]
12["c"]
13["d"]
2["exit test_and_operator"]
3["exit test_and_operator (normal)"]
4["BlockExpr"]
5["LetStmt"]
6["d"]
7["a"]
8["[boolean(false)] ... && ..."]
9["[boolean(true)] ... && ..."]

1 --> 5
3 --> 2
4 --> 3
5 --> 7
6 -- match, no-match --> 13
7 -- false --> 8
7 -- true --> 11
8 -- false --> 10
9 -- true --> 12
10 --> 6
11 -- false --> 8
11 -- true --> 9
12 --> 10
13 --> 4
Loading

Complex nesting

    fn test_nested_if_match(a: i64) -> i64 {
        if (match a {
            0 => true,
            _ => false,
        }) {
            1
        } else {
            0
        }
    }

Note how in the after-CFG it is now evident that the 1 expression is guarded by a matching the 0 pattern (and dually that the 0 expression is guarded by a not matching the 0 pattern).

Before
flowchart TD
1["enter test_nested_if_match"]
10["WildcardPat"]
11["false"]
12["BlockExpr"]
13["1"]
14["BlockExpr"]
15["0"]
2["exit test_nested_if_match"]
3["exit test_nested_if_match (normal)"]
4["BlockExpr"]
5["IfExpr"]
6["MatchExpr"]
7["a"]
8["LiteralPat"]
9["true"]

1 --> 7
3 --> 2
4 --> 3
5 --> 4
6 -- false --> 15
6 -- true --> 13
7 --> 8
8 -- match --> 9
8 -- no-match --> 10
9 --> 6
10 -- match --> 11
11 --> 6
12 --> 5
13 --> 12
14 --> 5
15 --> 14
Loading
After
flowchart TD
1["enter test_nested_if_match"]
10["true"]
11["WildcardPat"]
12["false"]
13["BlockExpr"]
14["1"]
15["BlockExpr"]
16["0"]
2["exit test_nested_if_match"]
3["exit test_nested_if_match (normal)"]
4["BlockExpr"]
5["IfExpr"]
6["[boolean(false)] MatchExpr"]
7["[boolean(true)] MatchExpr"]
8["a"]
9["LiteralPat"]

1 --> 8
3 --> 2
4 --> 3
5 --> 4
6 -- false --> 16
7 -- true --> 14
8 --> 9
9 -- match --> 10
9 -- no-match --> 11
10 -- true --> 7
11 -- match --> 12
12 -- false --> 6
13 --> 5
14 --> 13
15 --> 5
16 --> 15
Loading

@github-actions github-actions bot added the Rust Pull requests that update Rust code label Oct 3, 2024
@hvitved hvitved force-pushed the rust/cfg-conditional-splitting branch from 8a790b1 to aa5e0c3 Compare October 3, 2024 19:26
@hvitved hvitved marked this pull request as ready for review October 4, 2024 10:24
Copy link
Contributor

@geoffw0 geoffw0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CFG changes LGTM. Splitting is new to me, but the graphs in your PR description make sense.

@hvitved hvitved requested a review from geoffw0 October 8, 2024 11:41
Copy link
Contributor

@geoffw0 geoffw0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My review hasn't been especially deep, but this has been open for 5 days and is clearly the direction we want to head. Approving.

@hvitved hvitved merged commit fcf1b6d into github:main Oct 8, 2024
14 checks passed
@hvitved hvitved deleted the rust/cfg-conditional-splitting branch October 8, 2024 14:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Rust Pull requests that update Rust code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants