astral-sh / ruff

An extremely fast Python linter and code formatter, written in Rust.
https://docs.astral.sh/ruff
MIT License
32.74k stars 1.09k forks source link

[red-knot] Double-store assertions when encountering invalid syntax #14313

Closed sharkdp closed 19 hours ago

sharkdp commented 21 hours ago

During type inference on some invalid syntax examples, we encounter situations where the exact same expression appears in multiple places inside the AST. The smallest example I could find is this:

for

When parsing it, we get the following AST. Note that the same Name(…) expression (same range) appears both as target and as iter field on StmtFor, once with Store context, once with Invalid context:

Module(
    ModModule {
        range: 0..3,
        body: [
            For(
                StmtFor {
                    range: 0..3,
                    is_async: false,
                    target: Name(
                        ExprName {
                            range: 3..3,
                            id: Name(""),
                            ctx: Store,
                        },
                    ),
                    iter: Name(
                        ExprName {
                            range: 3..3,
                            id: Name(""),
                            ctx: Invalid,
                        },
                    ),
                    body: [],
                    orelse: [],
                },
            ),
        ],
    },
)

This leads to a subsequent failing double-store assertion in store_expression_type because we infer types for both the target and the iter expressions:

https://github.com/astral-sh/ruff/blob/f789b12705048a4f75cddeceb843fd4afbe783f1/crates/red_knot_python_semantic/src/types/infer.rs#L2176-L2184

I'm not sure how to fix this.

Skipping storage when encountering expressions with Invalid context would work for this example, but there are other cases (:x=) where such an invalid expression is not "backed up" by a second valid expression.

Ignoring double-storage for Invalid expressions is also not an option, since in this example here, we run inference on iter first.

MichaReiser commented 21 hours ago

Hmm, this is interesting. We could consider using the ptr address instead of Range + Kind

sharkdp commented 20 hours ago

A similar example is

x if $z

Mentioning it here because it leads to a slightly different panic (Calling self.infer_expression on a standalone-expression is not allowed…). But the underlying issue is the same, I believe.