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

Start node has no immediate dominator #111547

Merged
merged 1 commit into from
May 15, 2023

Conversation

tmiasko
Copy link
Contributor

@tmiasko tmiasko commented May 13, 2023

Change the immediate_dominator return type to Option, and use None to
indicate that node has no immediate dominator.

Also fix the issue where the start node would be returned as its own
immediate dominator.

@rustbot
Copy link
Collaborator

rustbot commented May 13, 2023

r? @compiler-errors

(rustbot has picked a reviewer for you, use r? to override)

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels May 13, 2023
@cjgillot
Copy link
Contributor

Would it be more consistent to make the immediate_dominators vec hold None on the entry node, and adapt is_reachable?

@tmiasko
Copy link
Contributor Author

tmiasko commented May 14, 2023

Right now there is no other information that would describe which nodes are reachable, nor which node is the start node (the code is generic and works with arbitrary control flow graphs, not just MIR basic blocks). Of course, I can add information about the start node, if you prefer that approach.

@cjgillot
Copy link
Contributor

Yes, that would be more natural. The algorithm in this file is complicated enough to avoid adding corner cases.
Reachability information is not very important in that file, as we typically use traversal::reachable_as_bitset for MIR.

@cjgillot cjgillot self-assigned this May 14, 2023
Change the immediate_dominator return type to Option, and use None to
indicate that node has no immediate dominator.

Also fix the issue where the start node would be returned as its own
immediate dominator.
@compiler-errors compiler-errors removed their assignment May 14, 2023
@cjgillot
Copy link
Contributor

Thanks!
@bors r+

@bors
Copy link
Contributor

bors commented May 14, 2023

📌 Commit f16d2b1 has been approved by cjgillot

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 14, 2023
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request May 15, 2023
…illot

Start node has no immediate dominator

Change the immediate_dominator return type to Option, and use None to
indicate that node has no immediate dominator.

Also fix the issue where the start node would be returned as its own
immediate dominator.
bors added a commit to rust-lang-ci/rust that referenced this pull request May 15, 2023
…iaskrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#102673 (Update doc for `PhantomData` to match code example)
 - rust-lang#111531 (Fix ice caused by shorthand fields in NoFieldsForFnCall)
 - rust-lang#111547 (Start node has no immediate dominator)
 - rust-lang#111548 (add util function to TokenStream to eliminate some clones)
 - rust-lang#111560 (Simplify find_width_of_character_at_span.)
 - rust-lang#111569 (Appease lints)
 - rust-lang#111581 (Fix some misleading and copy-pasted `Pattern` examples)
 - rust-lang#111582 ((docs) Change "wanting" to "want")

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit be8718a into rust-lang:master May 15, 2023
@rustbot rustbot added this to the 1.71.0 milestone May 15, 2023
@tmiasko tmiasko deleted the immediate-dominator branch May 15, 2023 15:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants