visibility design and discussion

[mighdoll]

TLDR

1. Module visibility. Three levels: public (visible to any package),
   private (declaring module only), and package (same package, the unmarked
   default).
2. Re-exports. public import re-exports the imported names under the
   current module's path; public import path::* does the same for a wildcard
   (re-uses @wildcardable diagnostics).
3. Host pipeline visibility. The link's root module defines the host-visible
   API: entry points, resource variables, and pipeline-overridable constants
   reach the host only if the root module declares them or public imports them.
4. package.wesl. The file backing a package's top-level module, so its
   items are reachable as <package>::item (e.g., import wgsl_test::*). One
   place a library can put a re-export prelude.

Why these four things are covered:

• module visibility is needed for re-exports
• re-exports are needed for wildcards
• pipeline visibility works best integrated with the module visibility design
• package.wesl is so that we can write e.g., import wgsl_test::*;

(Note that VisibilityLanguages.md is discussion background, it'll be removed before merging.)

Would resolve:

• #65 @publish import
• #98 allow private marking
• #28 public by default (closed)
• #130 wildcards
  (largely covered already; this PR fills in the re-export piece)
• PR #62 comment for lib.wesl

Related

• #103 sub-modules private by default

[s-ol]

This looks very solid to me and would definitely bring a big usability gain to libraries like lygia. The one thing that doesn't seem be discussed in much detail is package.wesl - from what I gather the package structure becomes something like:

• pkgname/
  • package.wesl
  • mod_a.wesl
  • mod_b.wesl
  • mod_b/
    • nested.wesl

It seems like the docs here don't explain why this approach is chosen over an index-style approach, i.e.:

• pkgname/
  • index.wesl
  • mod_a.wesl
  • mod_b/
    • index.wesl
    • nested.wesl

Personally I don't have a strong opinion either way, but there is lots of prior art for the second option (most JS package systems, Python, Rust, Lua) while I can't think of anything that has this strict root/non-root separation off the top of my head.

[stefnotch]

I believe that we should be able to dig up the Rust recommendations for that. IIRC, Rust discourages the mod.rs style, because it leads to a lot of files with the same name. Which means that whenever a tool only or primarily shows a file name, one ends up with very little useful info.

For example, if I have 3 tabs open in VSCode and they all say mod.rs, I will have a hard time finding the right one.

[stefnotch]

Also quick and very inconvenient note: private is somehow not a reserved WGSL word.

[stefnotch]

public import path::* has one notable technical difficulty: It makes import resolution a lot more complex.

Without wildcards, resolving an import means going to the right module and finding the item.

With wildcards, resolving an import means looking through a set of modules to find the right item.

With wildcard re-exports, resolving an import means looking through a set of modules recursively to find the right item. And since modules can have cycles, one now needs to keep track of which modules have been visited during import resolution. In the language server, this is a fair amount of extra complexity.

This particular feature also, I believe, interacts really badly with macros or other compile time code generation. Rust's import system ends up being a very complex fixed point finding algorithm which may sometimes simply fail.

(I think it also interacts with parameterized modules. Since wildcard reexports is the missing piece to make import statements recursive. If imports can cause any sort of computations, which compile time generated code does, and which parameterized modules do, then one gets the halting problem. Or so the story goes.)

Usually I wouldn't object to complexity too much.
In this case, I did want to point it out, since wildcard re-exports enables nothing new. It is a very rarely used feature and I could always manually write it out.

[mighdoll]

Personally I don't have a strong opinion either way, but there is lots of prior art for the second option (most JS package systems, Python, Rust, Lua)

while I can't think of anything that has this strict root/non-root separation off the top of my head.

Not sure, always interesting to look around tho!

Here I was only filling the gap where our current scheme doesn't allow something at the root level. I think small libraries especially might like to enable their users to my_pkg::foo(); or import wgsl_test::*; without adding an extra path level.

It'd be a separate issue, but your comments about consistency... I suppose we could revise the existing rules so that X::X.wesl always flattens the contents to X::. package::package.wesl flattens to package::. lygia::math::math.wesl flattens to lygia::math, etc.

[mighdoll]

Also quick and very inconvenient note: private is somehow not a reserved WGSL word.

wat!? good catch. Perhaps @public @private. I think we'll want to ask the WGSL committee too.

[mighdoll]

Usually I wouldn't object to complexity too much.
In this case, I did want to point it out, since wildcard re-exports enables nothing new. It is a very rarely used feature and I could always manually write it out.

Worth thinking about. I like the idea of implementing the needed features w/o gilding the lily too much.

The cycles/complexity concerns seem heavy for a feature that isn't must-have. And foreclosing options for parameterized modules would be unfortunate.

Also I realize that the PR doesn't address some related issues:

• what if there's a latent collision between wildcard re-exports in a library? That would be something we'd want to catch at publish time, not wait for clients to use the library and find an error.
• what happens if you wildcard re-export from a module with wildcard imports?

I'm thinking best would be to pull wildcard re-exports from this PR.

Side note, if we go with the publish tool route as planned with #183, I suppose we could also expand wildcard re-exports like we're planning for wildcard imports. That would help for the cross package cases.

[mighdoll]

pushed a draft

• @public and @private
• wildcard re-exports deferred

[stefnotch]

We could also go with

• pub, priv
• public, protected (Rust does not have a private keyword, upside of private by default)

Also, what does private mean? Rust goes all in on the hierarchical module system and says

If an item is private, it may be accessed by the current module and its descendants.
https://doc.rust-lang.org/reference/visibility-and-privacy.html

[mighdoll]

* `pub`, `priv`

I like this option too, a bit more cryptic but if perhaps best if we prefer keywords to attributes.

* `public`, `protected` 

I like public clear and readable. I fear confusion around protected - it has a contrary meaning in other languages visibility systems.

I suppose we could do public and priv..

Also, what does private mean? Rust goes all in on the hierarchical module system

just the module, no hierarchy. we could expand hierarchically later, it's a nice idea. But I think of most WESL programs as smallish, so perhaps unnecessary complexity, better to defer.

[stefnotch]

For the public, protected option, I was trying to suggest having the following visibility modifiers

• public (keyword)
• protected (keyword)
• private (the default, thus does not need a keyword)

[mighdoll]

* `private` (the default, thus does not need a keyword)

Ah, that was my first instinct too. I like modularity. But I've come to think that package by default for WESL is better. See Why package by default? in the design discussion doc. package by default is low ceremony for most projects, and works better for wgsl compat at the root module..

[stefnotch]

Also interesting tidbit:
Rust allows for two different styles of visibility. One is a local and hierarchical style, where I only use public and private. And I apply that to both items and modules. A internal (protected) item is modeled by having a public item inside a private module.

The other style is the global style, where I use 3 access modifiers. Public, internal (protected) and private.

And apparently some of the very prolific Rust contributors, including epage who maintains a bunch of extremely well written libraries, vastly prefer the global style. So we are on the right track!

https://www.reddit.com/r/rust/comments/1k5yv6p/two_ways_of_interpreting_visibility_in_rust/

[stefnotch]

More from Rust land:
The reasons for why they went with private by default for fields is to allow for local reasoning and

Visibility is often how soundness is achieved for many types in rust.
https://rust-lang.github.io/rfcs/0001-private-fields.html

They also include statistics which says that private is more common than public.

[stefnotch]

And from Kotlin lang: They went with public by default, only to realize that it might have been a mistake. So defaulting to a more restrictive choice like "internal" seems wise

https://kotlinlang.org/docs/api-guidelines-simplicity.html

[stefnotch]

Note from Java:
Java's default is actually somewhat similar to Rust. In Java, a "package" corresponds to a folder. And classes are visible within a package by default.

[stefnotch]

We really should ask the WGSL committee regarding the private keyword. Would you like to do the honors?

[stefnotch]

I dug into it and asked the WebGPU commitee gpuweb/gpuweb#6264

I couldn't quite figure out where it got lost though. It seems to have happened quite a while ago, judging from Naga's source code

[stefnotch]

I talked to a friend regarding this. priv would be a weird choice. As much as it pains me to say this, I would prefer private as a context sensitive keyword over priv.

[stefnotch]

private is intentionally a context sensitive keyword said David Neto. We can use it, but it'll require a bit of context sensitive parsing.

And Jim Blandy pointed out that Rust doesn't have a private keyword.

[stefnotch]

I asked the Slang team!
Here's their response and the Discord invite link is https://khr.io/slangdiscord. I'm happy to repost the thread here if someone wants me to.

[stefnotch]

Can public import also re-export modules?

[mighdoll]

could be a future thing, perhaps..

[stefnotch]

#189

We can do it as a future thing, but we might need this for Bevy

[stefnotch]

How does this interact with wildcards?

Is this a name clash?

import foo::*; 
import bar::*;

const B = A;

when

// foo.wesl
private const A = 3;

// bar.wesl
public const A = 3;

Or is visibility more intertwined with name resolution?

[mighdoll]

good question, I think it should be only be a clash if both are visible.

[stefnotch]

Is there a reason why this should be allowed at all?

[mighdoll]

Authors can write a fn that makes and returns a struct with private visibility. API users can pass the struct back to another fn, but can't build one them themselves w/o going through the API. That might be nice for the author, maybe they want to track every struct built in a table or something and don't want users to be able to able to cheat around their intended makeThing() api.

Requiring that every struct mentioned in a public fn also be public is reasonable, but less flexible for API authors. Other languages undoubtedly differ on this. I was just scanning through Rust RFC 2145 which makes it sound like Rust had a strict requirement and then changed to warn/lint. Go allows it but has a lint. TypeScript allows it I think.

[stefnotch]

Couldn't one achieve the same result by making one of the struct fields private? Then the struct is also not constructible for the API users.

I don't fully understand why Rust relaxed their rules, from the sounds of it they started out with a suboptimal version of "no private in public".

[mighdoll]

Couldn't one achieve the same result by making one of the struct fields private? Then the struct is also not constructible for the API users.

Struct field visibility is interesting, but perhaps mostly orthogonal.

I agree we could block consumer construction of structs w/ private fields. Foo(1,2,3) wouldn't work outside the module if one of Foo's fields was private. But that workaround only matters now if we go strict on the leaked-type rule, and going strict cascades through every transitive field reference. Sounds like Rust wanted to relax that in RFC 2145, so not sure we'd go that way.

i.e. if we eventually add private struct fields, it's plausible, maybe even likely that we'd still want to allow public structs to have fields of private type. In that case, private fields would give a new way to have private struct types: named-but-not-constructable in addition to the current unnamed-not-constructable.

Either way, I'd suggest we defer til we decide to do field level visibility.

[stefnotch]

What about other WESL tools like the language server?

[mighdoll]

good point, other tools like the lang server should warn too.

[stefnotch]

Why restrict re-exporting to @public? I find it quite useful within a package as well, once the package crosses a certain size threshold.

[mighdoll]

simplicity, could add later I agree

[stefnotch]

I think it'd be simpler to implement if we have the internal variant as well. The way it's currently phrased with @public means that I need to add quite specific checks for the restrictions.

(This is very speculative of course. My point is mostly that I think we can safely allow more here, without adding notable implementation complexity.)

[mighdoll]

It's a good question, I went back and forth on this as I was drafting. What's simpler for the user? I agree it's a little odd to have to declare an app's own fragment shader as public just so that it can be pulled into the root module. On the other hand it's another kind of re-exporting for users to learn about, which adds complexity and brings up questions about internal re-exporting in general (two paths to a declaration, module path vs declaration visibility, etc.) that I'd rather keep out of scope for this PR.

I'd prefer to keep re-export to public import only for this if possible. We can revisit and extend to package import if need be in a subsequent PR.

[stefnotch]

Ah, so basically I cannot do public import package::some_plain_wgsl_file::fragment_shader;. Because the fragment_shader in the plain WGSL file isn't public.

[stefnotch]

But I should be able to do internal import package::some_plain_wgsl_file::fragment_shader;. After all, package-level items are pipeline-visible.

[mighdoll]

agree that would be nicely orthogonal, but prefer to defer to keep this PR constrained.

[stefnotch]

I don't think this argument holds up. Pretty much every host language (C, C++, Rust, Typescript, ...) already makes the authors do this work. And my the dream shading language would be whatever host language I use.
So using the same patterns across my codebase is less mental overhead for me. Instead of having to remember "wait, well written WESL means that I have to write private more often"

[mighdoll]

It's a good challenge, this argument may not land for many. Presuming we end up going with package by default, I'll reorder this section to lead with the structural arguments and de-emphasize this point.

The host-language story is mixed (e.g. Java, Scala, Kotlin, Swift are not private by default, so it's not a clean vote), but for users coming from TS or Rust you're right that this argument doesn't carry much weight on its own.

[stefnotch]

I sort of agree with this one. It should be possible to incrementally adopt wesl.

But at the same time, I'd also like to note that that existing WGSL code has no imports and is not always designed for it.

If I were to add visibility to WGSL itself, I would likely choose an option where one has to add visibility markers before being able to use the code in other files. This is analoguous to what Javascript did with the export keyword.

In fact, as an imaginary WGSL (not WESL) standard writer, I would argue that allowing arbitrary imports is a footgun. Existing WGSL code has been written without a module system in mind. So there are no encapsulation boundaries anywhere. I would start accessing all the right and all the wrong things, without the language steering me in the correct direction.

With my IDE hat on, I would agree. If I add "allow imports from anywhere" into any of my existing WGSL codebases, I'd get a hundred silly recommendations. Often I'd get 5 separate recommendations for the same type, because I copied it 5 times. And worse yet, of those 5 times, 2 of them would be subtly different, due to reasons like gpuweb/gpuweb#4574

[mighdoll]

I suppose the IDE suggestions perspective might cut both ways. If I copy in some useful WGSL code from an old project or the web, then I'd like to use the IDE auto complete to explore the WGSL. But if I leave the WGSL code in for a while, then I agree I'd prefer to just have the one routine I know I need suggested but can't quite remember how to spell and not the other ought-to-be-private declarations. Autocomplete is probably not the deciding factor regardless; the incremental adoption is more important, as you note.

[stefnotch]

This is a solid argument. So "private by default" with a file-level private would immediately break all WGSL files.

Which means that the WGSL standards commitee could not choose this option without introducing a breaking change to WGSL.

In terms of other options:

• For the entrypoints and bindings, there is the option of saying "@fragment implies public" and "@group implies public". But that feels weirdly inconsistent with the rest of the language. And this rule fails to cover overrides.
• Root modules with a different behaviour seem weird. Having a consistent language means fewer things that I have to remember.
• private could simply not affect pipeline visibility. But then we break encapsulation in a weird way.
• private could be much less strict, similar to Rust. Rust made the observation that file level private is often still too strict.

[mighdoll]

private could be much less strict

We could use package by default, and have private cover both the module and its descendant modules. So a private fn foo declaration in module shaders/bar.wesl would be visible also to shaders/bar/zap.wesl.

Seems reasonable and doesn't conflict with the current default or the import path model. The locality cost for the reader is mild, they'd have to consider their relative place in the module tree, where today they just have to consider the declaration itself. It's additive, we can add it later without breaking anything. Prefer to defer until we know we want this approach vs. other potential mechanisms for bigger projects.

[mighdoll]

other options:

(suggested on discord) Package visibility by default for items, but a submodule is path-private: not reachable to other modules unless its parent forwards it. The forwarding mechanism is re-exports syntax applied to module paths, rather than just items as in the current PR. So a tests/ directory whose parent doesn't forward it is unreachable from non-test code in the same package.

But:

• Today an item's visibility is clear from its declaration. Path-private modules mean you'd have to read ancestor files to know what the parent forwarded. Visibility becomes non-local
• A main advantage of package by default is that small projects with a few files don't need to care about visibility rules. This would be effectively private by default. We'd lose the advantages of package by default.

I agree we'll want a solution for tests/, but #160 sketches an alternate approach based on wesl.toml, so we've at least one alternate way forward.

Path visibility might be worth a separate PR if #160 doesn't work out. Either way I think we'd still want declaration level private for the routine within-module case. Path visibility wouldn't replace it.

[stefnotch]

Today an item's visibility is clear from its declaration. Path-private modules mean you'd have to read ancestor files to know what the parent forwarded. Visibility becomes non-local

There are semver relevant cases where making a module private is important. Main one being module re-exports.
If I write a little library that does the following

public bevy_pbr::internal_module as bar;

Then Bevy's internal_module is suddenly part of the public API of my library users. Aka renaming it becomes a breaking change for Bevy, despite the intent very much being "that module should not be public".

Therefore, we do need a way of saying "this module is private".
Which yes, makes visibility less local.

We could also get there with an explicit rule of "a module's visibility is the maximum visibility of all its items". As in, if I only have internal items, the entire module is internal.

[stefnotch]

This would be effectively private by default

It is significantly less strong than private by default.
With a strong interpretation of private by default, I have to add a marker to each item.

// my_module.wesl
public const a = 3;
public const b = 5;

With "sub modules are private by default", I only have to add a marker to the parent module's import statement.

// package.wesl
public import package::my_module;

[stefnotch]

So "private by default" with a file-level private would immediately break all WGSL files
-- @stefnotch

Well, looking at Javascript's history, this isn't entirely correct either. JS introduced modules and required the callers to say "this is a module"

That said, WebGPU could do an equivalent thing.

const vertexShader = device.createShaderModule({ 
  module: fancyShaderString, 
  // instead of the current 
  // code: shaderString
});

[stefnotch]

I'm not sure if I agree with the assumption here. I'm more familiar with game development, where a typical shader codebase is

• a few files that I wrote. These are indeed somewhat small. But they are also very standalone, due to the nature of things. In fact, while many big game engines (e.g. Unreal Engine) have "material functions", it is kind of rare for me to import code from another shader that I wrote.
• a massive amount of game engine shader code that I rely on. I import a few types from here and the rest is a black box

And I suspect that the same is true on the web, since frameworks like Babylon.js and Three.js are quite popular.

So targeting small shader projects that are not already a part of a game engine is, I think, targeting an overly small part of the userbase.

[mighdoll]

Agreed that many people will use big frameworks when they use webgpu. And the authors of big projects still have all the capabilities they need, regardless of the defaults discussed here. Here it's more about syntax convenience for the users of those frameworks (and those not using frameworks). The point is that most people using WebGPU will be writing smallish things with smallish teams that aren't so important to carefully internally modularize.

[stefnotch]

I suppose one way of properly dealing with override would be to extend WESL to also cover a small bit of the WebGPU API. We'd extend the way createShaderModule works or something along those lines.

[stefnotch]

There is one amusing way of bypassing this restriction.

//- utils.wesl
struct InternalStuff { a: 32 }

//- package.wesl
import package::utils::InternalStuff;
public alias NowItIsPublic = InternalStuff;

[mighdoll]

clever and amusing, indeed!

[stefnotch]

Should we see if we can get #183 over the finish line?

I do have one alternative proposal for this and #183, which is based around making full use of import statements. Here I'm only listing the upsides.

It would look like this

@wildcardable
public import bevy::prelude;

Upside: No new module keyword for this use-case. Fewer keywords is better.

Sub-modules would be private by default.

public import package::lights;
public import package::shadows;
internal import package::utils;

Upside: Automodules would no longer be a feature. It adds a lot of complexity to the language server. (I also suspect that the current version of automodules is not implementable in browsers, since it relies on web servers returning a "not found" within a reasonable timeframe.)
Upside: Automodules were an anti-pattern for libraries, since libraries should document their modules. Which cannot be done with automodules.

@if would compose nicely. For example, to do a switch between desktop and mobile, I would do

@if(DESKTOP)
internal import package::desktop::raytraced_lights as lights;
@if(!DESKTOP)
internal import package::mobile::lights;

and now package::lights always refers to the correct thing. More importantly, the language server will not suggest package::mobile::lights as the autoimport whenever I use lights. I think this is a big upside. The current proposals do not have a good way of doing this.

internal becomes a keyword. Thus enabling the following, which does not widen the import

internal import package::vertex::vertex_main;

[stefnotch]

New question: If it is an item in the current module, but the item is not accessible, what should happen?

1. Throw an error. This always behaves consistently.
2. Ignore it and continue on. Downside is that name resolution can now depend on where you're importing something from in a very funky way.