bevyengine/naga_oil

Imports clobber field names in locally defined structs

Runi-c opened this issue · 5 comments

#import dep::{foo, bar}

struct Example {
    bar: u32,
}

fn test() {
    let example = Example(0u);
    foo(example.bar);
}

This code produces this error:

error: invalid field accessor `bar`
  ┌─ shaders/test.wgsl:9:39
  │
9 │     dep::foo(example.bar);
  │                                       ^^^ invalid accessor
  │
  = invalid field accessor `bar`

If I amend the code to not import a symbol named bar, the error goes away. But it took a lot more debugging than I'd have liked to figure out that that's what was happening.

hm. i was aware that local names got stamped on, but i thought they got stamped everywhere including in the struct access. we probably need to widen the search/replace to catch example.bar and map it to example.dep__NAGA_OIL_MOD__bar as well.

i realise this is not a nice approach, but more sophisticated handling requires AST context, which we can't currently obtain without a complete/working shader as input.

That makes sense actually. It sounds like the current behavior is from #31? If there's already an exception carved out for identifiers following ., maybe an alternative here could be widening that exception to include identifiers preceding : (presumably, these are always struct members or function arguments)

That sounds like a better idea.

What about switch statements? They can also have a colon after a variable name.

switch foo {
  case bar: {       // Const-expression can be used in case selectors
    foo = 3;
  }
  default { 
    foo = 4;
  }
}

Perhaps it’s best to explicitly disallow shadowing of modules with variables then.

Though we can’t easily prevent it in field names of imported structs …