Rust: Unreachable code query

[geoffw0]

Unreachable code query for Rust.

@paldepind please would you review, particularly whether the logic in firstUnreachable and skipNode is reasonable and written as cleanly as possible right now; and whether there are obvious explanations for the false positive results in the test?

[github-actions]

QHelp previews:

rust/ql/src/queries/unusedentities/UnreachableCode.qhelp

Unreachable code

This rule finds code that is never reached. Unused code should be removed to increase readability and avoid confusion.

Recommendation

Remove any unreachable code.

Example

In the following example, the final return statement can never be reached:

fn fib(input: u32) -> u32 {
	if (input == 0) {
		return 0;
	} else if (input == 1) {
		return 1;
	} else {
		return fib(input - 1) + fib(input - 2);
	}

	return input; // BAD: this code is never reached
}

The problem can be fixed simply by removing the unreachable code:

fn fib(input: u32) -> u32 {
	if (input == 0) {
		return 0;
	} else if (input == 1) {
		return 1;
	} else {
		return fib(input - 1) + fib(input - 2);
	}
}

References

• Wikipedia: Unreachable code

[subatoi]

👍 LGTM

[aibaars]

This looks fine to me. However, it may be worth fixing some of the missing CFG steps instead of marking a lot of false results in the expected output. By the looks of it they are mainly caused by the lack of special handling of boolean literals. I updated the branch to pull in the latest CFG improvements, with a bit of luck some of the issues have already been fixed.

[aibaars]

@hvitved Could you have a quick look at this too?

[hvitved]

We also need to restrict this to nodes that can possibly be part of a CFG, for example ParenExprs cannot. I think the best heuristic is simply n instanceof ControlFlowTree.

[geoffw0]

n instanceof ControlFlowGraphImpl::ControlFlowTree doesn't work ... unless this fix requires more of your work-in-progress to apply?

[hvitved]

Right. Try with

  exists(ControlFlowGraphImpl::ControlFlowTree cft |
    cft.succ(n, _, _)
    or
    cft.succ(_, n, _)
  )

instead.

[geoffw0]

Yep, that works perfectly and I can see what the intent is now.

I've updated the PR.