[move 2024] Combine namespaces for expansion lookup #14978
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
4 Ignored Deployments
|
cgswords
force-pushed
the
cgswords/combined_namespaces
branch
from
606884a to
688ba59
Compare
…ess table, ignoring aliases
cgswords
force-pushed
the
cgswords/combined_namespaces
branch
from
688ba59 to
37c9520
Compare
cgswords
changed the title
| } | ||
|
|
||
| // Parse the beginning of an access, either an address or an identifier with a specific description | ||
| fn parse_leading_name_access_<'a, F: FnOnce() -> &'a str>( | ||
| context: &mut Context, | ||
| global_name: bool, |
There was a problem hiding this comment.
Maybe we should make the variant GlobalName instead of AddressName then? I think either is fine really, the name GlobalName entered my head as an alternative, and then I saw it in the flag here :)
There was a problem hiding this comment.
I will split the difference and call it a GlobalAddress.
| } | ||
| } else if let Some(previous) = self.previous.as_mut() { |
There was a problem hiding this comment.
- We might want to make this a loop because... Rust 😡
- Have you observed a noticeable perf difference with this? Probably not since this is likely... what? like 5 lists deep?
- How will this work with locals? Will I have to push a new scope on at each
letbinding?
There was a problem hiding this comment.
- We might want to make this a loop because... Rust 😡
Ah, sure thing.
- Have you observed a noticeable perf difference with this? Probably not since this is likely... what? like 5 lists deep?
First, yeah, it never gets too deep. Second, the previous implementation cloned the whole map every time it pushed a new scope, so I think this will still be a perf win. Third, if it is ever slow we can do path compression: if we ever look something up in a previous, we can update all the intermediary tables to also hold that information, amortizing it to O(1) lookup (at the cost of space).
- How will this work with locals? Will I have to push a new scope on at each let binding?
You will only need to push scopes when entering new blocks, because any lets (or uses) in that block should "fall out" of scope on exit. (Technically, we only need to do this for blocks that have let bindings or use statements in them.) Otherwise, a let should shadow the previous definition, so we just add it to the innermost scope (replacing anything already there).
| //************************************************************************************************** | ||
| // Context | ||
| //************************************************************************************************** | ||
|
|
||
| type ModuleMembers = BTreeMap<Name, ModuleMemberKind>; | ||
|
|
||
| struct Context<'env, 'map> { | ||
| module_members: UniqueMap<ModuleIdent, ModuleMembers>, | ||
| // NB: We carry a few things separately because we need to split them out during path resolution to |
There was a problem hiding this comment.
| top_level_address( | ||
| &mut context.defn_context, | ||
| /* suggest_declaration */ true, | ||
| a, | ||
| ), |
There was a problem hiding this comment.
nit, maybe bind this to a local to reduce some amount of indenting
## Description This adds a Move 2024 path resolution mechanism to expansion, which revises name chain resolution to combine address, module, and member namespaces. In addition, it adds `::<addr>::<module>` and `::<addr>::<module>::<member>` forms to alias-dodge when necessary. This is toward getting ready to add enums to the name chain space. ## Test Plan New and updated tests, subject to feature gating. --- If your changes are not user-facing and not a breaking change, you can skip the following section. Otherwise, please indicate what changed, and then add to the Release Notes section as highlighted during the release process. ### Type of Change (Check all that apply) - [ ] protocol change - [ ] user-visible impact - [ ] breaking change for a client SDKs - [ ] breaking change for FNs (FN binary must upgrade) - [ ] breaking change for validators or node operators (must upgrade binaries) - [ ] breaking change for on-chain data layout - [ ] necessitate either a data wipe or data migration ### Release notes
Description
This adds a Move 2024 path resolution mechanism to expansion, which revises name chain resolution to combine address, module, and member namespaces. In addition, it adds
::<addr>::<module>and::<addr>::<module>::<member>forms to alias-dodge when necessary.This is toward getting ready to add enums to the name chain space.
Test Plan
New and updated tests, subject to feature gating.
If your changes are not user-facing and not a breaking change, you can skip the following section. Otherwise, please indicate what changed, and then add to the Release Notes section as highlighted during the release process.
Type of Change (Check all that apply)
Release notes