Skip to content

Commit 4975c2f

Browse files
authored
[red-knot] Fix panic on cyclic * imports (astral-sh#16958)
## Summary Further work towards astral-sh#14169. We currently panic on encountering cyclic `*` imports. This is easily fixed using fixpoint iteration. ## Test Plan Added a test that panics on `main`, but passes with this PR
1 parent dd5b02a commit 4975c2f

File tree

2 files changed

+47
-1
lines changed

2 files changed

+47
-1
lines changed

crates/red_knot_python_semantic/resources/mdtest/import/star.md

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -834,6 +834,35 @@ reveal_type(g) # revealed: Unknown
834834
reveal_type(h) # revealed: Unknown
835835
```
836836

837+
## Cyclic star imports
838+
839+
Believe it or not, this code does _not_ raise an exception at runtime!
840+
841+
`a.py`:
842+
843+
```py
844+
from b import *
845+
846+
A: bool = True
847+
```
848+
849+
`b.py`:
850+
851+
```py
852+
from a import *
853+
854+
B: bool = True
855+
```
856+
857+
`c.py`:
858+
859+
```py
860+
from a import *
861+
862+
reveal_type(A) # revealed: bool
863+
reveal_type(B) # revealed: bool
864+
```
865+
837866
## Integration test: `collections.abc`
838867

839868
The `collections.abc` standard-library module provides a good integration test, as all its symbols

crates/red_knot_python_semantic/src/semantic_index/re_exports.rs

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,10 @@
1515
//! separate query, we would need to complete semantic indexing on `bar` in order to
1616
//! complete analysis of the global namespace of `foo`. Since semantic indexing is somewhat
1717
//! expensive, this would be undesirable. A separate query allows us to avoid this issue.
18+
//!
19+
//! An additional concern is that the recursive nature of this query means that it must be able
20+
//! to handle cycles. We do this using fixpoint iteration; adding fixpoint iteration to the
21+
//! whole [`super::semantic_index()`] query would probably be prohibitively expensive.
1822
1923
use ruff_db::{files::File, parsed::parsed_module};
2024
use ruff_python_ast::{
@@ -26,7 +30,20 @@ use rustc_hash::FxHashSet;
2630

2731
use crate::{module_name::ModuleName, resolve_module, Db};
2832

29-
#[salsa::tracked(return_ref)]
33+
fn exports_cycle_recover(
34+
_db: &dyn Db,
35+
_value: &FxHashSet<Name>,
36+
_count: u32,
37+
_file: File,
38+
) -> salsa::CycleRecoveryAction<FxHashSet<Name>> {
39+
salsa::CycleRecoveryAction::Iterate
40+
}
41+
42+
fn exports_cycle_initial(_db: &dyn Db, _file: File) -> FxHashSet<Name> {
43+
FxHashSet::default()
44+
}
45+
46+
#[salsa::tracked(return_ref, cycle_fn=exports_cycle_recover, cycle_initial=exports_cycle_initial)]
3047
pub(super) fn exported_names(db: &dyn Db, file: File) -> FxHashSet<Name> {
3148
let module = parsed_module(db.upcast(), file);
3249
let mut finder = ExportFinder::new(db, file);

0 commit comments

Comments
 (0)