module parameters with < > proposal

[mighdoll]

module parameters proposal, goes with #222

Please review this PR for form - is this how we'd like to do proposals? Approving this PR doesn't mean agreeing to the proposal, just that we have a proposal we can discuss.

I used Claude and Codex during the draft/review cycles though I wrote and rewrote almost every sentence. I think the AI helped overall - certainly it caught a number of inconsistencies and poorly written sentences in earlier drafts.

I'm sure there'll be plenty to discuss on the actual module parameters topic, let's do that in #222!

[k2d222]

Form: the PR doc looks just right: not too verbose, clear enough. In terms of workflow, #222 is basically empty, so I'm wondering if we should just do PR right away for reasonably well-defined proposals. Doesn't matter too much.

Content: there are two main differences with #164

• module <> syntax instead of @param. I don't care much about syntax at this early stage, but my new favorite is a module { } block with curly instead of angle brackets. This way, module params are just const declarations inside that block, and we can apply visibility modifiers / re-export mechanisms the same way we do outside. I find it ackward in < > that the param const is essentially a declaration, but has no ;, and it doesn't mix well with the syntax proposed for re-export of submodule params in @main module <> IMO.
• params can be overridden in import statements. This was part of #146 (ancestor of #164) but was stripped to simplify things because we were unsure about module instantiation / duplication of variables, and that declaring conditions in shader code + allowing constants in conditions was already a nice improvement. I'm still unsure about the duplication behavior, so I still think we should start without it. For instance, idea of sharing types when possible is interesting. It deserves further discussion as it might cause unexpected things when a type was shared and suddently is no longer unified (e.g., the host code changed a param and that can affect the whole compilation chain).

I don't understand what @main does and why we need it. Can you elaborate?

Soo, overall I'm happy with the workflow and approve to merge, and we can continue discussion with the doc in the repo.

[k2d222]

I'm also thinking that once we merge such proposals it might become more difficult to edit them as we discuss them. I propose that these docs can be edited on the main branch by its author without further PRs.

[k2d222]

After we merge this I'll write the proposal for #200

[mighdoll]

I'm wondering if we should just do PR right away for reasonably well-defined proposals. Doesn't matter too much.

agreed, low bar. what the bar for proposal PR? I guess: has use case issue, has discussion issue, isn't obviously out of scope/impossible, maybe some sections required...? (We can write it down in #195.)

[mighdoll]

Content: there are two main differences with #164

(responded to the content in #222)

[stefnotch]

Approved on basis of this proposal being very clear!

I'll leave my review comments here, commenting on the technical specifics.

[stefnotch]

Nitpick: I think having multiple module statements would look confusing. It's conceptually just a single module that we have.

[stefnotch]

Nitpick: Should we really tie visibility into the module parameter design? It makes the feature more complex.
What does it bring us?

[stefnotch]

Nitpick: "don't stabilize" is slightly challenging to implement. It implies being able to solve the Collatz conjecture.

[stefnotch]

I thought this was a very funny comment, so I felt inspired to actually demonstrate it

// We do two wildcard imports. 
// If they refer to the same thing it's fine.
// But we have to check if they really refer to the same thing.
import package::bar::*;
import package::collatz::*;

// Usage of A
const B = A + 1;



// bar.wesl
// So this is the trivial setup
public const A = 3;



// collatz.wesl
// And here we have a parameterized module
module <const N: u32>;

// Base case. We re-export the A from bar.wesl
@if(N == 1)
public import package::bar::A; // this one is okay

// Use an arbitrary number here
@if(N == 24524623)
public const A = 7; // this one clashes

// And drive the collatz conjecture logic
// This repeatedly imports a different collatz.wesl
// And the collatz conjecture says that it should stabilize at 1
public import package::collatz<select(N/2, 3*N+1, N % 2 == 1)>::A;

[stefnotch]

Nitpick: This is a bit unclear.

const A = 3;

// this should be disallowed
@if(A)
const B = 5;

// but that should be allowed?
import package::foo<A>;

[stefnotch]

This is the one thing that worries me. We're deviating from well studied module systems with this one rule.

WGSL currently relies on shared state quite often. Like var<private> or like this piece of code that does a bit of memory management on the GPU. I don't think I could realistically reason about how such code would behave if the var declarations were shared across multiple instances.

Especially since the sharing would be a global property, as opposed to a local one. Two completely unrelated places can import a my_buffer<u32> module and would see the data from the other place.

We should enable local reasoning. There are two ways of getting there, IMO.

1. Discourage module level vars. This is the world where binding structs, Dynamic pointers proposal #200 and co live. Explicitly pass the vars around.
2. Learn from languages that do in fact have a well thought out module system. OCaml distinguishes between "this is a module" and "that creates a module". OCaml goes many steps further, each of which enables completely new and lovely ways of composing code. We should learn from there!

[stefnotch]

Nitpick: "Link wide defaults" is messing with me.

As a quick, not too hypothetical example:
I write a PBR shader and rely on Lygia's gamma correction. I also write a texture fetching routine and rely on Lygia's gamma correction. I expect the gamma value to be something very specific, since the PBR shader's math is based on that. And I expect the gamma value to be something very specific in the texture fetching shader, since that gamma value is specified in some external standard.

So I now have to fully specify the gamma value and cannot rely on the default. If I accidentally rely on the default, that is a bug waiting to happen. I have to remember to check each module for "wait, does this have behaviour that anyone else could override". I think an explicit world is simpler and more convenient to use.

[stefnotch]

Mostly unimportant design question: I think this affects name mangling, which affects our shared test suite. I guess we'll have to go down the road of having a shared test suite that is name mangling agnostic?

[stefnotch]

Here's a good example for where "sharing per parameterization" breaks down.

When two unrelated modules happen to import a module with the same parameterization

// perlin_noise.wesl
import math::rng<STREAM = 42>

fn get_value(a: vec2f) -> f32 { ... }

// simplex_noise.wesl
import math::rng<STREAM = 42>

fn get_value(a: vec2f) -> f32 { ... }

Then we get the insane behaviour where calling

perlin_noise::get_value()
simplex_noise::get_value()

gives different results from calling

simplex_noise::get_value()
perlin_noise::get_value()

[stefnotch]

And interestingly Rust also wants module level generics

https://smallcultfollowing.com/babysteps/blog/2022/04/17/coherence-and-crate-level-where-clauses/#module-level-generics

[ncthbrt]

Has an alternative syntax that looks roughly like this been considered?:

export default module<SAMPLES: u32 = 8> {
 // module contents
}

The reason why I prefer this variant over having a separate block is that it provides a scope in which these module arguments are valid, as well as later allowing space in designs for nested modules...

I also tend to agree that I somewhat prefer an optional syntax for specifying these module arguments with a {}. Especially if you wanted to also be able to specify functions.

For example this could be allowed:
lights<const SAMPLES: u32=16> but this would also be allowed: lights with { const SAMPLES: u32 = 16; }

Edit: The above style also might feel a bit more familiar to users.

Another option would be a keyword for each overridable property instead of a module block. I think that would be less confusing to users who are used to {} denoting a scope. That the variables within module{} are accessible outside that block feels a little counterintuitive. In my opinion, either all functions that need access to overridable variables should live in the module scope, or the overridable variables should be declared in the outer scope. The former, where all functions that reference the overridables must be in the module scope has the advantage that only that specific block needs specialisation versus the entire module at large.

You could conceivably also allow multiple module blocks. This is interesting because then it allows users to more precisely control emission of specialised code and thus reduce register pressure.