mirror of
https://github.com/astral-sh/ruff.git
synced 2025-10-01 14:21:53 +00:00
[red-knot] Reachability analysis (#17199)
## Summary This implements a new approach to silencing `unresolved-reference` diagnostics by keeping track of the reachability of each use of a symbol. The changes merged in https://github.com/astral-sh/ruff/pull/17169 are still needed for the "Use of variable in nested function" test case, but that could also be solved in another way eventually (see https://github.com/astral-sh/ruff/issues/15777). We can use the same technique to silence `unresolved-import` and `unresolved-attribute` false-positives, but I think this could be merged in isolation. ## Test Plan New Markdown tests, ecosystem tests
This commit is contained in:
parent
cb7f56fb20
commit
60f2e67454
5 changed files with 382 additions and 44 deletions
|
@ -1502,13 +1502,14 @@ if True:
|
||||||
from module import symbol
|
from module import symbol
|
||||||
```
|
```
|
||||||
|
|
||||||
## Unsupported features
|
## Unreachable code
|
||||||
|
|
||||||
We do not support full unreachable code analysis yet. We also raise diagnostics from
|
A closely related feature is the ability to detect unreachable code. For example, we do not emit a
|
||||||
statically-known to be false branches:
|
diagnostic here:
|
||||||
|
|
||||||
```py
|
```py
|
||||||
if False:
|
if False:
|
||||||
# error: [unresolved-reference]
|
|
||||||
x
|
x
|
||||||
```
|
```
|
||||||
|
|
||||||
|
See [unreachable.md](unreachable.md) for more tests on this topic.
|
||||||
|
|
|
@ -1,9 +1,16 @@
|
||||||
# Unreachable code
|
# Unreachable code
|
||||||
|
|
||||||
|
This document describes our approach to handling unreachable code. There are two aspects to this.
|
||||||
|
One is to detect and mark blocks of code that are unreachable. This is useful for notifying the
|
||||||
|
user, as it can often be indicative of an error. The second aspect of this is to make sure that we
|
||||||
|
do not emit (incorrect) diagnostics in unreachable code.
|
||||||
|
|
||||||
## Detecting unreachable code
|
## Detecting unreachable code
|
||||||
|
|
||||||
In this section, we look at various scenarios how sections of code can become unreachable. We should
|
In this section, we look at various scenarios how sections of code can become unreachable. We should
|
||||||
eventually introduce a new diagnostic that would detect unreachable code.
|
eventually introduce a new diagnostic that would detect unreachable code. In an editor/LSP context,
|
||||||
|
there are ways to 'gray out' sections of code, which is helpful for blocks of code that are not
|
||||||
|
'dead' code, but inactive under certain conditions, like platform-specific code.
|
||||||
|
|
||||||
### Terminal statements
|
### Terminal statements
|
||||||
|
|
||||||
|
@ -85,7 +92,7 @@ def f():
|
||||||
print("unreachable")
|
print("unreachable")
|
||||||
```
|
```
|
||||||
|
|
||||||
## Python version and platform checks
|
### Python version and platform checks
|
||||||
|
|
||||||
It is common to have code that is specific to a certain Python version or platform. This case is
|
It is common to have code that is specific to a certain Python version or platform. This case is
|
||||||
special because whether or not the code is reachable depends on externally configured constants. And
|
special because whether or not the code is reachable depends on externally configured constants. And
|
||||||
|
@ -93,13 +100,13 @@ if we are checking for a set of parameters that makes one of these branches unre
|
||||||
likely not something that the user wants to be warned about, because there are probably other sets
|
likely not something that the user wants to be warned about, because there are probably other sets
|
||||||
of parameters that make the branch reachable.
|
of parameters that make the branch reachable.
|
||||||
|
|
||||||
### `sys.version_info` branches
|
#### `sys.version_info` branches
|
||||||
|
|
||||||
Consider the following example. If we check with a Python version lower than 3.11, the import
|
Consider the following example. If we check with a Python version lower than 3.11, the import
|
||||||
statement is unreachable. If we check with a Python version equal to or greater than 3.11, the
|
statement is unreachable. If we check with a Python version equal to or greater than 3.11, the
|
||||||
import statement is definitely reachable. We should not emit any diagnostics in either case.
|
import statement is definitely reachable. We should not emit any diagnostics in either case.
|
||||||
|
|
||||||
#### Checking with Python version 3.10
|
##### Checking with Python version 3.10
|
||||||
|
|
||||||
```toml
|
```toml
|
||||||
[environment]
|
[environment]
|
||||||
|
@ -115,7 +122,7 @@ if sys.version_info >= (3, 11):
|
||||||
from typing import Self
|
from typing import Self
|
||||||
```
|
```
|
||||||
|
|
||||||
#### Checking with Python version 3.12
|
##### Checking with Python version 3.12
|
||||||
|
|
||||||
```toml
|
```toml
|
||||||
[environment]
|
[environment]
|
||||||
|
@ -129,12 +136,12 @@ if sys.version_info >= (3, 11):
|
||||||
from typing import Self
|
from typing import Self
|
||||||
```
|
```
|
||||||
|
|
||||||
### `sys.platform` branches
|
#### `sys.platform` branches
|
||||||
|
|
||||||
The problem is even more pronounced with `sys.platform` branches, since we don't necessarily have
|
The problem is even more pronounced with `sys.platform` branches, since we don't necessarily have
|
||||||
the platform information available.
|
the platform information available.
|
||||||
|
|
||||||
#### Checking with platform `win32`
|
##### Checking with platform `win32`
|
||||||
|
|
||||||
```toml
|
```toml
|
||||||
[environment]
|
[environment]
|
||||||
|
@ -148,7 +155,7 @@ if sys.platform == "win32":
|
||||||
sys.getwindowsversion()
|
sys.getwindowsversion()
|
||||||
```
|
```
|
||||||
|
|
||||||
#### Checking with platform `linux`
|
##### Checking with platform `linux`
|
||||||
|
|
||||||
```toml
|
```toml
|
||||||
[environment]
|
[environment]
|
||||||
|
@ -164,7 +171,33 @@ if sys.platform == "win32":
|
||||||
sys.getwindowsversion()
|
sys.getwindowsversion()
|
||||||
```
|
```
|
||||||
|
|
||||||
#### Checking without a specified platform
|
##### Checking with platform set to `all`
|
||||||
|
|
||||||
|
```toml
|
||||||
|
[environment]
|
||||||
|
python-platform = "all"
|
||||||
|
```
|
||||||
|
|
||||||
|
If `python-platform` is set to `all`, we treat the platform as unspecified. This means that we do
|
||||||
|
not infer a literal type like `Literal["win32"]` for `sys.platform`, but instead fall back to
|
||||||
|
`LiteralString` (the `typeshed` annotation for `sys.platform`). This means that we can not
|
||||||
|
statically determine the truthiness of a branch like `sys.platform == "win32"`.
|
||||||
|
|
||||||
|
See <https://github.com/astral-sh/ruff/issues/16983#issuecomment-2777146188> for a plan on how this
|
||||||
|
could be improved.
|
||||||
|
|
||||||
|
```py
|
||||||
|
import sys
|
||||||
|
|
||||||
|
if sys.platform == "win32":
|
||||||
|
# TODO: we should not emit an error here
|
||||||
|
# error: [possibly-unbound-attribute]
|
||||||
|
sys.getwindowsversion()
|
||||||
|
```
|
||||||
|
|
||||||
|
##### Checking without a specified platform
|
||||||
|
|
||||||
|
If `python-platform` is not specified, we currently default to `all`:
|
||||||
|
|
||||||
```toml
|
```toml
|
||||||
[environment]
|
[environment]
|
||||||
|
@ -180,25 +213,29 @@ if sys.platform == "win32":
|
||||||
sys.getwindowsversion()
|
sys.getwindowsversion()
|
||||||
```
|
```
|
||||||
|
|
||||||
#### Checking with platform set to `all`
|
## No (incorrect) diagnostics in unreachable code
|
||||||
|
|
||||||
```toml
|
```toml
|
||||||
[environment]
|
[environment]
|
||||||
python-platform = "all"
|
python-version = "3.10"
|
||||||
```
|
```
|
||||||
|
|
||||||
```py
|
In this section, we demonstrate that we do not emit (incorrect) diagnostics in unreachable sections
|
||||||
import sys
|
of code.
|
||||||
|
|
||||||
if sys.platform == "win32":
|
It could be argued that no diagnostics at all should be emitted in unreachable code. The reasoning
|
||||||
# TODO: we should not emit an error here
|
is that any issues inside the unreachable section would not cause problems at runtime. And type
|
||||||
# error: [possibly-unbound-attribute]
|
checking the unreachable code under the assumption that it *is* reachable might lead to false
|
||||||
sys.getwindowsversion()
|
positives (see the "Global constants" example below).
|
||||||
```
|
|
||||||
|
|
||||||
## No false positive diagnostics in unreachable code
|
On the other hand, it could be argued that code like `1 + "a"` is incorrect, no matter if it is
|
||||||
|
reachable or not. Some developers like to use things like early `return` statements while debugging,
|
||||||
|
and for this use case, it is helpful to still see some diagnostics in unreachable sections.
|
||||||
|
|
||||||
In this section, we make sure that we do not emit false positive diagnostics in unreachable code.
|
We currently follow the second approach, but we do not attempt to provide the full set of
|
||||||
|
diagnostics in unreachable sections. In fact, we silence a certain category of diagnostics
|
||||||
|
(`unresolved-reference`, `unresolved-attribute`, …), in order to avoid *incorrect* diagnostics. In
|
||||||
|
the future, we may revisit this decision.
|
||||||
|
|
||||||
### Use of variables in unreachable code
|
### Use of variables in unreachable code
|
||||||
|
|
||||||
|
@ -218,26 +255,24 @@ def f():
|
||||||
|
|
||||||
In the example below, since we use `x` in the `inner` function, we use the "public" type of `x`,
|
In the example below, since we use `x` in the `inner` function, we use the "public" type of `x`,
|
||||||
which currently refers to the end-of-scope type of `x`. Since the end of the `outer` scope is
|
which currently refers to the end-of-scope type of `x`. Since the end of the `outer` scope is
|
||||||
unreachable, we treat `x` as if it was not defined. This behavior can certainly be improved.
|
unreachable, we need to make sure that we do not emit an `unresolved-reference` diagnostic:
|
||||||
|
|
||||||
```py
|
```py
|
||||||
def outer():
|
def outer():
|
||||||
x = 1
|
x = 1
|
||||||
|
|
||||||
def inner():
|
def inner():
|
||||||
return x # Name `x` used when not defined
|
reveal_type(x) # revealed: Unknown
|
||||||
while True:
|
while True:
|
||||||
pass
|
pass
|
||||||
```
|
```
|
||||||
|
|
||||||
## No diagnostics in unreachable code
|
### Global constants
|
||||||
|
|
||||||
In general, no diagnostics should be emitted in unreachable code. The reasoning is that any issues
|
|
||||||
inside the unreachable section would not cause problems at runtime. And type checking the
|
|
||||||
unreachable code under the assumption that it *is* reachable might lead to false positives:
|
|
||||||
|
|
||||||
```py
|
```py
|
||||||
FEATURE_X_ACTIVATED = False
|
from typing import Literal
|
||||||
|
|
||||||
|
FEATURE_X_ACTIVATED: Literal[False] = False
|
||||||
|
|
||||||
if FEATURE_X_ACTIVATED:
|
if FEATURE_X_ACTIVATED:
|
||||||
def feature_x():
|
def feature_x():
|
||||||
|
@ -248,7 +283,166 @@ def f():
|
||||||
# Type checking this particular section as if it were reachable would
|
# Type checking this particular section as if it were reachable would
|
||||||
# lead to a false positive, so we should not emit diagnostics here.
|
# lead to a false positive, so we should not emit diagnostics here.
|
||||||
|
|
||||||
# TODO: no error should be emitted here
|
|
||||||
# error: [unresolved-reference]
|
|
||||||
feature_x()
|
feature_x()
|
||||||
```
|
```
|
||||||
|
|
||||||
|
### Exhaustive check of syntactic constructs
|
||||||
|
|
||||||
|
We include some more examples here to make sure that silencing of diagnostics works for
|
||||||
|
syntactically different cases. To test this, we use `ExceptionGroup`, which is only available in
|
||||||
|
Python 3.11 and later. We have set the Python version to 3.10 for this whole section, to have
|
||||||
|
`match` statements available, but not `ExceptionGroup`.
|
||||||
|
|
||||||
|
To start, we make sure that we do not emit a diagnostic in this simple case:
|
||||||
|
|
||||||
|
```py
|
||||||
|
import sys
|
||||||
|
|
||||||
|
if sys.version_info >= (3, 11):
|
||||||
|
ExceptionGroup # no error here
|
||||||
|
```
|
||||||
|
|
||||||
|
Similarly, if we negate the logic, we also emit no error:
|
||||||
|
|
||||||
|
```py
|
||||||
|
if sys.version_info < (3, 11):
|
||||||
|
pass
|
||||||
|
else:
|
||||||
|
ExceptionGroup # no error here
|
||||||
|
```
|
||||||
|
|
||||||
|
This also works for more complex `if`-`elif`-`else` chains:
|
||||||
|
|
||||||
|
```py
|
||||||
|
if sys.version_info >= (3, 13):
|
||||||
|
ExceptionGroup # no error here
|
||||||
|
elif sys.version_info >= (3, 12):
|
||||||
|
ExceptionGroup # no error here
|
||||||
|
elif sys.version_info >= (3, 11):
|
||||||
|
ExceptionGroup # no error here
|
||||||
|
elif sys.version_info >= (3, 10):
|
||||||
|
pass
|
||||||
|
else:
|
||||||
|
pass
|
||||||
|
```
|
||||||
|
|
||||||
|
The same works for ternary expressions:
|
||||||
|
|
||||||
|
```py
|
||||||
|
class ExceptionGroupPolyfill: ...
|
||||||
|
|
||||||
|
MyExceptionGroup1 = ExceptionGroup if sys.version_info >= (3, 11) else ExceptionGroupPolyfill
|
||||||
|
MyExceptionGroup1 = ExceptionGroupPolyfill if sys.version_info < (3, 11) else ExceptionGroup
|
||||||
|
```
|
||||||
|
|
||||||
|
Due to short-circuiting, this also works for Boolean operators:
|
||||||
|
|
||||||
|
```py
|
||||||
|
sys.version_info >= (3, 11) and ExceptionGroup
|
||||||
|
sys.version_info < (3, 11) or ExceptionGroup
|
||||||
|
```
|
||||||
|
|
||||||
|
And in `match` statements:
|
||||||
|
|
||||||
|
```py
|
||||||
|
reveal_type(sys.version_info.minor) # revealed: Literal[10]
|
||||||
|
|
||||||
|
match sys.version_info.minor:
|
||||||
|
case 13:
|
||||||
|
ExceptionGroup
|
||||||
|
case 12:
|
||||||
|
ExceptionGroup
|
||||||
|
case 11:
|
||||||
|
ExceptionGroup
|
||||||
|
case _:
|
||||||
|
pass
|
||||||
|
```
|
||||||
|
|
||||||
|
Terminal statements can also lead to unreachable code:
|
||||||
|
|
||||||
|
```py
|
||||||
|
def f():
|
||||||
|
if sys.version_info < (3, 11):
|
||||||
|
raise RuntimeError("this code only works for Python 3.11+")
|
||||||
|
|
||||||
|
ExceptionGroup
|
||||||
|
```
|
||||||
|
|
||||||
|
Finally, not that anyone would ever use it, but it also works for `while` loops:
|
||||||
|
|
||||||
|
```py
|
||||||
|
while sys.version_info >= (3, 11):
|
||||||
|
ExceptionGroup
|
||||||
|
```
|
||||||
|
|
||||||
|
### Silencing errors for actually unknown symbols
|
||||||
|
|
||||||
|
We currently also silence diagnostics for symbols that are not actually defined anywhere. It is
|
||||||
|
conceivable that this could be improved, but is not a priority for now.
|
||||||
|
|
||||||
|
```py
|
||||||
|
if False:
|
||||||
|
does_not_exist
|
||||||
|
|
||||||
|
def f():
|
||||||
|
return
|
||||||
|
does_not_exist
|
||||||
|
```
|
||||||
|
|
||||||
|
### Attributes
|
||||||
|
|
||||||
|
When attribute expressions appear in unreachable code, we should not emit `unresolved-attribute`
|
||||||
|
diagnostics:
|
||||||
|
|
||||||
|
```py
|
||||||
|
import sys
|
||||||
|
import builtins
|
||||||
|
|
||||||
|
if sys.version_info >= (3, 11):
|
||||||
|
# TODO
|
||||||
|
# error: [unresolved-attribute]
|
||||||
|
builtins.ExceptionGroup
|
||||||
|
```
|
||||||
|
|
||||||
|
### Imports
|
||||||
|
|
||||||
|
When import statements appear in unreachable code, we should not emit `unresolved-import`
|
||||||
|
diagnostics:
|
||||||
|
|
||||||
|
```py
|
||||||
|
import sys
|
||||||
|
|
||||||
|
if sys.version_info >= (3, 11):
|
||||||
|
# TODO
|
||||||
|
# error: [unresolved-import]
|
||||||
|
from builtins import ExceptionGroup
|
||||||
|
|
||||||
|
# TODO
|
||||||
|
# error: [unresolved-import]
|
||||||
|
import builtins.ExceptionGroup
|
||||||
|
|
||||||
|
# See https://docs.python.org/3/whatsnew/3.11.html#new-modules
|
||||||
|
|
||||||
|
# TODO
|
||||||
|
# error: [unresolved-import]
|
||||||
|
import tomllib
|
||||||
|
|
||||||
|
# TODO
|
||||||
|
# error: [unresolved-import]
|
||||||
|
import wsgiref.types
|
||||||
|
```
|
||||||
|
|
||||||
|
### Emit diagnostics for definitely wrong code
|
||||||
|
|
||||||
|
Even though the expressions in the snippet below are unreachable, we still emit diagnostics for
|
||||||
|
them:
|
||||||
|
|
||||||
|
```py
|
||||||
|
if False:
|
||||||
|
1 + "a" # error: [unsupported-operator]
|
||||||
|
|
||||||
|
def f():
|
||||||
|
return
|
||||||
|
|
||||||
|
1 / 0 # error: [division-by-zero]
|
||||||
|
```
|
||||||
|
|
|
@ -546,6 +546,42 @@ impl<'db> SemanticIndexBuilder<'db> {
|
||||||
.simplify_visibility_constraints(snapshot);
|
.simplify_visibility_constraints(snapshot);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Record a constraint that affects the reachability of the current position in the semantic
|
||||||
|
/// index analysis. For example, if we encounter a `if test:` branch, we immediately record
|
||||||
|
/// a `test` constraint, because if `test` later (during type checking) evaluates to `False`,
|
||||||
|
/// we know that all statements that follow in this path of control flow will be unreachable.
|
||||||
|
fn record_reachability_constraint(
|
||||||
|
&mut self,
|
||||||
|
predicate: Predicate<'db>,
|
||||||
|
) -> ScopedVisibilityConstraintId {
|
||||||
|
let predicate_id = self.add_predicate(predicate);
|
||||||
|
self.record_reachability_constraint_id(predicate_id)
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Similar to [`Self::record_reachability_constraint`], but takes a [`ScopedPredicateId`].
|
||||||
|
fn record_reachability_constraint_id(
|
||||||
|
&mut self,
|
||||||
|
predicate_id: ScopedPredicateId,
|
||||||
|
) -> ScopedVisibilityConstraintId {
|
||||||
|
let visibility_constraint = self
|
||||||
|
.current_visibility_constraints_mut()
|
||||||
|
.add_atom(predicate_id);
|
||||||
|
self.current_use_def_map_mut()
|
||||||
|
.record_reachability_constraint(visibility_constraint)
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Record the negation of a given reachability/visibility constraint.
|
||||||
|
fn record_negated_reachability_constraint(
|
||||||
|
&mut self,
|
||||||
|
reachability_constraint: ScopedVisibilityConstraintId,
|
||||||
|
) {
|
||||||
|
let negated_constraint = self
|
||||||
|
.current_visibility_constraints_mut()
|
||||||
|
.add_not_constraint(reachability_constraint);
|
||||||
|
self.current_use_def_map_mut()
|
||||||
|
.record_reachability_constraint(negated_constraint);
|
||||||
|
}
|
||||||
|
|
||||||
fn push_assignment(&mut self, assignment: CurrentAssignment<'db>) {
|
fn push_assignment(&mut self, assignment: CurrentAssignment<'db>) {
|
||||||
self.current_assignments.push(assignment);
|
self.current_assignments.push(assignment);
|
||||||
}
|
}
|
||||||
|
@ -1252,6 +1288,8 @@ where
|
||||||
self.visit_expr(&node.test);
|
self.visit_expr(&node.test);
|
||||||
let mut no_branch_taken = self.flow_snapshot();
|
let mut no_branch_taken = self.flow_snapshot();
|
||||||
let mut last_predicate = self.record_expression_narrowing_constraint(&node.test);
|
let mut last_predicate = self.record_expression_narrowing_constraint(&node.test);
|
||||||
|
let mut reachability_constraint =
|
||||||
|
self.record_reachability_constraint(last_predicate);
|
||||||
self.visit_body(&node.body);
|
self.visit_body(&node.body);
|
||||||
|
|
||||||
let visibility_constraint_id = self.record_visibility_constraint(last_predicate);
|
let visibility_constraint_id = self.record_visibility_constraint(last_predicate);
|
||||||
|
@ -1281,11 +1319,14 @@ where
|
||||||
// taken
|
// taken
|
||||||
self.flow_restore(no_branch_taken.clone());
|
self.flow_restore(no_branch_taken.clone());
|
||||||
self.record_negated_narrowing_constraint(last_predicate);
|
self.record_negated_narrowing_constraint(last_predicate);
|
||||||
|
self.record_negated_reachability_constraint(reachability_constraint);
|
||||||
|
|
||||||
let elif_predicate = if let Some(elif_test) = clause_test {
|
let elif_predicate = if let Some(elif_test) = clause_test {
|
||||||
self.visit_expr(elif_test);
|
self.visit_expr(elif_test);
|
||||||
// A test expression is evaluated whether the branch is taken or not
|
// A test expression is evaluated whether the branch is taken or not
|
||||||
no_branch_taken = self.flow_snapshot();
|
no_branch_taken = self.flow_snapshot();
|
||||||
|
reachability_constraint =
|
||||||
|
self.record_reachability_constraint(last_predicate);
|
||||||
let predicate = self.record_expression_narrowing_constraint(elif_test);
|
let predicate = self.record_expression_narrowing_constraint(elif_test);
|
||||||
Some(predicate)
|
Some(predicate)
|
||||||
} else {
|
} else {
|
||||||
|
@ -1320,6 +1361,7 @@ where
|
||||||
|
|
||||||
let pre_loop = self.flow_snapshot();
|
let pre_loop = self.flow_snapshot();
|
||||||
let predicate = self.record_expression_narrowing_constraint(test);
|
let predicate = self.record_expression_narrowing_constraint(test);
|
||||||
|
self.record_reachability_constraint(predicate);
|
||||||
|
|
||||||
// We need multiple copies of the visibility constraint for the while condition,
|
// We need multiple copies of the visibility constraint for the while condition,
|
||||||
// since we need to model situations where the first evaluation of the condition
|
// since we need to model situations where the first evaluation of the condition
|
||||||
|
@ -1467,6 +1509,7 @@ where
|
||||||
&case.pattern,
|
&case.pattern,
|
||||||
case.guard.as_deref(),
|
case.guard.as_deref(),
|
||||||
);
|
);
|
||||||
|
self.record_reachability_constraint(predicate);
|
||||||
if let Some(expr) = &case.guard {
|
if let Some(expr) = &case.guard {
|
||||||
self.visit_expr(expr);
|
self.visit_expr(expr);
|
||||||
}
|
}
|
||||||
|
@ -1770,12 +1813,14 @@ where
|
||||||
self.visit_expr(test);
|
self.visit_expr(test);
|
||||||
let pre_if = self.flow_snapshot();
|
let pre_if = self.flow_snapshot();
|
||||||
let predicate = self.record_expression_narrowing_constraint(test);
|
let predicate = self.record_expression_narrowing_constraint(test);
|
||||||
|
let reachability_constraint = self.record_reachability_constraint(predicate);
|
||||||
self.visit_expr(body);
|
self.visit_expr(body);
|
||||||
let visibility_constraint = self.record_visibility_constraint(predicate);
|
let visibility_constraint = self.record_visibility_constraint(predicate);
|
||||||
let post_body = self.flow_snapshot();
|
let post_body = self.flow_snapshot();
|
||||||
self.flow_restore(pre_if.clone());
|
self.flow_restore(pre_if.clone());
|
||||||
|
|
||||||
self.record_negated_narrowing_constraint(predicate);
|
self.record_negated_narrowing_constraint(predicate);
|
||||||
|
self.record_negated_reachability_constraint(reachability_constraint);
|
||||||
self.visit_expr(orelse);
|
self.visit_expr(orelse);
|
||||||
self.record_negated_visibility_constraint(visibility_constraint);
|
self.record_negated_visibility_constraint(visibility_constraint);
|
||||||
self.flow_merge(post_body);
|
self.flow_merge(post_body);
|
||||||
|
@ -1848,7 +1893,7 @@ where
|
||||||
self.record_visibility_constraint_id(*vid);
|
self.record_visibility_constraint_id(*vid);
|
||||||
}
|
}
|
||||||
|
|
||||||
// For the last value, we don't need to model control flow. There is short-circuiting
|
// For the last value, we don't need to model control flow. There is no short-circuiting
|
||||||
// anymore.
|
// anymore.
|
||||||
if index < values.len() - 1 {
|
if index < values.len() - 1 {
|
||||||
let predicate = self.build_predicate(value);
|
let predicate = self.build_predicate(value);
|
||||||
|
@ -1877,6 +1922,7 @@ where
|
||||||
// has been evaluated, so we only push it onto the stack here.
|
// has been evaluated, so we only push it onto the stack here.
|
||||||
self.flow_restore(after_expr);
|
self.flow_restore(after_expr);
|
||||||
self.record_narrowing_constraint_id(predicate_id);
|
self.record_narrowing_constraint_id(predicate_id);
|
||||||
|
self.record_reachability_constraint_id(predicate_id);
|
||||||
visibility_constraints.push(visibility_constraint);
|
visibility_constraints.push(visibility_constraint);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
|
@ -297,6 +297,9 @@ pub(crate) struct UseDefMap<'db> {
|
||||||
/// [`SymbolBindings`] reaching a [`ScopedUseId`].
|
/// [`SymbolBindings`] reaching a [`ScopedUseId`].
|
||||||
bindings_by_use: IndexVec<ScopedUseId, SymbolBindings>,
|
bindings_by_use: IndexVec<ScopedUseId, SymbolBindings>,
|
||||||
|
|
||||||
|
/// Tracks whether or not a given use of a symbol is reachable from the start of the scope.
|
||||||
|
reachability_by_use: IndexVec<ScopedUseId, ScopedVisibilityConstraintId>,
|
||||||
|
|
||||||
/// If the definition is a binding (only) -- `x = 1` for example -- then we need
|
/// If the definition is a binding (only) -- `x = 1` for example -- then we need
|
||||||
/// [`SymbolDeclarations`] to know whether this binding is permitted by the live declarations.
|
/// [`SymbolDeclarations`] to know whether this binding is permitted by the live declarations.
|
||||||
///
|
///
|
||||||
|
@ -345,6 +348,24 @@ impl<'db> UseDefMap<'db> {
|
||||||
self.bindings_iterator(&self.bindings_by_use[use_id])
|
self.bindings_iterator(&self.bindings_by_use[use_id])
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Returns true if a given 'use' of a symbol is reachable from the start of the scope.
|
||||||
|
/// For example, in the following code, use `2` is reachable, but `1` and `3` are not:
|
||||||
|
/// ```py
|
||||||
|
/// def f():
|
||||||
|
/// x = 1
|
||||||
|
/// if False:
|
||||||
|
/// x # 1
|
||||||
|
/// x # 2
|
||||||
|
/// return
|
||||||
|
/// x # 3
|
||||||
|
/// ```
|
||||||
|
pub(crate) fn is_symbol_use_reachable(&self, db: &dyn crate::Db, use_id: ScopedUseId) -> bool {
|
||||||
|
!self
|
||||||
|
.visibility_constraints
|
||||||
|
.evaluate(db, &self.predicates, self.reachability_by_use[use_id])
|
||||||
|
.is_always_false()
|
||||||
|
}
|
||||||
|
|
||||||
pub(crate) fn public_bindings(
|
pub(crate) fn public_bindings(
|
||||||
&self,
|
&self,
|
||||||
symbol: ScopedSymbolId,
|
symbol: ScopedSymbolId,
|
||||||
|
@ -533,6 +554,7 @@ impl std::iter::FusedIterator for DeclarationsIterator<'_, '_> {}
|
||||||
pub(super) struct FlowSnapshot {
|
pub(super) struct FlowSnapshot {
|
||||||
symbol_states: IndexVec<ScopedSymbolId, SymbolState>,
|
symbol_states: IndexVec<ScopedSymbolId, SymbolState>,
|
||||||
scope_start_visibility: ScopedVisibilityConstraintId,
|
scope_start_visibility: ScopedVisibilityConstraintId,
|
||||||
|
reachability: ScopedVisibilityConstraintId,
|
||||||
}
|
}
|
||||||
|
|
||||||
#[derive(Debug)]
|
#[derive(Debug)]
|
||||||
|
@ -550,14 +572,57 @@ pub(super) struct UseDefMapBuilder<'db> {
|
||||||
pub(super) visibility_constraints: VisibilityConstraintsBuilder,
|
pub(super) visibility_constraints: VisibilityConstraintsBuilder,
|
||||||
|
|
||||||
/// A constraint which describes the visibility of the unbound/undeclared state, i.e.
|
/// A constraint which describes the visibility of the unbound/undeclared state, i.e.
|
||||||
/// whether or not the start of the scope is visible. This is important for cases like
|
/// whether or not a use of a symbol at the current point in control flow would see
|
||||||
/// `if True: x = 1; use(x)` where we need to hide the implicit "x = unbound" binding
|
/// the fake `x = <unbound>` binding at the start of the scope. This is important for
|
||||||
/// in the "else" branch.
|
/// cases like the following, where we need to hide the implicit unbound binding in
|
||||||
|
/// the "else" branch:
|
||||||
|
/// ```py
|
||||||
|
/// # x = <unbound>
|
||||||
|
///
|
||||||
|
/// if True:
|
||||||
|
/// x = 1
|
||||||
|
///
|
||||||
|
/// use(x) # the `x = <unbound>` binding is not visible here
|
||||||
|
/// ```
|
||||||
pub(super) scope_start_visibility: ScopedVisibilityConstraintId,
|
pub(super) scope_start_visibility: ScopedVisibilityConstraintId,
|
||||||
|
|
||||||
/// Live bindings at each so-far-recorded use.
|
/// Live bindings at each so-far-recorded use.
|
||||||
bindings_by_use: IndexVec<ScopedUseId, SymbolBindings>,
|
bindings_by_use: IndexVec<ScopedUseId, SymbolBindings>,
|
||||||
|
|
||||||
|
/// Tracks whether or not the scope start is visible at the current point in control flow.
|
||||||
|
/// This is subtly different from `scope_start_visibility`, as we apply these constraints
|
||||||
|
/// at the beginnging of a branch. Visibility constraints, on the other hand, need to be
|
||||||
|
/// applied at the end of a branch, as we apply them retroactively to all live bindings:
|
||||||
|
/// ```py
|
||||||
|
/// y = 1
|
||||||
|
///
|
||||||
|
/// if test:
|
||||||
|
/// # we record a reachability constraint of [test] here,
|
||||||
|
/// # so that it can affect the use of `x`:
|
||||||
|
///
|
||||||
|
/// x # we store a reachability constraint of [test] for this use of `x`
|
||||||
|
///
|
||||||
|
/// y = 2
|
||||||
|
///
|
||||||
|
/// # we record a visibility constraint of [test] here, which retroactively affects
|
||||||
|
/// # the `y = 1` and the `y = 2` binding.
|
||||||
|
/// else:
|
||||||
|
/// # we record a reachability constraint of [~test] here.
|
||||||
|
///
|
||||||
|
/// pass
|
||||||
|
///
|
||||||
|
/// # we record a visibility constraint of [~test] here, which retroactively affects
|
||||||
|
/// # the `y = 1` binding.
|
||||||
|
///
|
||||||
|
/// use(y)
|
||||||
|
/// ```
|
||||||
|
/// Depending on the value of `test`, the `y = 1`, `y = 2`, or both bindings may be visible.
|
||||||
|
/// The use of `x` is recorded with a reachability constraint of `[test]`.
|
||||||
|
reachability: ScopedVisibilityConstraintId,
|
||||||
|
|
||||||
|
/// Tracks whether or not a given use of a symbol is reachable from the start of the scope.
|
||||||
|
reachability_by_use: IndexVec<ScopedUseId, ScopedVisibilityConstraintId>,
|
||||||
|
|
||||||
/// Live declarations for each so-far-recorded binding.
|
/// Live declarations for each so-far-recorded binding.
|
||||||
declarations_by_binding: FxHashMap<Definition<'db>, SymbolDeclarations>,
|
declarations_by_binding: FxHashMap<Definition<'db>, SymbolDeclarations>,
|
||||||
|
|
||||||
|
@ -581,6 +646,8 @@ impl Default for UseDefMapBuilder<'_> {
|
||||||
visibility_constraints: VisibilityConstraintsBuilder::default(),
|
visibility_constraints: VisibilityConstraintsBuilder::default(),
|
||||||
scope_start_visibility: ScopedVisibilityConstraintId::ALWAYS_TRUE,
|
scope_start_visibility: ScopedVisibilityConstraintId::ALWAYS_TRUE,
|
||||||
bindings_by_use: IndexVec::new(),
|
bindings_by_use: IndexVec::new(),
|
||||||
|
reachability: ScopedVisibilityConstraintId::ALWAYS_TRUE,
|
||||||
|
reachability_by_use: IndexVec::new(),
|
||||||
declarations_by_binding: FxHashMap::default(),
|
declarations_by_binding: FxHashMap::default(),
|
||||||
bindings_by_declaration: FxHashMap::default(),
|
bindings_by_declaration: FxHashMap::default(),
|
||||||
symbol_states: IndexVec::new(),
|
symbol_states: IndexVec::new(),
|
||||||
|
@ -592,6 +659,7 @@ impl Default for UseDefMapBuilder<'_> {
|
||||||
impl<'db> UseDefMapBuilder<'db> {
|
impl<'db> UseDefMapBuilder<'db> {
|
||||||
pub(super) fn mark_unreachable(&mut self) {
|
pub(super) fn mark_unreachable(&mut self) {
|
||||||
self.record_visibility_constraint(ScopedVisibilityConstraintId::ALWAYS_FALSE);
|
self.record_visibility_constraint(ScopedVisibilityConstraintId::ALWAYS_FALSE);
|
||||||
|
self.reachability = ScopedVisibilityConstraintId::ALWAYS_FALSE;
|
||||||
}
|
}
|
||||||
|
|
||||||
pub(super) fn add_symbol(&mut self, symbol: ScopedSymbolId) {
|
pub(super) fn add_symbol(&mut self, symbol: ScopedSymbolId) {
|
||||||
|
@ -671,6 +739,16 @@ impl<'db> UseDefMapBuilder<'db> {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
pub(super) fn record_reachability_constraint(
|
||||||
|
&mut self,
|
||||||
|
constraint: ScopedVisibilityConstraintId,
|
||||||
|
) -> ScopedVisibilityConstraintId {
|
||||||
|
self.reachability = self
|
||||||
|
.visibility_constraints
|
||||||
|
.add_and_constraint(self.reachability, constraint);
|
||||||
|
self.reachability
|
||||||
|
}
|
||||||
|
|
||||||
pub(super) fn record_declaration(
|
pub(super) fn record_declaration(
|
||||||
&mut self,
|
&mut self,
|
||||||
symbol: ScopedSymbolId,
|
symbol: ScopedSymbolId,
|
||||||
|
@ -703,6 +781,9 @@ impl<'db> UseDefMapBuilder<'db> {
|
||||||
.bindings_by_use
|
.bindings_by_use
|
||||||
.push(self.symbol_states[symbol].bindings().clone());
|
.push(self.symbol_states[symbol].bindings().clone());
|
||||||
debug_assert_eq!(use_id, new_use);
|
debug_assert_eq!(use_id, new_use);
|
||||||
|
|
||||||
|
let new_use = self.reachability_by_use.push(self.reachability);
|
||||||
|
debug_assert_eq!(use_id, new_use);
|
||||||
}
|
}
|
||||||
|
|
||||||
pub(super) fn snapshot_eager_bindings(
|
pub(super) fn snapshot_eager_bindings(
|
||||||
|
@ -718,6 +799,7 @@ impl<'db> UseDefMapBuilder<'db> {
|
||||||
FlowSnapshot {
|
FlowSnapshot {
|
||||||
symbol_states: self.symbol_states.clone(),
|
symbol_states: self.symbol_states.clone(),
|
||||||
scope_start_visibility: self.scope_start_visibility,
|
scope_start_visibility: self.scope_start_visibility,
|
||||||
|
reachability: self.reachability,
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -732,6 +814,7 @@ impl<'db> UseDefMapBuilder<'db> {
|
||||||
// Restore the current visible-definitions state to the given snapshot.
|
// Restore the current visible-definitions state to the given snapshot.
|
||||||
self.symbol_states = snapshot.symbol_states;
|
self.symbol_states = snapshot.symbol_states;
|
||||||
self.scope_start_visibility = snapshot.scope_start_visibility;
|
self.scope_start_visibility = snapshot.scope_start_visibility;
|
||||||
|
self.reachability = snapshot.reachability;
|
||||||
|
|
||||||
// If the snapshot we are restoring is missing some symbols we've recorded since, we need
|
// If the snapshot we are restoring is missing some symbols we've recorded since, we need
|
||||||
// to fill them in so the symbol IDs continue to line up. Since they don't exist in the
|
// to fill them in so the symbol IDs continue to line up. Since they don't exist in the
|
||||||
|
@ -787,12 +870,17 @@ impl<'db> UseDefMapBuilder<'db> {
|
||||||
self.scope_start_visibility = self
|
self.scope_start_visibility = self
|
||||||
.visibility_constraints
|
.visibility_constraints
|
||||||
.add_or_constraint(self.scope_start_visibility, snapshot.scope_start_visibility);
|
.add_or_constraint(self.scope_start_visibility, snapshot.scope_start_visibility);
|
||||||
|
|
||||||
|
self.reachability = self
|
||||||
|
.visibility_constraints
|
||||||
|
.add_or_constraint(self.reachability, snapshot.reachability);
|
||||||
}
|
}
|
||||||
|
|
||||||
pub(super) fn finish(mut self) -> UseDefMap<'db> {
|
pub(super) fn finish(mut self) -> UseDefMap<'db> {
|
||||||
self.all_definitions.shrink_to_fit();
|
self.all_definitions.shrink_to_fit();
|
||||||
self.symbol_states.shrink_to_fit();
|
self.symbol_states.shrink_to_fit();
|
||||||
self.bindings_by_use.shrink_to_fit();
|
self.bindings_by_use.shrink_to_fit();
|
||||||
|
self.reachability_by_use.shrink_to_fit();
|
||||||
self.declarations_by_binding.shrink_to_fit();
|
self.declarations_by_binding.shrink_to_fit();
|
||||||
self.bindings_by_declaration.shrink_to_fit();
|
self.bindings_by_declaration.shrink_to_fit();
|
||||||
self.eager_bindings.shrink_to_fit();
|
self.eager_bindings.shrink_to_fit();
|
||||||
|
@ -803,6 +891,7 @@ impl<'db> UseDefMapBuilder<'db> {
|
||||||
narrowing_constraints: self.narrowing_constraints.build(),
|
narrowing_constraints: self.narrowing_constraints.build(),
|
||||||
visibility_constraints: self.visibility_constraints.build(),
|
visibility_constraints: self.visibility_constraints.build(),
|
||||||
bindings_by_use: self.bindings_by_use,
|
bindings_by_use: self.bindings_by_use,
|
||||||
|
reachability_by_use: self.reachability_by_use,
|
||||||
public_symbols: self.symbol_states,
|
public_symbols: self.symbol_states,
|
||||||
declarations_by_binding: self.declarations_by_binding,
|
declarations_by_binding: self.declarations_by_binding,
|
||||||
bindings_by_declaration: self.bindings_by_declaration,
|
bindings_by_declaration: self.bindings_by_declaration,
|
||||||
|
|
|
@ -4173,8 +4173,8 @@ impl<'db> TypeInferenceBuilder<'db> {
|
||||||
let use_def = self.index.use_def_map(file_scope_id);
|
let use_def = self.index.use_def_map(file_scope_id);
|
||||||
|
|
||||||
// If we're inferring types of deferred expressions, always treat them as public symbols
|
// If we're inferring types of deferred expressions, always treat them as public symbols
|
||||||
let local_scope_symbol = if self.is_deferred() {
|
let (local_scope_symbol, report_unresolved_usage) = if self.is_deferred() {
|
||||||
if let Some(symbol_id) = symbol_table.symbol_id_by_name(symbol_name) {
|
let symbol = if let Some(symbol_id) = symbol_table.symbol_id_by_name(symbol_name) {
|
||||||
symbol_from_bindings(db, use_def.public_bindings(symbol_id))
|
symbol_from_bindings(db, use_def.public_bindings(symbol_id))
|
||||||
} else {
|
} else {
|
||||||
assert!(
|
assert!(
|
||||||
|
@ -4182,10 +4182,14 @@ impl<'db> TypeInferenceBuilder<'db> {
|
||||||
"Expected the symbol table to create a symbol for every Name node"
|
"Expected the symbol table to create a symbol for every Name node"
|
||||||
);
|
);
|
||||||
Symbol::Unbound
|
Symbol::Unbound
|
||||||
}
|
};
|
||||||
|
|
||||||
|
(symbol, true)
|
||||||
} else {
|
} else {
|
||||||
let use_id = name_node.scoped_use_id(db, scope);
|
let use_id = name_node.scoped_use_id(db, scope);
|
||||||
symbol_from_bindings(db, use_def.bindings_at_use(use_id))
|
let symbol = symbol_from_bindings(db, use_def.bindings_at_use(use_id));
|
||||||
|
let report_unresolved_usage = use_def.is_symbol_use_reachable(db, use_id);
|
||||||
|
(symbol, report_unresolved_usage)
|
||||||
};
|
};
|
||||||
|
|
||||||
let symbol = SymbolAndQualifiers::from(local_scope_symbol).or_fall_back_to(db, || {
|
let symbol = SymbolAndQualifiers::from(local_scope_symbol).or_fall_back_to(db, || {
|
||||||
|
@ -4335,11 +4339,15 @@ impl<'db> TypeInferenceBuilder<'db> {
|
||||||
symbol
|
symbol
|
||||||
.unwrap_with_diagnostic(|lookup_error| match lookup_error {
|
.unwrap_with_diagnostic(|lookup_error| match lookup_error {
|
||||||
LookupError::Unbound(qualifiers) => {
|
LookupError::Unbound(qualifiers) => {
|
||||||
report_unresolved_reference(&self.context, name_node);
|
if report_unresolved_usage {
|
||||||
|
report_unresolved_reference(&self.context, name_node);
|
||||||
|
}
|
||||||
TypeAndQualifiers::new(Type::unknown(), qualifiers)
|
TypeAndQualifiers::new(Type::unknown(), qualifiers)
|
||||||
}
|
}
|
||||||
LookupError::PossiblyUnbound(type_when_bound) => {
|
LookupError::PossiblyUnbound(type_when_bound) => {
|
||||||
report_possibly_unresolved_reference(&self.context, name_node);
|
if report_unresolved_usage {
|
||||||
|
report_possibly_unresolved_reference(&self.context, name_node);
|
||||||
|
}
|
||||||
type_when_bound
|
type_when_bound
|
||||||
}
|
}
|
||||||
})
|
})
|
||||||
|
|
Loading…
Add table
Add a link
Reference in a new issue