direct import resolution#199
Conversation
- `@wildcardable` directive. publishers can mark any module not just `prelude` - `builtin_shadow` moves to publish time warning (and dropped -ing suffix) - write out scope precedence levels and error vs. warning cases. - add @stefnotch's wildcard expansion idea as future work work for publishing tool to protect against transitive version skew
just add sentence in the precedence section also drop the packag names can shadow line iit's clear enough from the table
to make clear that other names than 'prelude' are allowable
also add note that builtin_shadow client side check is only when referenced
- add `attribute*` to `import_statement` so `@diagnostic` is grammatical on imports - `module` becomes a reserved keyword
- rationale for @wildcardable syntax - rationale for @wildcardable design
and whether that's more readable depends on how many there are.
as in: `@!wildcardable;`
- rename diagnostic to cross_package_wildcard - rename diagnostic to unsuppored_wildcard - clarify suppression of builtin_shadow - clarify module level suppression generally - clarify that wildcards can be in import collections
replace the left-to-right segment walk with direct resolution:
the last path segment names the declared item, the preceding segments name the module.
declarations and modules naturally live in separate namespaces now
- `fn foo() {}` and `foo::bar();` do not conflict
lazy vs eager resolution is an implementation choice
clarify terms a bit
There was a problem hiding this comment.
Two main comments:
-
before, we had this condition:
File not found: We assume an empty module as the current module, and continue with that.
now idk what happens when importing an intermediate module path.
-
It's unclear who decides to use the filesystem storage or some other storage. It's unclear that the fs storage is normative, but the others are implementation-defined.
| * Any other first segment must name a known package, and anchors the path at | ||
| that package. Tools find the known packages in the | ||
| [`wesl.toml`](WeslToml.md) file or through the host package manager's | ||
| dependencies. |
There was a problem hiding this comment.
is the current package name importable? I forgot what we agreed on.
There was a problem hiding this comment.
There is no "current package name" in the spec at the moment. That should be a separate issue to deal with.
(Lygia wanted it after all.)
Maybe we'll do it with the same thing that was proposed here: #189 (comment) ?
There was a problem hiding this comment.
AFAIR we agreed it's importable. Fixing with #189 seems reasonable.
| An import collection imports multiple items, and allows for nested imports. | ||
|
|
||
| ### Import resolution algorithm | ||
| A wildcard import imports all top-level declarations from a module. Submodule names and submodule contents are not imported. A wildcard must follow a module path; a bare `import *;` is an error. A wildcard may also appear as a member of an import collection, applying to the module path before the braces (see [Import bindings](#import-bindings)). |
There was a problem hiding this comment.
It looks like the direct import resolution no longer warrants that intermediate submodule exist, i.e import lygia::math::abs is a valid module path, but import lygia::math is not. Is that the case?
There was a problem hiding this comment.
Yes, exactly. Intermediate submodules are no longer necessary.
There was a problem hiding this comment.
if so then there are some wording tweaks to do. For instance: import foo::bar::* is only valid if foo::bar is a full path to a module. But import foo::bar is valid even if foo::bar is only an intermediate path (not a valid module), because then I can do bar::baz::decl().
| [Resolving a declaration path](#resolving-a-declaration-path). | ||
| The root folder, or the root module, needs to be provided to the linker. This is currently a linker-specific API, and may change once we introduce a `wesl.toml`. | ||
|
|
||
| Linkers should fall back to `.wgsl` files when a `.wesl` file cannot be found. |
There was a problem hiding this comment.
can we move sec. rationale and alternatives here.
There was a problem hiding this comment.
I was sorely tempted, but seemed better as a separate PR.
| ## Filesystem Resolution | ||
| To resolve a module on a filesystem, one follows the algorithm above. | ||
| To resolve a module on a filesystem, one follows the mapping in | ||
| [Resolving a declaration path](#resolving-a-declaration-path). |
There was a problem hiding this comment.
not just that: we start with a filesystem path prefix provided by the linker or wesl.toml, we append the segments from the mapping above, and then we suffix the last segment with .wesl or .wgsl. That's the complete fs path.
Conflict resolution per 193-into-187 merge checklist: - keep the public import re-export paragraph in the grammar section; take direct-imports' module attribute wording - replace the old import walk algorithm with the new 'Resolving a declaration path' model from direct-imports - reattach public import re-exports to item lookup in the new resolution section - carry over the resolution-vs-visibility paragraph and the wildcard visibility-skip rule after the resolution rules - point Filesystem Resolution at the new resolution section - keep the Visibility.md re-exports link in the wildcardable recommendations Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- demote the "tools can enumerate resolutions" paragraph to a [!NOTE] - clarify intermediate paths don't need to be modules
See if it's more clear now on #199. 🤞 |
|
the conflicts between 199 and 197 make it hard to review. I say let't merge this and we can adjust the minute details in followup PRs. |
|
@k2d222 Am happy with that. Alternatively we could play around with rebasing. |
|
Okay, I tried out the rebase. Relatively straightforward with jj, but it would be a force push and it would affect the #187 pull request. Which, if I remember correctly, git does not like at all. So I vote in favour of |
fix for #193
replace the left-to-right segment walk with direct resolution:
the last path segment names the declared item, the preceding segments name the module.
declarations and modules naturally live in separate namespaces now
fn foo() {}andfoo::bar();do not conflictlazy vs eager resolution is an implementation choice
clarify terms a bit