sarugaku / resolvelib

Resolve abstract dependencies into concrete ones
ISC License
143 stars 33 forks source link

Avoid using exception for control flow in backtracking #41

Closed pfmoore closed 4 years ago

pfmoore commented 4 years ago

The current backtracking code uses a RequirementsConflicted exception to report back to the backtracker that removing one candidate didn't fix the issue. While it's not an unreasonable interpretation of the exception, the control flow is entirely local between _backtrack and excluded_of. As a result, it makes it harder to reason about the global uses of the RequirementsConflicted exception.

This PR changes the control flow to use a None return value rather than an exception.

This is (hopefully) the first of a series of PRs to clean up the resolver's exception handling, to make it easier for client code to provide useful diagnostics to the user when resolution fails.

uranusjr commented 4 years ago

Exceptions are more easy to make sense for me personally, and honestly I’m a little worried whether the locality would always hold. But this is an implementation detail anyway, and we can always change it back if bubbling via return value does not work well anymore. OTOH I’m interested to see where the clean ups would head.