document missing import restrictions#694
Conversation
There was a problem hiding this comment.
These comments are blocking because the PR currently adds only part of the upstream keyword-import model. In particular, the existing self and super legality rules still conflict with the new restriction paragraphs, so the FLS would remain internally inconsistent after merge. The remaining comments cover precision of the extern-prelude wording, preservation of the $crate distinction, and changelog classification.
> this is a requirement aimed at the programmer, not a fact of the conforming tool Co-authored-by: Hristian Kirtchev <60669983+kirtchev-adacore@users.noreply.github.com>
752f06e to
9c70941
Compare
|
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. |
Co-authored-by: Hristian Kirtchev <60669983+kirtchev-adacore@users.noreply.github.com>
Co-authored-by: Hristian Kirtchev <60669983+kirtchev-adacore@users.noreply.github.com>
There was a problem hiding this comment.
I rechecked this against rust-lang/rust#146972, rust-lang/reference#2136, the current Reference text, and the FLS path/import model. The PR is pointed at the right upstream change. I think there are a few alignment points left where the new restriction paragraphs and the older path/import rules would otherwise disagree.
The main thing I am trying to preserve is that the new restriction paragraphs line up with the surrounding path/import rules. The current Reference text and the post-rust-lang/rust#146972 resolver model also rely on the surrounding self, super, $crate, and :: path rules lining up with the import restrictions. The FLS currently still has older rules that conflict with the new paragraphs, especially around use self as name;, use self::{self};, braced parent-entity imports such as use a::b::{self as name}; and use a::{b::self as name};, and use self::super as name;.
The remaining alignment points I see are:
- Use the meeting outcome/Hristian follow-up wording: the :t:
entityimported by the related :t:simple importis subject to the :t:renaming, not the textual path segment. - Update the existing
selfimport rules so renamed current-module imports are accepted and unrenamed current-module imports remain rejected. - Update
fls_opn5n5t2mo3mfor Rust 2021 use paths soselfis still restricted in ordinary paths, while braced/nesting-import parent-entity imports such asuse a::b::{self};,use a::b::{self as name};,use a::{b::self};, anduse a::{b::self as name};remain valid. Direct final-selfpaths such asuse a::b::self as name;are still rejected by Rust 1.95.0 with E0429; rust-lang/rust#146972 leaves general support forselfat the end of paths as future work. - Update
fls_7k88ypcgaoffsosuperis permitted to follow an initialself, while still rejecting non-leadingsuperin paths such asfoo::superorcrate::super. - Preserve the existing FLS distinction between
crateand$crate;$crateresolves to the crate that declares the macro being expanded, not simply to the current crate in the same way ascrate. - Keep the Rust 2021
::rule narrow:use ::std::io;remains valid, while importing the extern-prelude root itself is not valid. The constructed import-prefix model should preserve leading::rather than treating a bare::prefix as an empty local prefix. - Avoid saying that a :t:
simple importbrings a path into scope. Paths are syntax; imports bring entities/names into scope. Also update the simple-import semantics for final-selfnesting imports, make empty non-global import prefixes resolve to the current module, preserve global::import prefixes, and update the existing underscore-renaming rule so it is not trait-specific. - The enum-variant-through-type-alias changelog classification is addressed by moving
fls_LV94x3HlpBWkto FLS corrections; the final changelog needs to account for the paragraph and glossary changes after this source-shape pass.
The changelog should account for IDs relative to the merge base rather than earlier PR revisions. With the source shape from the thread comments, fls_sUhnfV62HJrb, fls_QGdeRTe0H1Uc, fls_aam34hsRmKU2, fls_P6dFw89ZDKv2, and fls_61qKMG5tuv12 are new paragraphs; fls_opn5n5t2mo3m, fls_WAA4WmohGu6T, fls_2bkcn83smy2y, fls_iuzvtr3oax1o, fls_90hQvSh7Bfyg, fls_wRmvtgQkFA6w, fls_kz2Gij5wHXnl, fls_ar03D5rxjzy0, fls_iQOgxNihUEr7, fls_RUiFQ17bmRLt, and fls_7k88ypcgaoff are changed; fls_cw006jhlboa, fls_yY58pFpkig9o, fls_Pxc0Ts8Y7pfW, and fls_hv3xT2CjZuxc are removed; and the :t:simple import glossary entry is updated.
I trial-applied this source shape together, expanded the Rust 1.95.0 edition-2021 test matrix for braced final-self imports, and ./make.py --clear builds. The snippets should be copy-paste-ready as a combined patch.
I left the detailed suggestions in individual threads/comments; they are intended to be applied as one source shape.
| A :t:`simple import` is a :t:`use import` that brings all :t:`entities <entity>` | ||
| it refers to into scope, optionally with a different | ||
| :t:`name` than they are declared with by using a :t:`renaming`. | ||
| A :dt:`simple import` is a :t:`use import` that brings a :t:`simple path` into scope, optionally with a :t:`renaming`. |
There was a problem hiding this comment.
I think this definition should stay entity/name based. A path is syntax; it does not get brought into scope. A use declaration brings selected entities into scope, usually by creating local name bindings, and as changes the local name for the imported entity. It can also use as _, where the imported entity is added to scope without a name.
That distinction matters for the new keyword-import restrictions because the meeting discussion resolved the renaming target as the imported entity, not the path or path segment. It also matters for final-self nesting imports: use a::{b::self}; imports the parent entity selected by the constructed import prefix, not an entity resolved through a literal final self member.
Suggested wording for both this paragraph and the glossary entry:
A :dt:`simple import` is a :t:`use import` that brings
:t:`entities <entity>` selected by its :t:`simple import path`, or by its
:t:`import path prefix` when its :t:`simple path` ends in :t:`keyword` ``self``
and it appears in a :t:`nesting import`, into :t:`scope`, either under their
declared :t:`[name]s`, under a different :t:`name` by using a :t:`renaming`, or
without a :t:`name` by using a :t:`renaming` with character underscore ``_``.I think the empty-prefix and global-prefix cases should also be explicit. Rust 1.95.0 accepts both use {self as this_module}; and use {self::self as this_module};, where the prefix is local and empty. But use ::{self as root}; is different in Rust 2021: the prefix is the extern-prelude root and is rejected by the separate :: import restriction. I would update fls_WAA4WmohGu6T so empty local prefixes and global :: prefixes remain distinct:
:dp:`fls_WAA4WmohGu6T`
An :dt:`import path prefix` is the fully constructed :t:`path` prefix of a
:t:`use import`. An :t:`import path prefix` with no :t:`[path segment]s` and no
:t:`namespace qualifier` resolves to the current :t:`module`. An
:t:`import path prefix` that starts with :t:`namespace qualifier` ``::``
preserves that :t:`namespace qualifier` in the constructed prefix. An
:t:`import path prefix` for a given :t:`simple import` or :t:`glob import` is
constructed as follows:
#. :dp:`fls_IPYvldMqduf4`
Start the :t:`import path prefix` as follows:
* :dp:`fls_MOXId37fcNPY`
If the :t:`use import` is a :t:`simple import`, then start with the
:t:`simple import`'s :t:`simple path` :t:`path prefix`.
* :dp:`fls_2UyFcB6Our1v`
If the :t:`use import` is a :t:`glob import`, then start with the
:t:`glob import`'s :t:`simple path prefix`.
* :dp:`fls_irdKqoYzBM0M`
If the :t:`use import` is a :t:`nesting import`, then start with the
:t:`nesting import`'s :t:`simple path prefix`.
#. :dp:`fls_gAWsqibl4GLq`
Then if the current :t:`use import` is the child of a :t:`nesting import`,
prepend the :t:`nesting import`'s :t:`simple path prefix` to the
:t:`import path prefix`. Repeat this step with the :t:`nesting import` as
the current :t:`use import`.Then I would update fls_wRmvtgQkFA6w so final-self simple imports inside brace syntax import the parent entity selected by the fully constructed prefix:
:dp:`fls_wRmvtgQkFA6w`
A :t:`simple import` brings :t:`[name]s` into :t:`scope` as follows:
* :dp:`fls_kz2Gij5wHXnl`
If the :t:`simple import` appears in a :t:`nesting import` and the last
:t:`path segment` of its :t:`simple path` is expressed as :t:`keyword`
``self``, then bring the :t:`entity` in :t:`type namespace` that the
:t:`import path prefix` resolves to into :t:`scope`.
* :dp:`fls_ar03D5rxjzy0`
If the :t:`simple path` is :t:`keyword` ``self``, then bring the containing
:t:`module` into :t:`scope`.
* :dp:`fls_ce73bg0BqV1X`
Otherwise bring all :t:`entities <entity>` that the :t:`simple import path`
resolves to that are visible from the location of the
:t:`simple import` into :t:`scope`.This is the semantic counterpart to the path-position rule in the separate self path thread. It keeps use a::b::{self};, use a::{b::self};, and use ::std::{vec::self as std_vec}; valid, while direct final-self paths such as use a::b::self as name; remain rejected by the path-position rule.
I would also update fls_iQOgxNihUEr7 so the underscore-import rule matches this definition:
:dp:`fls_iQOgxNihUEr7`
An :t:`entity` imported by a :t:`simple import` subject to a :t:`renaming` with
character underscore ``_`` is added into :t:`scope` without a :t:`name`.The changelog should list fls_WAA4WmohGu6T, fls_2bkcn83smy2y, fls_wRmvtgQkFA6w, fls_kz2Gij5wHXnl, fls_ar03D5rxjzy0, and fls_iQOgxNihUEr7 as changed; fls_yY58pFpkig9o as removed; and the :t:simple import glossary entry as updated. No extra paragraph ID should be needed if the :: prefix preservation wording stays folded into fls_WAA4WmohGu6T.
There was a problem hiding this comment.
56129b0 is for "simple import"... I removed the renaming thing since it already is covered by fls_FILuR3pfwjw3 and fls_iQOgxNihUEr7
There was a problem hiding this comment.
-
does this text refer to a
use {segment}:An import path prefix with no path segments and no namespace qualifier resolves to the current module -
in
An import path prefix that starts with namespace qualifier :: preserves that namespace qualifier in the constructed prefix, what does "preserve" mean
as a bonus question, what is a "constructed prefix"
There was a problem hiding this comment.
e3a53f6 takes the suggestion that tightens terminology, where we now use "path segment" term more (in this pr), and flattens the list (to 1 level deep instead of 2 levels)
There was a problem hiding this comment.
Yes, the idea is that the sentence is meant to cover imports with no prefix before the braces. For example:
use {segment as segment_alias};
use {self as this_module};In those cases, the import path prefix has no path segments and no leading ::, so it is a local empty prefix and resolves to the current module.
That is the same prefix shape as use {segment};; the alias just avoids the unrelated duplicate-name error you can get when re-importing a module under its existing name.
The contrast is:
use ::{self as root};That also has no path segments before self, but it does have the :: namespace qualifier. So it is not the empty local prefix; it is the global extern-prelude root, which Rust rejects importing directly.
On terminology: import path prefix is already a defined FLS term here. constructed prefix was just shorthand on my side for the result of constructing the import path prefix; we can avoid that phrase in final wording.
By "preserve", I meant: do not drop the leading :: while constructing the import path prefix.
This matters because :: is a namespace qualifier, not a path segment. If the construction only carries over path segments, ::std can accidentally become indistinguishable from local std in the FLS model.
Clearer wording than "preserves" would be:
If the :t:`simple path prefix` used to construct an :t:`import path prefix`
starts with :t:`namespace qualifier` ``::``, then the resulting
:t:`import path prefix` starts with :t:`namespace qualifier` ``::``.There was a problem hiding this comment.
I don't see the braces in use {segment} as semantically meaningful, and that makes me hesitate to consider that import as having an empty import path prefix. Similarly, the braces of use {{{segment}}} are also not semantically meaningful (else we would have to consider that import as having 3 import path prefixes).
As an aside, in use a::{b, c::d}, would you consider a to be the only path prefix, or would you consider that to include a.c? What about use a.b.c... is the path prefix a or a.b?
This matches fls_FILuR3pfwjw3, where the term "entity" is used to refer to the thing brought into scope.
Co-authored-by: Hristian Kirtchev <60669983+kirtchev-adacore@users.noreply.github.com>
Co-authored-by: Hristian Kirtchev <60669983+kirtchev-adacore@users.noreply.github.com>
Here we are avoiding "simple path" in favor of "path segment"
It's not just traits that are affected
Co-authored-by: Hristian Kirtchev <60669983+kirtchev-adacore@users.noreply.github.com>
Co-authored-by: Hristian Kirtchev <60669983+kirtchev-adacore@users.noreply.github.com>
Co-authored-by: Hristian Kirtchev <60669983+kirtchev-adacore@users.noreply.github.com>
| A :t:`simple import` is a :t:`use import` that brings all :t:`entities <entity>` | ||
| it refers to into scope, optionally with a different | ||
| :t:`name` than they are declared with by using a :t:`renaming`. | ||
| A :dt:`simple import` is a :t:`use import` that brings a :t:`simple path` into scope, optionally with a :t:`renaming`. |
There was a problem hiding this comment.
Yes, the idea is that the sentence is meant to cover imports with no prefix before the braces. For example:
use {segment as segment_alias};
use {self as this_module};In those cases, the import path prefix has no path segments and no leading ::, so it is a local empty prefix and resolves to the current module.
That is the same prefix shape as use {segment};; the alias just avoids the unrelated duplicate-name error you can get when re-importing a module under its existing name.
The contrast is:
use ::{self as root};That also has no path segments before self, but it does have the :: namespace qualifier. So it is not the empty local prefix; it is the global extern-prelude root, which Rust rejects importing directly.
On terminology: import path prefix is already a defined FLS term here. constructed prefix was just shorthand on my side for the result of constructing the import path prefix; we can avoid that phrase in final wording.
By "preserve", I meant: do not drop the leading :: while constructing the import path prefix.
This matters because :: is a namespace qualifier, not a path segment. If the construction only carries over path segments, ::std can accidentally become indistinguishable from local std in the FLS model.
Clearer wording than "preserves" would be:
If the :t:`simple path prefix` used to construct an :t:`import path prefix`
starts with :t:`namespace qualifier` ``::``, then the resulting
:t:`import path prefix` starts with :t:`namespace qualifier` ``::``.| the :t:`visibility` of the :t:`name` is the most permissive one. | ||
|
|
||
| :dp:`fls_RUiFQ17bmRLt` | ||
| A :t:`simple import` with a single :t:`path segment` expressed as :t:`keyword` ``self`` shall be subject to a :t:`renaming`. |
There was a problem hiding this comment.
This seems pretty close, but I think it still misses one current-module import spelling.
The rule now catches single-segment forms like:
use self;
use {self};But rustc also rejects this unrenamed current-module import:
use {self::self};That path has two self segments, so it is not covered by "a single path segment expressed as self". The restriction should likely be made about importing the current module without a renaming, not only about a single textual segment.
Suggested shape:
When a :t:`path segment` expressed as :t:`keyword` ``self`` is used to import
the current :t:`module`, the imported :t:`entity` shall be subject to a
:t:`renaming`.This still allows parent-entity imports like use a::b::{self};, because those import a::b, not the current module.
| When a :t:`path segment` expressed as :t:`keyword` ``super`` is used to import a parent :t:`module`, the imported :t:`entity` shall be subject to a :t:`renaming`. | ||
|
|
||
| :dp:`fls_aam34hsRmKU2` | ||
| A :t:`global path` where any of its :t:`[path segment]s` are expressed as any of the :t:`[keyword]s`, ``self``, ``super``, ``crate``, and ``$crate``, shall not be used. |
There was a problem hiding this comment.
I think this rule might be a bit broad now.
It should reject importing the bare global/extern-prelude root:
use ::{self as root}; // invalidBut it should not reject every global path that later contains self. This remains valid because std is the extern-prelude crate and the final self is the nesting-import shorthand for std::vec:
use ::std::{vec::self as std_vec}; // validfls_P6dFw89ZDKv2 already covers the general rule that a Rust 2021 global path starts with an extern-prelude crate name. I think this paragraph should stay narrow and cover only the bare-root import case, for example:
A :t:`simple import` whose :t:`import path prefix` consists only of
:t:`namespace qualifier` ``::`` and whose :t:`simple path` consists of a single
:t:`path segment` expressed as :t:`keyword` ``self`` shall not be used.By the way, I am not trying to drop the crate / super / $crate cases. Those should fall out of the general global-path rule (fls_P6dFw89ZDKv2): after leading ::, the first path segment must be an extern-prelude crate identifier, so ::crate, ::super, ::self, and ::$crate are already rejected. This paragraph seems to only need to cover the separate final-self nesting-import case where the prefix is bare ::, as in use ::{self as root};.
| :t:`entity` in :t:`type namespace` that the :t:`import path prefix` resolves | ||
| to into :t:`scope`. | ||
| * :dp:`fls_yY58pFpkig9o` | ||
| If the :t:`simple import` appears in a :t:`nesting import`, and the last :t:`path segment` of its :t:`simple path` is expressed as :t:`keyword` ``self``, then the :t:`simple import` brings the :t:`entity` that the :t:`import path prefix` resolves to in :t:`scope` of the :t:`type namespace`. |
There was a problem hiding this comment.
The intended semantics are right, but this seems a bit hard to parse for me when it says the entity is brought "in scope of the type namespace".
I think it might read a little cleaner if we said that the prefix resolves in the type namespace, and the resulting entity is brought into scope. Something like the below, maybe.
If the :t:`simple import` appears in a :t:`nesting import` and the last
:t:`path segment` of its :t:`simple path` is expressed as :t:`keyword`
``self``, then the :t:`simple import` brings the :t:`entity` in
:t:`type namespace` that the :t:`import path prefix` resolves to into
:t:`scope`.Example:
use a::b::{self as b_alias};Here final self imports the entity selected by prefix a::b, not a child item named self.
515179e to
4332f41
Compare
No description provided.