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

use self::b has stopped consulting local non-pub use of b #13598

Closed
pnkfelix opened this issue Apr 18, 2014 · 5 comments · Fixed by #37127
Closed

use self::b has stopped consulting local non-pub use of b #13598

pnkfelix opened this issue Apr 18, 2014 · 5 comments · Fixed by #37127
Labels
A-resolve Area: Name/path resolution done by `rustc_resolve` specifically

Comments

@pnkfelix
Copy link
Member

I used to be able to do:

use a::b;
use D = self::b::C;

but now I cannot, at least not in all cases.

Here is a test case:

mod a {
    pub mod b {
        pub type C = uint;
    }
}

#[cfg(variant1)]
mod variant1 {
    use a::b;
    use D = self::b::C;

    pub fn main() {
        let d : D = 3;
        println!("d: {}", d);
    }
}

#[cfg(variant2)]
mod variant2 {
    use D = a::b::C;

    pub fn main() {
        let d : D = 3;
        println!("d: {}", d);
    }
}

#[cfg(variant1)]
pub fn main() {
    variant1::main();
}

#[cfg(variant2)]
pub fn main() {
    variant2::main();
}

Here is a transcript of how this used to work (on a rustc v0.11-pre circa 2014-04-07) versus breaking now (on a rustc v0.11-pre circa 2014-04018).

% rustc --version
/Users/fklock/opt/rust-dbg/bin/rustc 0.11-pre (0deb16a 2014-04-07 02:26:37 -0700)
host: x86_64-apple-darwin
% rustc --cfg variant1 /tmp/n.rs && ./n
d: 3
% rustc --cfg variant2 /tmp/n.rs && ./n
d: 3
% ./objdir-dbgopt/x86_64-apple-darwin/stage2/bin/rustc --version
./objdir-dbgopt/x86_64-apple-darwin/stage2/bin/rustc 0.11-pre (ce2bab6 2014-04-18 04:11:19 -0700)
host: x86_64-apple-darwin
% ./objdir-dbgopt/x86_64-apple-darwin/stage2/bin/rustc --cfg variant1 /tmp/n.rs && ./n
/tmp/n.rs:10:9: 10:24 error: unresolved import: could not find `b` in `variant1`.
/tmp/n.rs:10     use D = self::b::C;
                     ^~~~~~~~~~~~~~~
/tmp/n.rs:10:9: 10:24 error: failed to resolve import `self::b::C`
/tmp/n.rs:10     use D = self::b::C;
                     ^~~~~~~~~~~~~~~
error: aborting due to 2 previous errors
% ./objdir-dbgopt/x86_64-apple-darwin/stage2/bin/rustc --cfg variant2 /tmp/n.rs && ./n
d: 3
% 

Discussion with @alexcrichton leads me to think that this change may have been injected as part of PR #13409.

It may or may be a bug. Whether you see it as a bug may depend on your view of how privacy and use interact. (In particular, if you make a variant3 of variant1 where the use a::b is rewritten to pub use a::b, so that the b is now publicly exported from self, then things seem to work.)

My reasoning is that we should be able to do:

use a::b; // a non-pub import
use D = self::b::C;

because the way we resolve use self::b::C, we look for other non-pub things in self, such as a hypothetical non-pub mod b defined within self.

@pnkfelix
Copy link
Member Author

Here is a more fully fleshed out illustration of the alternatives I described above:

mod a {
    pub mod b {
        pub type C = uint;
    }
}

#[cfg(variant1)]
mod variant1 {
    use a::b;
    use D = self::b::C;

    pub fn main() {
        let d : D = 3;
        println!("d: {}", d);
    }
}

#[cfg(variant2)]
mod variant2 {
    use D = a::b::C;

    pub fn main() {
        let d : D = 3;
        println!("d: {}", d);
    }
}

#[cfg(variant3)]
mod variant3 {
    pub use a::b;
    use D = self::b::C;

    pub fn main() {
        let d : D = 3;
        println!("d: {}", d);
    }
}

#[cfg(variant4)]
mod variant4 {
    use D = self::b::C;

    pub fn main() {
        let d : D = 3;
        println!("d: {}", d);
    }

    mod b {
        pub type C = uint;
    }
}

#[cfg(variant1)]
pub fn main() {
    variant1::main();
}

#[cfg(variant2)]
pub fn main() {
    variant2::main();
}

#[cfg(variant3)]
pub fn main() {
    variant3::main();
}

#[cfg(variant4)]
pub fn main() {
    variant4::main();
}

The point is that variant4 is meant to illustrate that we allow use D = self::b::C to look at other non-pub locally-visible things named b, so why should we make an exception for use a::b; ?

(There may indeed be good reasons. For example, if we need to impose this restriction to avoid import cycles, I can accept that. But I want to make sure we did not just accidentally regress this feature without consideration.)

@alexcrichton
Copy link
Member

If we wanted to restore this behavior, it would also enable something like:

mod a {
    use b::c;

    use self::c::foo;
    use a::c::foo;
}

mod b {
    pub mod c {
        pub fn foo() {}
    }
}

Allowing self::c::foo seems ok, but allowing a::c::foo sits less well with me... Arguably they're the same thing though.

@pnkfelix
Copy link
Member Author

@alexcrichton I guess from my POV, I do not see use a::... as synonymous with use self::...

It makes sense to me that use a::... would be restricted to the public interface of a; from my POV, one would like use a::[STUFF] to have the same meaning at any valid use-decl context within a given crate (i.e. I would like to be able to freely cut and paste such use-decls down into other fns or mods), and that would require restricting use a::... to the public interface of a.

But use self::[STUFF] cannot possibly enjoy the same property. It cannot have the same meaning in other mods within the crate. So it would make sense to me for it to obey different rules about lookup, where it would have access to the non-pub names that have been imported into the current mod.

@flaper87
Copy link
Contributor

Triage:

The syntax use D = a::b::C has been deprecated in favor of use a::b::C as D. However, the problem still exists when using the new syntax. Here's an updated example:

mod a {
    pub mod b {
        pub type C = uint;
    }
}

#[cfg(variant1)]
mod variant1 {
    use a::b;
    use self::b::C as D;

    pub fn main() {
        let d : D = 3;
        println!("d: {}", d);
    }
}

#[cfg(variant2)]
mod variant2 {
    use a::b::C as D;

    pub fn main() {
        let d : D = 3;
        println!("d: {}", d);
    }
}

#[cfg(variant1)]
pub fn main() {
    variant1::main();
}

#[cfg(variant2)]
pub fn main() {
    variant2::main();
}

And the updated output:

phantom /tmp $ rustc --cfg variant1 tests.rs
tests.rs:10:9: 10:24 error: unresolved import `self::b::C`. Could not find `b` in `variant1`
tests.rs:10     use self::b::C as D;
                    ^~~~~~~~~~~~~~~
error: aborting due to previous error
phantom /tmp $ rustc --cfg variant2 tests.rs 

@steveklabnik
Copy link
Member

Triage; still reproduces

bors added a commit that referenced this issue Nov 21, 2016
arcnmx pushed a commit to arcnmx/rust that referenced this issue Jan 9, 2023
Add action to expand a declarative macro once, inline. Fixes rust-lang#13598

This commit adds a new r-a method, `expandMacroInline`, which expands the macro that's currently selected. See  rust-lang#13598 for the most applicable issue; though I suspect it'll resolve part of rust-lang#5949 and make rust-lang#11888 significantly easier).

The macro works like this:

![rust-analyser-feature](https://user-images.githubusercontent.com/10906982/208813167-3123e379-8fd5-4206-a4f4-5af1129565f9.gif)

I have 2 questions before this PR can be merged:

1. **Should we rustfmt the output?** The advantage of doing this is neater code. The disadvantages are we'd have to format the whole expr/stmt/block (since there's no point just formatting one part, especially over multiple lines), and maybe it moves the code around more in weird ways. My suggestion here is to start off by not doing any formatting; and if it appears useful we can decide to do formatting in a later release.
2.   **Is it worth solving the `$crate` hygiene issue now?** -- I think this PR is usable as of right now for some use-cases; but it is annoying that many common macros (i.e. `println!()`, `format!()`) can't be expanded further unless the user guesses the correct `$crate` value. The trouble with solving that issue is that I think it's complicated and imperfect. If we do solve it; we'd also need to either change the existing `expandMacro`/`expandMacroInline` commands; provide some option to allow/disallow `$crate` expanding; or come to some other compromise.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-resolve Area: Name/path resolution done by `rustc_resolve` specifically
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants