Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Semantic: ScopeFlags for a class in a class constructor has ScopeFlags::Constructor #7900

Closed
overlookmotel opened this issue Dec 14, 2024 · 3 comments · Fixed by #7932 or #7950
Closed
Assignees
Labels
A-semantic Area - Semantic C-bug Category - Bug

Comments

@overlookmotel
Copy link
Contributor

Input:

class Outer {
  constructor() {
    class Inner {}
  }
}

Semantic sets ScopeFlags for class Inner {} as StrictMode | Constructor. I believe this is a mistake. It should just be StrictMode.

Weirdly I cannot replicate this in playground (playground). But on latest main, when I print ScopeFlags for all scopes I get this:

ScopeId(0): Program: ScopeFlags(StrictMode | Top)
ScopeId(1): Class(Outer): ScopeFlags(StrictMode)
ScopeId(2): Function(<anonymous>): ScopeFlags(StrictMode | Function | Constructor)
ScopeId(3): Class(Inner): ScopeFlags(StrictMode | Constructor)

Debug code:

let scopes = semantic.scopes();
let nodes = semantic.nodes();
#[allow(clippy::cast_possible_truncation)]
for i in 0..scopes.len() {
    let scope_id = ScopeId::new(i as u32);
    let flags = scopes.get_flags(scope_id);
    let node_id = scopes.get_node_id(scope_id);
    let kind = nodes.get_node(node_id).kind();
    let name = kind.debug_name();
    println!("{scope_id:?}: {name}: {flags:?}");
}

As this isn't visible on playground, is this the result of a very recent change? Or is there something else funny going on?

@overlookmotel overlookmotel added C-bug Category - Bug A-semantic Area - Semantic labels Dec 14, 2024
Dunqing pushed a commit that referenced this issue Dec 15, 2024
…alizer scope re-parenting (#7901)

Add a fast path for inserting instance property initializers into constructor, when no existing constructor or constructor has no bindings. This should be reasonably common.

The `Scope flags mismatch` errors are due to #7900.
@Boshen Boshen self-assigned this Dec 15, 2024
@Dunqing Dunqing self-assigned this Dec 16, 2024
overlookmotel pushed a commit that referenced this issue Dec 16, 2024
…ope (#7932)

close: #7900

After #4283 changed, we don't need to inherit `ScopeFlags` from the `constructor`, `set`, `get` anymore, I think this is a logic of forgetting to remove
@overlookmotel
Copy link
Contributor Author

Fixed in #7932.

@overlookmotel
Copy link
Contributor Author

overlookmotel commented Dec 16, 2024

@Dunqing @Boshen Do you have any idea why playground showed different ScopeFlags for this example than when run locally?

I thought maybe it was because this problem was created by a recent change, and so Playground just wasn't up-to-date with latest main. But now I don't think that's true. Right now Playground still shows that class Inner does not have Constructor flag, and it must be up-to-date now.

It's a problem for debugging if Playground behaves differently from what we get locally / in production.

@Dunqing
Copy link
Member

Dunqing commented Dec 17, 2024

@Dunqing @Boshen Do you have any idea why playground showed different ScopeFlags for this example than when run locally?

I thought maybe it was because this problem was created by a recent change, and so Playground just wasn't up-to-date with latest main. But now I don't think that's true. Right now Playground still shows that class Inner does not have Constructor flag, and it must be up-to-date now.

It's a problem for debugging if Playground behaves differently from what we get locally / in production.

I found the root cause is #7950

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-semantic Area - Semantic C-bug Category - Bug
Projects
None yet
3 participants