-
-
Notifications
You must be signed in to change notification settings - Fork 14.3k
resolve: Rename "name bindings" to "name declarations" #150521
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
Conversation
|
r? @nnethercote rustbot has assigned @nnethercote. Use |
|
There are some cases in which I'm not sure in the naming. E.g. what if something contains a name declaration that is known to always be a definition, should we call it "decl" or "def". Maybe if it contains the |
I read over the commits and they seem fine to me, but I don't have any specific knowledge/opinions about all of this stuff. @yaahc: you have been looking closely at this stuff lately. Any thoughts? |
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
I don't necessarily object to using the underlying types as a guide for which names to use in which case but that isn't how I'd think about it by default. Noting that I haven't spent a ton of time thinking about this and this response is off the cuff fresh out of vacation, my default would be to try to define what exactly a decl vs a def is in terms of the language, e.g. approximately how these would fit into the reference, then apply the names as appropriate in the compiler. Looking at the reference we don't seem to have any consistency and seem to use all of these terms interchangeably and untangling them may be infeasible at this point:
I guess my current inclination is to just decide that "decl" is sort of a superset that includes defs and bindings, accept this change as a nice clarification where decl is more intuitive, and not worry about trying to eliminate all usages of binding at the moment, if we want to try to standardize that language we can but I think that's a bigger conversation and not necessarily high priority. |
|
Ok, this seems like an improvement overall. r=me once the conflicts are fixed. @rustbot author |
|
Reminder, once the PR becomes ready for a review, use |
Also, rename `DeclKind::Res` to `DeclKind::Def`.
`MacroRulesBinding` -> `MacroRulesDef`
|
This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
|
Rebased and |
resolve: Rename "name bindings" to "name declarations" This was discussed previously in some name resolution specification threads on zulip. Link: [#t-compiler/help > understanding early name resolution of imports @ 💬](https://rust-lang.zulipchat.com/#narrow/channel/182449-t-compiler.2Fhelp/topic/understanding.20early.20name.20resolution.20of.20imports/near/545306658) The main change is that `NameBinding` is renamed to `Decl`. The `Name` part is skipped because almost everything in `rustc_resolve` is about names. So in general the naming looks more compact now (`binding: NameBinding` -> `decl: Decl`). I didn't rename *everything* including all the local variables, but did rename most of significant interfaces. Late resolution in particular uses "bindings" in slightly different meanings, I didn't touch that. Also, some interface comments are added/improved. Renaming list: - fn `define_binding_local` -> `plant_decl_into_local_module` - fn `add_macro_use_binding` -> `add_macro_use_decl` - fn `binding_description` -> `decl_description` - enum `PendingBinding` -> `PendingDecl` - field `ImportKind::Single::bindings` -> `ImportKind::Single::decls` - field `NameResolution::non_glob_binding` -> `NameResolution::non_glob_decl` - field `NameResolution::glob_binding` -> `NameResolution::glob_decl` - fn `best_binding` -> `best_decl` - fn `import` -> `new_import_decl` - fn `try_define_local` -> `try_plant_decl_into_local_module` - fn `new_ambiguity_binding` -> `new_decl_with_ambiguity` - fn `new_warn_ambiguity_binding` -> `new_decl_with_warn_ambiguity` - field `UnnecessaryQualification::binding` -> `UnnecessaryQualification::decl` - struct `MacroRulesBinding` -> `MacroRulesDef` - field `MacroRulesBinding::binding` -> `MacroRulesDef::decl` - variant `MacroRulesScope::Binding` -> `MacroRulesScope::Def` - enum `LexicalScopeBinding` -> `LateDecl` - variant `LexicalScopeBinding::Item` -> `LateDecl::Decl` - variant `LexicalScopeBinding::Res` -> `LateDecl::RibDef` - field `ModuleData::self_binding` -> `ModuleData::self_decl` - struct `NameBindingData` -> `DeclData` - type `NameBinding` -> `Decl` - enum `NameBindingKind` -> `DeclKind` - variant `NameBindingKind::Res` -> `DeclKind::Def` - field `NameBindingKind::Import::binding` -> `DeclKind::Import::source_decl` - field `PrivacyError::binding` -> `PrivacyError::decl` - field `ExternPreludeEntry::item_binding` -> `ExternPreludeEntry::item_decl` - field `ExternPreludeEntry::flag_binding` -> `ExternPreludeEntry::flag_decl` - field `Resolver::binding_parent_modules` -> `Resolver::decl_parent_modules` - field `Resolver::dummy_binding` -> `Resolver::dummy_decl` - field `Resolver::builtin_types_bindings` -> `Resolver::builtin_type_decls` - field `Resolver::builtin_attrs_bindings` -> `Resolver::builtin_attr_decls` - field `Resolver::registered_tool_bindings` -> `Resolver::registered_tool_decls` - fn `ResolverArenas::new_res_binding` -> `ResolverArenas::new_def_decl` - fn `ResolverArenas::new_pub_res_binding` -> `ResolverArenas::new_pub_def_decl` - fn `ResolverArenas::alloc_name_binding` -> `ResolverArenas::alloc_decl` - fn `ResolverArenas::alloc_macro_rules_binding` -> `ResolverArenas::alloc_macro_rules_def` - fn `set_binding_parent_module` -> `set_decl_parent_module`
Rollup of 4 pull requests Successful merges: - #150026 (Fix macro_metavar_expr_concat behavior with nested repetitions) - #150521 (resolve: Rename "name bindings" to "name declarations") - #150704 (MGCA: Const constructors support) - #150728 (Cleanup some ui tests for const-traits) r? `@ghost` `@rustbot` modify labels: rollup
Rollup of 4 pull requests Successful merges: - #150026 (Fix macro_metavar_expr_concat behavior with nested repetitions) - #150521 (resolve: Rename "name bindings" to "name declarations") - #150704 (MGCA: Const constructors support) - #150728 (Cleanup some ui tests for const-traits) r? `@ghost` `@rustbot` modify labels: rollup
Rollup merge of #150521 - petrochenkov:decl, r=nnethercote resolve: Rename "name bindings" to "name declarations" This was discussed previously in some name resolution specification threads on zulip. Link: [#t-compiler/help > understanding early name resolution of imports @ 💬](https://rust-lang.zulipchat.com/#narrow/channel/182449-t-compiler.2Fhelp/topic/understanding.20early.20name.20resolution.20of.20imports/near/545306658) The main change is that `NameBinding` is renamed to `Decl`. The `Name` part is skipped because almost everything in `rustc_resolve` is about names. So in general the naming looks more compact now (`binding: NameBinding` -> `decl: Decl`). I didn't rename *everything* including all the local variables, but did rename most of significant interfaces. Late resolution in particular uses "bindings" in slightly different meanings, I didn't touch that. Also, some interface comments are added/improved. Renaming list: - fn `define_binding_local` -> `plant_decl_into_local_module` - fn `add_macro_use_binding` -> `add_macro_use_decl` - fn `binding_description` -> `decl_description` - enum `PendingBinding` -> `PendingDecl` - field `ImportKind::Single::bindings` -> `ImportKind::Single::decls` - field `NameResolution::non_glob_binding` -> `NameResolution::non_glob_decl` - field `NameResolution::glob_binding` -> `NameResolution::glob_decl` - fn `best_binding` -> `best_decl` - fn `import` -> `new_import_decl` - fn `try_define_local` -> `try_plant_decl_into_local_module` - fn `new_ambiguity_binding` -> `new_decl_with_ambiguity` - fn `new_warn_ambiguity_binding` -> `new_decl_with_warn_ambiguity` - field `UnnecessaryQualification::binding` -> `UnnecessaryQualification::decl` - struct `MacroRulesBinding` -> `MacroRulesDef` - field `MacroRulesBinding::binding` -> `MacroRulesDef::decl` - variant `MacroRulesScope::Binding` -> `MacroRulesScope::Def` - enum `LexicalScopeBinding` -> `LateDecl` - variant `LexicalScopeBinding::Item` -> `LateDecl::Decl` - variant `LexicalScopeBinding::Res` -> `LateDecl::RibDef` - field `ModuleData::self_binding` -> `ModuleData::self_decl` - struct `NameBindingData` -> `DeclData` - type `NameBinding` -> `Decl` - enum `NameBindingKind` -> `DeclKind` - variant `NameBindingKind::Res` -> `DeclKind::Def` - field `NameBindingKind::Import::binding` -> `DeclKind::Import::source_decl` - field `PrivacyError::binding` -> `PrivacyError::decl` - field `ExternPreludeEntry::item_binding` -> `ExternPreludeEntry::item_decl` - field `ExternPreludeEntry::flag_binding` -> `ExternPreludeEntry::flag_decl` - field `Resolver::binding_parent_modules` -> `Resolver::decl_parent_modules` - field `Resolver::dummy_binding` -> `Resolver::dummy_decl` - field `Resolver::builtin_types_bindings` -> `Resolver::builtin_type_decls` - field `Resolver::builtin_attrs_bindings` -> `Resolver::builtin_attr_decls` - field `Resolver::registered_tool_bindings` -> `Resolver::registered_tool_decls` - fn `ResolverArenas::new_res_binding` -> `ResolverArenas::new_def_decl` - fn `ResolverArenas::new_pub_res_binding` -> `ResolverArenas::new_pub_def_decl` - fn `ResolverArenas::alloc_name_binding` -> `ResolverArenas::alloc_decl` - fn `ResolverArenas::alloc_macro_rules_binding` -> `ResolverArenas::alloc_macro_rules_def` - fn `set_binding_parent_module` -> `set_decl_parent_module`
This was discussed previously in some name resolution specification threads on zulip.
Link: #t-compiler/help > understanding early name resolution of imports @ 💬
The main change is that
NameBindingis renamed toDecl.The
Namepart is skipped because almost everything inrustc_resolveis about names.So in general the naming looks more compact now (
binding: NameBinding->decl: Decl).I didn't rename everything including all the local variables, but did rename most of significant interfaces.
Late resolution in particular uses "bindings" in slightly different meanings, I didn't touch that.
Also, some interface comments are added/improved.
Renaming list:
define_binding_local->plant_decl_into_local_moduleadd_macro_use_binding->add_macro_use_declbinding_description->decl_descriptionPendingBinding->PendingDeclImportKind::Single::bindings->ImportKind::Single::declsNameResolution::non_glob_binding->NameResolution::non_glob_declNameResolution::glob_binding->NameResolution::glob_declbest_binding->best_declimport->new_import_decltry_define_local->try_plant_decl_into_local_modulenew_ambiguity_binding->new_decl_with_ambiguitynew_warn_ambiguity_binding->new_decl_with_warn_ambiguityUnnecessaryQualification::binding->UnnecessaryQualification::declMacroRulesBinding->MacroRulesDefMacroRulesBinding::binding->MacroRulesDef::declMacroRulesScope::Binding->MacroRulesScope::DefLexicalScopeBinding->LateDeclLexicalScopeBinding::Item->LateDecl::DeclLexicalScopeBinding::Res->LateDecl::RibDefModuleData::self_binding->ModuleData::self_declNameBindingData->DeclDataNameBinding->DeclNameBindingKind->DeclKindNameBindingKind::Res->DeclKind::DefNameBindingKind::Import::binding->DeclKind::Import::source_declPrivacyError::binding->PrivacyError::declExternPreludeEntry::item_binding->ExternPreludeEntry::item_declExternPreludeEntry::flag_binding->ExternPreludeEntry::flag_declResolver::binding_parent_modules->Resolver::decl_parent_modulesResolver::dummy_binding->Resolver::dummy_declResolver::builtin_types_bindings->Resolver::builtin_type_declsResolver::builtin_attrs_bindings->Resolver::builtin_attr_declsResolver::registered_tool_bindings->Resolver::registered_tool_declsResolverArenas::new_res_binding->ResolverArenas::new_def_declResolverArenas::new_pub_res_binding->ResolverArenas::new_pub_def_declResolverArenas::alloc_name_binding->ResolverArenas::alloc_declResolverArenas::alloc_macro_rules_binding->ResolverArenas::alloc_macro_rules_defset_binding_parent_module->set_decl_parent_module