-
Notifications
You must be signed in to change notification settings - Fork 7
Add wildcard imports #183
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Add wildcard imports #183
Changes from 15 commits
071d6db
0e146af
3232a88
ae56930
216457c
8fcb4a7
f4940e2
a1ec6c8
b758d95
abfb0e7
81e06f4
7d4bfd0
9e46adf
2a21505
1a46cd8
b58f43b
983259d
eb455ad
fa9e837
946d64d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -22,6 +22,10 @@ import my::geom::sphere::{ draw, default_radius as foobar }; | |
|
|
||
| // Imports a whole module. Use it with `bevy_ui::name` | ||
| import bevy_ui; | ||
|
|
||
| // Import all items from another module | ||
| import bevy::prelude::*; | ||
| import wgsl_test::expect::*; | ||
| ``` | ||
|
|
||
| These can then be used anywhere in the source code. | ||
|
|
@@ -66,22 +70,23 @@ translation_unit: | |
| | import_statement* global_directive* global_decl* | ||
|
|
||
| import_statement: | ||
| | 'import' import_relative? (import_collection | import_path_or_item) ';' | ||
| | attribute* 'import' import_relative? (import_collection | import_path_or_item) ';' | ||
|
|
||
| import_relative: | ||
| | 'package' '::' | 'super' '::' ('super' '::')* | ||
|
|
||
| import_path_or_item: | ||
| | ident '::' (import_collection | import_path_or_item) | ||
| | ident ('as' ident)? | ||
| | '*' | ||
|
|
||
| import_collection: | ||
| | '{' (import_path_or_item) (',' (import_path_or_item))* ','? '}' | ||
| ``` | ||
|
|
||
| Where `translation_unit` and `ident` are defined in the WGSL grammar. | ||
| `ident`s must not be current WGSL keywords. `ident`s also must not be | ||
| current WESL keywords: `as`, `import`, `package`, `super`, or `self`. | ||
| current WESL keywords: `as`, `import`, `module`, `package`, `super`, or `self`. | ||
| Reserved words that are | ||
| not current keywords are allowed, | ||
| but not recommended. | ||
|
|
@@ -91,6 +96,19 @@ An item import imports a single item. The item can be renamed with the `as` keyw | |
|
|
||
| An import collection imports multiple items, and allows for nested imports. | ||
|
|
||
| A wildcard import imports all top-level declarations from a module. Submodule names and submodule contents are not imported. | ||
|
|
||
| WESL also extends WGSL's `global_directive` rule with a `module_declaration` form, used by `@wildcardable` (see [Wildcard imports](#wildcard-imports)) and reserved for future module-level metadata. `attribute` is the WGSL attribute rule. | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
or would it make sense to allow multiple?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'll change to clarify just one for now. It's not really necessary to have more than one yet afaict, and we can relax to multiple later (or not if we introduce other future semantics on
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. just one, clarified in the PR |
||
|
|
||
| ```ebnf | ||
| global_directive: | ||
| | ... // existing WGSL forms | ||
| | module_declaration | ||
|
|
||
| module_declaration: | ||
| | attribute* 'module' ';' | ||
| ``` | ||
|
|
||
| ### Import resolution algorithm | ||
|
|
||
| To resolve the import, the recursive structure is flattened out. This means turning every `import_collection` into multiple separate imports, ending with the items. | ||
|
|
@@ -111,8 +129,9 @@ Then, one iterates over each segment from left to right, and looks it up one by | |
| 2. We take that as the "current module". | ||
| 3. We repeatedly look at the next segment. | ||
| 1. Item in current module: Take that item. We must be at the last segment, otherwise it's an error. | ||
| 2. (Else if re-exported or inline module in current module: We continue with that module.) | ||
| 3. Else go to `current module path/ident.wesl` | ||
| 2. Wildcard import: Take all items in the current module. | ||
| 3. (Else if re-exported or inline module in current module: We continue with that module.) | ||
| 4. Else go to `current module path/ident.wesl` | ||
| * File found: We take that file as the current module. | ||
| * File not found: We assume an empty module as the current module, and continue with that. | ||
| * (Re-exporting changes the path.) | ||
|
|
@@ -121,10 +140,7 @@ Then, one iterates over each segment from left to right, and looks it up one by | |
| To get an absolute path to a module, one follows the algorithm above. In step 1, one takes the known absolute path of the `super` module, or the package. | ||
| The absolute path of the `super` module is always known, since the first loaded WESL file must always be the root module, and children are only discovered from there. | ||
|
|
||
| Once the import has been resolved, the last segment, or its alias, is brought into scope. | ||
|
|
||
| The order of the scopes is "user declarations and imported items > package names > predeclared items". | ||
| This lets WGSL add more predeclared items without breaking existing WESL code. Package names can shadow predeclared items, but we recommend that authors avoid doing that. | ||
| Once the import has been resolved, the last segment, or its `as` alias, is brought into scope. | ||
|
|
||
|
|
||
| ### Example | ||
|
|
@@ -245,6 +261,174 @@ const b = a + 1; | |
|
|
||
| Basic linker implementations do not need to check for this. Generating broken code and letting the underlying shader compiler throw an error is fine. | ||
|
|
||
| ## Wildcard imports | ||
|
|
||
| Wildcard imports bring all items from another module into the importing module's | ||
| scope. | ||
|
|
||
| Users can wildcard import: | ||
| - from any other module in the local package. | ||
| - from any external library module where the library author has added a | ||
| `@wildcardable` annotation. | ||
|
|
||
| Wildcard importing brings some stability risk when the imported module adds to | ||
| its API. The newly introduced names may conflict with other names in the | ||
|
k2d222 marked this conversation as resolved.
|
||
| importer's namespace (from local definitions and other imports), leading to | ||
| compiler warnings or errors. To help mitigate this risk, WESL provides a | ||
| `@wildcardable` annotation that library authors can place on modules that are | ||
| designed for wildcard importing. | ||
|
|
||
| Advanced users who wish to wildcard import from external modules not marked as | ||
| `@wildcardable` can do so by suppressing `wildcard_import` (see | ||
| [Suppressible diagnostics](#suppressible-diagnostics)). | ||
|
|
||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should probably start by saying something like "The following section applies when the current module has two or more wildcard imports. We specifically deal with the potential name clashes and corner cases. Users are not expected to run into this edge case. It exists to give us certain guarantees in the language itself. This will be useful, for example, to build semver checking tools."
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't see any mention of this "two wilcards rule" any more. Is this comment outdated? |
||
| ```wesl | ||
| // wildcard import from a @wildcardable external | ||
| import bevy::prelude::*; | ||
| import wgsl_test::expect::*; | ||
|
|
||
| // wildcard import from within the local package | ||
| import package::utils::*; | ||
| import super::fun::*; | ||
| ``` | ||
|
|
||
| ### `@wildcardable` annotation | ||
|
|
||
| Library authors mark modules they intend for library consumers to wildcard | ||
| import with `@wildcardable module;`: | ||
|
|
||
| ```wesl | ||
| // math.wesl (in a library) | ||
| @wildcardable module; | ||
|
|
||
| fn dot2(a: vec2f, b: vec2f) -> f32 { return a.x*b.x + a.y*b.y; } | ||
| fn cross2(a: vec2f, b: vec2f) -> f32 { return a.x*b.y - a.y*b.x; } | ||
| ``` | ||
|
|
||
| `@wildcardable` is an attribute on a module declaration (see [Grammar](#grammar)). | ||
| The module declaration must appear after any imports and before any global | ||
| declarations, and applies only to the module it appears in. | ||
|
|
||
| ### Recommendations for `@wildcardable` modules | ||
|
|
||
|
k2d222 marked this conversation as resolved.
|
||
| Because every name in a `@wildcardable` module is a potential collision in | ||
| importer code, library authors should curate these modules carefully. | ||
|
|
||
| **Add hesitantly.** Additions to a `@wildcardable` module are semver minor | ||
| version bumps but can break users who have local declarations or import other | ||
| `@wildcardable` modules. | ||
| - **Bundle** additions into a major release when one is upcoming. | ||
| - **Document** additions clearly in changelogs so downstream users debugging | ||
| unexpected name resolution can trace them. | ||
|
|
||
| **Compose with re-exports.** See [Re-exports](#re-exports) (TBD) to collect | ||
| items from other modules into a single `@wildcardable` module for user | ||
| convenience. | ||
|
|
||
| **Avoid generic names.** Prefer domain-specific names. Common names like | ||
| `Buffer`, `Config`, `Result`, `Vec`, etc. are more likely to collide with user | ||
| applications. | ||
|
|
||
| **Don't shadow WGSL builtins.** Names like `vec3`, `clamp`, `inverseSqrt` have | ||
| expected semantics that oughtn't be implicitly overridden with wildcards. | ||
| Similarly, avoid experimental Naga/Dawn/Safari builtins. | ||
| - WESL publishing tools should warn when a `@wildcardable` module exports an | ||
| item that shadows a WGSL builtin. Suppress with | ||
| `@diagnostic(off, builtin_shadow)` if the shadow is intentional. | ||
|
k2d222 marked this conversation as resolved.
Outdated
|
||
| - Consumers of the module will also see a `builtin_shadow` warning at the | ||
| import site. | ||
| - If a future WGSL update adds a conflicting builtin name, plan to update the | ||
| `@wildcardable` module to rename the conflicting item. | ||
|
|
||
| ### Library-to-library wildcard imports | ||
|
|
||
| Libraries that wildcard import from other libraries raise special concerns. If a | ||
| user's package manager chooses a newer version of the imported-from library, | ||
| the user may see a conflict in library code they don't expect to modify. | ||
|
|
||
| WESL library publishing tools address this by expanding wildcards to named | ||
| imports in the published version of the module: | ||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. (Hopefully we can easily implement this behaviour in wesl-rs. It might be a challenge, since Cargo defaults to "building from source on the end-user's machine". So there might be no pre-publish step without jumping through hoops.)
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In fact it doesn't have to actually be published with expanded wildcards, only processed as such. So yes, we can do that in wesl-rs because the wildcard resolver always knows if it comes from a user or library module.
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Wait, would that really work? I thought it wouldn't, because of this case:
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @k2d222 I think you'd once floated a publisher-side packaging step for WESL cargo crates (rather than building consumer-side). Is that likely? I guess that would help here. if cargo will follow semver and install a rev on the consumer side different than the rev the prelude publisher used, then things could break (because wildcards can break even on semver minor updates). So somebody on the app time runs cargo update and sees an upstream wesl crate fail to compile.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh, I got confused / forgot about this issue. Yeah it sounds like it has to be a pre-publish process and that's currently not the case. I'd need to think about how to achieve this. |
||
|
|
||
| ```wesl | ||
| // source | ||
| import bevy::prelude::*; | ||
| ``` | ||
|
|
||
| ```wesl | ||
| // published artifact | ||
| import bevy::prelude::{Color, Mesh, Transform, /* snapshot at publish time */}; | ||
| ``` | ||
|
|
||
| ## Import errors and warnings | ||
|
|
||
| WESL emits errors for name collisions and for external wildcards from unmarked | ||
| modules, and warnings for shadowing that could surprise readers. Genuine | ||
| collisions cannot be suppressed; other diagnostics are suppressible via | ||
| `@diagnostic`. | ||
|
|
||
| | Situation | Behavior | | ||
| | --- | --- | | ||
| | Local declaration conflicts with named import | Error | | ||
| | Named import conflicts with named import | Error | | ||
| | Wildcard import conflicts with wildcard import (when name is referenced) | Error | | ||
| | Wildcard import from a non-`@wildcardable` external module | Error (`wildcard_import`); suppressible | | ||
| | Local declaration or named import shadows a wildcard-imported name | Warning (`wildcard_shadow`); suppressible | | ||
| | Wildcard import shadows a WGSL builtin (when name is referenced) | Warning (`builtin_shadow`) on the import; suppressible | | ||
|
|
||
| When multiple wildcard imports are in scope, the same name may be exported by | ||
| more than one module. The potential conflict is dormant unless the name is | ||
| referenced; referencing it is an error: | ||
|
|
||
| ```wesl | ||
| import foo::*; // exports clashing_zap | ||
| import bar::*; // exports a different clashing_zap | ||
|
|
||
| fn main() { | ||
| let x = clashing_zap(); // error: ambiguous between foo::clashing_zap and bar::clashing_zap | ||
| } | ||
| ``` | ||
|
|
||
| The fix is to disambiguate with a named import (`import foo::clashing_zap;`) or | ||
| [inline module path](#inline-usage) (`foo::clashing_zap()`). | ||
|
|
||
| ### Suppressible diagnostics | ||
|
|
||
| - **`wildcard_import`** fires on a wildcard import from an external module not | ||
| marked `@wildcardable`. Suppress with `@diagnostic(off, wildcard_import)` on | ||
| the import statement to accept the upgrade risk that future versions of the | ||
| imported module may add conflicting names. | ||
|
|
||
| - **`wildcard_shadow`** fires when a local declaration or named import shadows a | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
where: on the local declaration I suppose?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. good question. I think you're right.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. clarified in the PR |
||
| name brought in by a wildcard import. The local wins by precedence (see | ||
| [Scope precedence](#scope-precedence)). | ||
|
|
||
| - **`builtin_shadow`** fires on a wildcard import when a referenced name in the | ||
| module resolves to a wildcard-imported item that shadows a WGSL builtin such | ||
| as `vec3` or `clamp`. Suppress at the import site if the override is | ||
| intentional; the suppression itself documents to readers that the builtins | ||
| have changed semantics. | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I find it slightly ackward that the consumer has to silence that warning, and not the wildcard module author. WGSL itself has no warning for shadowing built-ins, and I think it would make more sense if we added that warning in WESL, not just for wildcards, and it would be up to the wildcard module author to silence this. I would not push against consumers also needing the warning, I just find it not necessary. I would like defer this to a subsequent PR.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Works for me, can be be a separate PR.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. separated consumer side warning to PR #188 for us to consider at leisure. |
||
|
|
||
| ## Scope precedence | ||
|
|
||
| When a name could resolve to items at multiple precedence levels, the | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In what situation does this precedence rule occur?
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I think the last time this was brought up the counter was "what about associated items, like
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Agreed, just documenting it in the table. (Though see also @laundmo suggesting we make it a suppressible diagnostic later)
separable issue from this wildcard stuff?
Note that the proposal for now is to warn only
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. pushed a rev moving the builtin_shadow warning to client side, per @laundmo discussions |
||
| highest-precedence one wins. The table above describes whether such a resolution | ||
| is silent, produces a warning, or produces an error. | ||
|
|
||
| 1. user declarations | ||
| 2. named imports (non-wildcard) | ||
| 3. wildcard-imported names | ||
| 4. package names | ||
| 5. predeclared items (WGSL builtins) | ||
|
|
||
| Predeclared items rank lowest so that future WGSL spec revisions can add new | ||
| builtins without breaking existing shaders: any name already bound at a higher | ||
| level continues to resolve as before. Wildcard-imported names rank below user | ||
| declarations and named imports for the analogous reason: additions to a | ||
| `@wildcardable` module won't silently change resolution at call sites that | ||
| already have a local or named binding for the same name. Wildcard imports do not | ||
| bring in package names (only top-level declarations of the imported module), so | ||
| a wildcard cannot shadow a package. | ||
|
|
||
| ## Directives | ||
| Under discussion, see: <https://github.com/wgsl-tooling-wg/wesl-spec/issues/71> | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,95 @@ | ||
| # Imports Design | ||
|
|
||
| This document records design decisions behind WESL's import system. The | ||
| normative spec lives in [Imports.md](Imports.md). | ||
|
|
||
| ## Why the `@wildcardable module;` form? | ||
|
|
||
| `@wildcardable` is module-level metadata. WESL will likely want module-level | ||
| annotations for other module-scoped features, and libraries and users will want | ||
| a place to attach their own metadata to a whole module. A general-purpose syntax | ||
| for module metadata avoids inventing one ad-hoc form per feature. | ||
|
|
||
| The syntax pairs an attribute list with a `module` declaration: | ||
|
|
||
| - **`@`-prefixed attributes** match the existing item-level convention | ||
| (`@group`, `@binding`, `@if`, `@diagnostic`, ...). Module-level metadata uses | ||
| the same `@name` form, so users learn one annotation convention rather than | ||
| two, and the `@` always attaches to the element it precedes. | ||
| - **The `module` keyword** anchors the attributes to a declaration, but the | ||
| keyword is still usable for potential module-level features. For example, | ||
| syntax like `module(...)`, `module foo::bar`, or `module { ... }` is still | ||
| available for future use. | ||
|
|
||
| See [`@wildcardable` annotation](Imports.md#wildcardable-annotation) for the | ||
| normative spec. | ||
|
|
||
| ## Aren't wildcards an anti-pattern? | ||
|
|
||
| Many language communities discourage wildcard imports. TypeScript, Go, Zig, and | ||
| Carbon disallow wildcards entirely or restrict them to narrow cases. Java and | ||
| Rust permit them syntactically but discourage broad use by convention; Rust's | ||
| `prelude` modules are one curated pattern in that style. The general concerns | ||
| are practical: | ||
|
|
||
| - **Readability.** Direct imports make it obvious where a name comes from. | ||
|
k2d222 marked this conversation as resolved.
Outdated
|
||
| Wildcards push that work onto the reader, the language server, or the | ||
| compiler's name-resolution diagnostics. | ||
| - **API stability.** Adding a public item to a wildcard-imported module can | ||
| conflict with downstream declarations or with other wildcard imports. Stacked | ||
| wildcards across a dependency tree can create conflicts the end user neither | ||
| caused nor can easily fix. | ||
|
|
||
| The WESL environment adds further concerns: | ||
|
|
||
| - **Cross-ecosystem publishing.** WESL libraries can be published into multiple | ||
| host ecosystems (npm, crates, etc.), and the language's stability rules have | ||
| to work for all of them. npm in particular treats minor/patch breakage as an | ||
| upstream bug, so wildcard-driven conflicts on additive package updates would | ||
| be read there as buggy packages, not as users accepting a WESL-specific | ||
| tradeoff. The defaults can't be split per-ecosystem; even libraries that | ||
| aren't actively cross-published inherit the same rules. | ||
| - **Mixed-language ownership.** In host applications, dependency updates are | ||
| often routine maintenance handled by someone other than the shader author. A | ||
| wildcard conflict can land on a teammate who did not cause it and may not be | ||
| best positioned to fix shader-side breakage. | ||
| - **Shader test coverage.** Shader test coverage is often thinner than | ||
| application-code coverage, and some failures are visual or runtime-dependent. | ||
| Fewer tests and WGSL's comparatively small type system mean that wildcard | ||
| conflicts are less likely to be caught at the moment a dependency is updated. | ||
| - **Single namespace.** WGSL has a single namespace for types and values, and no | ||
| namespace construct or object-style surface to limit the scope of wildcarded | ||
| names after import. There are fewer places for names to coexist harmlessly. | ||
|
|
||
| These concerns motivate guardrails for WESL wildcard defaults. | ||
|
|
||
| ## Wildcards in WESL: when to allow, when to gate | ||
|
|
||
| WESL keeps wildcards available because some libraries are designed to feel | ||
| pervasive. Game engines, test frameworks, and math libraries expect a | ||
| domain-specific API where prefixing every call with `test::expect::` or similar | ||
| would obscure the shader rather than help it. Concise import syntax matters even | ||
| where an IDE can autocomplete: not every editor has a language server, and long | ||
| import blocks add noise regardless of how they were typed. | ||
|
|
||
| But the concerns in | ||
| [Aren't wildcards an anti-pattern?](#arent-wildcards-an-anti-pattern) still | ||
| apply, especially across package boundaries. WESL's defaults try to keep the | ||
| benefits while limiting the risk: | ||
|
|
||
| - **Not every public module suits wildcards.** Modules with a fast-growing API | ||
| or with generic names (`Buffer`, `Result`) are fine to import by name but | ||
| hazardous to wildcard. | ||
| - **Authors can signal which modules are curated for wildcards.** An explicit | ||
| `@wildcardable` marker lets library authors tell consumers (and tools) which | ||
| modules they've designed for wildcard use. It also gives tooling a hook for | ||
| lints around generic names, builtin shadowing, churn-prone additions, etc. | ||
| - **Defaults shape the ecosystem.** Red/yellow squiggles and linter messages | ||
| teach safe wildcard practice to new and part-time shader authors more reliably | ||
| than community blogs or documentation. | ||
| - **Advanced users are not blocked.** Within a package, wildcard imports are | ||
| unrestricted; externally, wildcard-importing a non-`@wildcardable` module is | ||
| possible via | ||
| [standard diagnostic controls](Imports.md#suppressible-diagnostics). The | ||
| default tunes the path of least resistance, but doesn't block users who | ||
| intentionally accept the risk. | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggest generalizing the example so it doesn't tie the feature to the
preludename. The directive (not the name) carries the meaning, e.g.: