Skip to content
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

Strange behaviour while resoving modules placed in constants #54525

Closed
weiznich opened this issue Sep 24, 2018 · 3 comments
Closed

Strange behaviour while resoving modules placed in constants #54525

weiznich opened this issue Sep 24, 2018 · 3 comments

Comments

@weiznich
Copy link
Contributor

There is a strange/inconsistent behaviour in the name resolution of models placed in constants.
In detail placing a module inside of a constant works.
Referring directly to types inside the module also works.

const _DUMMY: () = {
    mod some_mod {
        pub struct Bar;
    }
    
    fn foo(_: some_mod::Bar) {}
};

compiles successfully.

Trying to use use statements or starting the path otherwise with self:: results in compiler errors that some_mod is not found:

const _DUMMY: () = {
    mod some_mod {
        pub struct Bar;
    }
    // This fails
    use self::some_mod::Bar;    

    // This also fails
    fn foo(_: self::some_mod::Bar) {}
};

This behaviour seems inconsistent for me. Either it should not be allowed to place modules inside of constants, or it should be possible to use relative paths to reference items inside of such modules (in scope of the constant block).

I would guess that the scope of self is not associated with the inner scope of the constant, but with the current module. From there the module inside of the constant is not visible.

This happens on all channels.

Playground links:

@csmoe
Copy link
Member

csmoe commented Sep 24, 2018

Possibly Duplicate of #18084

@Havvy
Copy link
Contributor

Havvy commented Sep 25, 2018

self refers to the module that the path is in, not the scope it is in.

Personally, I think of the place of that module in the constant to be self::_DUMMY::$0::some_mod and we cannot refer to $n paths.

This is definitely also a duplicate of that issue.

bors added a commit that referenced this issue Nov 14, 2018
[beta] resolve: Implement uniform paths 2.0

With this PR paths in imports on 2018 edition are resolved as relative in the scope in which they are written, similarly to any other paths.
The previous implementation worked differently - it tried to resolve the import in the current module (`self::import`) and in the "crate universe" (`::import`), and then merge these two resolutions if possible.

The difference is that the new scheme can refer to strictly larger set of names in scope - names from unnamed blocks, names from all kinds of preludes, including macros imported with `#[macro_use] extern crate`, built-in types/macros, macros introduced with `macro_rules`.
This means strictly more potential ambiguities and therefore ambiguity errors, since we keep the rule that any two different candidate names in scope conflict with each other during import resolution. So this is a breaking change for 2018 edition, but it should be relatively minor.

All paths that don't start with an extern crate are also gated with the `uniform_paths` feature, paths that refer to extern crates are not gated (so we effectively get something like "future-proofed anchored paths" on stable).

Another difference is treatment of paths in visibilities (`pub(in path)`). Few people remember about paths in visibilities, so before this PR they still used the old 2015 rules even on 2018 edition. Namely, paths in visibilities were crate-relative, analogous to 2015 edition imports.
This PR resolves paths in visibilities as uniform as well, or rather future proofs them in this direction.
Paths in visibilities are restricted to parent modules, so relative paths almost never make sense there, and `pub(in a)` needs to be rewritten as `pub(in crate::a)` in the uniform scheme, de-facto cementing the discouraged status of non-`pub(crate)` and non-`pub(super)` fine-grained visibilities.
This is clearly a breaking change for 2018 edition as well, but also a minor one.

The in-scope resolution strategy for import paths mirrors what is currently done for macro paths on stable (on both editions), so it will continue working even if the "ambiguity always means error" restriction is relaxed in the future.

This PR also significantly improves diagnostics for all kinds of resolution ambiguities, from the newly introduced import ones to pretty old "glob vs glob" conflicts. (That's probably what I've spent most of the time on.)

Why beta:
- This is a breaking change on 2018 edition.
- This is a large PR, it's less risky to forward-port it to nightly, than back-port to beta.

cc #55618
cc #53130
cc rust-lang/rfcs#1289
Closes #18084
Closes #54525
Fixes #54390
Fixes #55668

r? @ghost
bors added a commit that referenced this issue Nov 18, 2018
[beta] resolve: Implement uniform paths 2.0

With this PR paths in imports on 2018 edition are resolved as relative in the scope in which they are written, similarly to any other paths.
The previous implementation worked differently - it tried to resolve the import in the current module (`self::import`) and in the "crate universe" (`::import`), and then merge these two resolutions if possible.

The difference is that the new scheme can refer to strictly larger set of names in scope - names from unnamed blocks, names from all kinds of preludes, including macros imported with `#[macro_use] extern crate`, built-in types/macros, macros introduced with `macro_rules`.
This means strictly more potential ambiguities and therefore ambiguity errors, since we keep the rule that any two different candidate names in scope conflict with each other during import resolution. So this is a breaking change for 2018 edition, but it should be relatively minor.

All paths that don't start with an extern crate are also gated with the `uniform_paths` feature, paths that refer to extern crates are not gated (so we effectively get something like "future-proofed anchored paths" on stable).

Another difference is treatment of paths in visibilities (`pub(in path)`). Few people remember about paths in visibilities, so before this PR they still used the old 2015 rules even on 2018 edition. Namely, paths in visibilities were crate-relative, analogous to 2015 edition imports.
This PR resolves paths in visibilities as uniform as well, or rather future proofs them in this direction.
Paths in visibilities are restricted to parent modules, so relative paths almost never make sense there, and `pub(in a)` needs to be rewritten as `pub(in crate::a)` in the uniform scheme, de-facto cementing the discouraged status of non-`pub(crate)` and non-`pub(super)` fine-grained visibilities.
This is clearly a breaking change for 2018 edition as well, but also a minor one.

The in-scope resolution strategy for import paths mirrors what is currently done for macro paths on stable (on both editions), so it will continue working even if the "ambiguity always means error" restriction is relaxed in the future.

This PR also significantly improves diagnostics for all kinds of resolution ambiguities, from the newly introduced import ones to pretty old "glob vs glob" conflicts. (That's probably what I've spent most of the time on.)

Why beta:
- This is a breaking change on 2018 edition.
- This is a large PR, it's less risky to forward-port it to nightly, than back-port to beta.

cc #55618
cc #53130
cc rust-lang/rfcs#1289
Closes #18084
Closes #54525
Fixes #54390
Fixes #55668

r? @ghost
@petrochenkov
Copy link
Contributor

self means "current mod" and cannot be changed backward-compatibly, but #55884 provides an alternative solution, on 2018 edition import paths are resolved in current scope and

const _DUMMY: () = {
    mod some_mod {
        pub struct Bar;
    }
    use some_mod::Bar;
};

"just works".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants