[RFC] nameRegistry bug?

[cieplypolar]

I am not sure if the condition in makeValid method is sufficient.

makeValid(primer: string): string {
  if (isValidIdentifier(primer) && !this.#usedNames.has(primer)) {
    this.usedFunctionScopeNames?.add(primer);
    return primer;
  }
  return this.makeUnique(primer, false);
}

In the resolution of a forOf loop, I need to check if an identifier is free by calling ctx.getById method

let index = 'i'; // it will be valid name, no need to call this.ctx.makeNameValid
  while (ctx.getById(index) !== null) {
    index += 'i';
  }

If we were checking the function scope in the makeValid method, the while loop could be replaced with a single call to makeNameValid

Please tell me if I am correct.

[cieplypolar]

I found out that in the following case, we are NOT responsible for the rename from i -> i2 (at least not in the nameRegistry)

test('trying to break things', () => {
  const f = () => {
    'use gpu';
    const i = 0;
    let foo = i;
    {
      const i = foo;
      const boo = i;
    }
    foo = i;
  };
  expect(tgpu.resolve([f])).toMatchInlineSnapshot(`
    "fn f() {
      const i = 0;
      var foo = i;
      {
        let i2 = foo;
        let boo = i2;
      }
      foo = i;
    }"
  `);
});

This is scary...

[cieplypolar]

After a long fight with transformations, I finally found our renamer - esbuild 😠 . You can verify it on the branch fix/i->i2, or just trust me.

[cieplypolar]

After changing vite bundler from esbuild to rolldown renaming disappeared.

[cieplypolar]

1. Good news is that wgsl handles variable shadowing by itself.
2. Bad news is that our naming logic is undeterministic (it depends on the order of declarations)

---

For example

const f = (foo: number) => ...
const foo = ...

resolves to

fn f(foo: u32) -> ... 
let foo = ...

but

const foo = ...
const f = (foo: number) => ...

resolves to

let foo = ...
fn f(foo_2: u32) -> ...