KittyCAD / modeling-app

The KittyCAD modeling app.
https://kittycad.io/modeling-app/download
MIT License
413 stars 35 forks source link

Scoping error between program body and function definition scope #1285

Closed Irev-Dev closed 3 months ago

Irev-Dev commented 10 months ago

The following code creates a Cannot redefine varShouldBeScoppedToFn error

fn fnDef = (varShouldBeScoppedToFn) => {
//          ^Cannot redefine varShouldBeScoppedToFn
  return varShouldBeScoppedToFn + 2
}

const result = fnDef(2)

const varShouldBeScoppedToFn = 6
const part001 = startSketchOn('-XZ')
  |> startProfileAt([-5.8, 23.97], %)
  |> line([21.62, -4.8], %)

const result2 = fnDef(4)

even though this variable should be within the function scope

note the first call const result = fnDef(2) doesn't trigger it

It's the second one const result2 = fnDef(4), must have something to do with the function call after the variable declaration.

Here's a more minimal viable reproduction

fn fnDef = (varShouldBeScoppedToFn) => {
  return varShouldBeScoppedToFn + 2
}

const varShouldBeScoppedToFn = 6

const result2 = fnDef(4)

Moving the variable declaration to the top still has the error

const varShouldBeScoppedToFn = 6

fn fnDef = (varShouldBeScoppedToFn) => {
  return varShouldBeScoppedToFn + 2
}

const result2 = fnDef(4)
Irev-Dev commented 10 months ago

Bit more context on the likely cause here https://kittycadworkspace.slack.com/archives/C04KFV6NKL0/p1704948075061969

image

Irev-Dev commented 10 months ago

Should be solved by grackle anyway

jessfraz commented 4 months ago

i thought this might be a fun one for showing jon around @adamchalmers but no worries if not

Irev-Dev commented 4 months ago

Yeah I was thinking the same thing.

jtran commented 4 months ago

Yes, I'd love to tackle this! It's related to how you handle variable shadowing, and I need to get up to speed on some things. But this is definitely on my radar.

In the meantime, if you want to read about it, there's a chapter in the Crafting Interpreters book about solving this problem: https://craftinginterpreters.com/resolving-and-binding.html I'd be happy to give the high level summary. I'd like to first get more familiar with the modeling-app codebase so that I can find the relevant parts of the code.

jtran commented 4 months ago

I started to work on this. I know it's not assigned to me, but I figured that even if it doesn't get used, I'm learning a lot by attempting this.

Irev-Dev commented 4 months ago

Sounds perfect, I don't think it's actively under development anyway.

Irev-Dev commented 3 months ago

🚀