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

The compiler should report publicly exported type names if possible #21934

Open
pcwalton opened this issue Feb 4, 2015 · 28 comments
Open

The compiler should report publicly exported type names if possible #21934

pcwalton opened this issue Feb 4, 2015 · 28 comments
Labels
A-diagnostics Area: Messages for errors, warnings, and lints A-resolve Area: Path resolution C-enhancement Category: An issue proposing an enhancement or a PR with one. D-newcomer-roadblock Diagnostics: Confusing error or lint; hard to understand for new users. E-hard Call for participation: Hard difficulty. Experience needed to fix: A lot. P-medium Medium priority T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@pcwalton
Copy link
Contributor

pcwalton commented Feb 4, 2015

If a private type is reexported at some public location, the Rust compiler will report the private location when referring to the type. It'd be more helpful if the compiler reported the public alias for it.

This was reported by Coda Hale on Twitter.


Update by pnkfelix: here is a standalone bit of code illustrating the problem. (play):

mod a {
    mod m { pub struct PrivateName; }
    pub use m::PrivateName as PublicName;
}

fn main() {
    // a::m::PrivateName; // would error, "module `m` is private"
    a::PublicName;
    a::PublicName.clone();
}

As of nightly 2019-06-17, this issues the diagnostic:

> error[E0599]: no method named `clone` found for type `a::m::PrivateName` [...]
                                                        ^^^^^^^^^^^^^^^^^
                                                                 |
             (author of `fn main` wants to see `a::PublicName` here)
@pcwalton pcwalton added A-typesystem Area: The type system C-enhancement Category: An issue proposing an enhancement or a PR with one. A-diagnostics Area: Messages for errors, warnings, and lints I-wishlist A-resolve Area: Path resolution and removed A-typesystem Area: The type system labels Feb 4, 2015
@steveklabnik
Copy link
Member

This happens a lot with core/std.

@steveklabnik
Copy link
Member

Triage: it seems like a private type can no longer be exported as a public type at all

pub mod A {
    struct Foo;
    pub use A::Foo as Bar;
}

fn main() {
    let b = A::Bar;
}

gives

hello.rs:5:13: 5:26 error: `Foo` is private, and cannot be reexported [E0364]
hello.rs:5     pub use A::Foo as Bar;
                       ^~~~~~~~~~~~~
hello.rs:5:13: 5:26 help: run `rustc --explain E0364` to see a detailed explanation
hello.rs:5:13: 5:26 note: consider marking `Foo` as `pub` in the imported module
hello.rs:5     pub use A::Foo as Bar;
                       ^~~~~~~~~~~~~

So I believe this is effectively fixed? My core/std comment is more about reexports where both are public:

pub mod A {
    pub struct Foo;
    pub use A::Foo as Bar;
}

fn main() {
    let b: i32 = A::Bar;
}

which gives

hello.rs:9:18: 9:24 error: mismatched types:
 expected `i32`,
    found `A::Foo`
(expected i32,
    found struct `A::Foo`) [E0308]
hello.rs:9     let b: i32 = A::Bar;
                            ^~~~~~
hello.rs:9:18: 9:24 help: run `rustc --explain E0308` to see a detailed explanation
error: aborting due to previous error

which is covered by another ticket whose number escapes me.

@petrochenkov
Copy link
Contributor

The issue is relevant to "unnameable" pub types which are still allowed.

mod m {
    pub struct PrivateName;
}

pub use m::PrivateName as PublicName.

m::PrivateName is still used in error messages, while it's preferable to use PublicName.

@torkleyy
Copy link

It also reports names which don't exist:

Consider this library:

// lib.rs of crate foo

mod a {
    pub trait Foo {}
}

pub use a::Foo as Afoo;

pub fn foo<T: Afoo>() {}

And some binary crate using it:

extern crate foo;

use foo::foo;

fn main() {
    foo::<()>(); // Afoo is not implemented for `()`
}

The error message is:

error[E0277]: the trait bound `(): foo::Foo` is not satisfied
 --> src/main.rs:6:5
  |
6 |     foo::<()>();
  |     ^^^^^^^^^ the trait `foo::Foo` is not implemented for `()`
  |
  = note: required by `foo::foo`

So it does not report the private name but a name that does not exist.

@leoyvens
Copy link
Contributor

leoyvens commented Jan 16, 2018

Notes from my attempt at solving this:

  • The module_exports query is helpful as it returns a list of Exports in a module, the ident field is the name or rename.
  • Modifying item_path.rs directly is problematic because it is used in many different parts of the compiler. It's unclear where exactly the logic should go.

Also linking relevant internal's thread.

@Manishearth
Copy link
Member

Do we need an RFC for this? My impression is that this is purely an implementation thing, and doesn't need to go through the actual RFC process. We could do that if we wanted to add configurability (It's not clear to me that that is necessary!), but I think the right way to go would be to implement it behind an env var first and then see what the compiler team thinks.

Like, this is something most folks working on the compiler have wanted for ages, the question is just about how to implement it, which doesn't need an RFC.

@da-x
Copy link
Member

da-x commented Nov 11, 2019

@Manishearth, previously, I was requested an RFC over a smaller change - #49898, and submitted it. This one has even a larger user impact, so it only made sense to go through some RFC process, even if most of the work is implementation.

@Manishearth
Copy link
Member

Manishearth commented Nov 11, 2019 via email

@pnkfelix
Copy link
Member

Like @Manishearth , I do not think an RFC is required here.

It might be nice to have a T-compiler design meeting for it (but even that might be overkill here; at the very least it would require synchronous participation from the proposer, which again may not be necessary here.)

@da-x
Copy link
Member

da-x commented Nov 14, 2019 via email

@pnkfelix
Copy link
Member

triage: I am downgrading this to P-medium. We have too large of a backlog of high priority items and this one does not warrant being visited every week at triage.

@eddyb
Copy link
Member

eddyb commented Jul 13, 2020

There was an idea I noted down in a pull request: #70911 (comment).
In short, it involves shortening paths to a name that is unique to them (e.g. if there's only one Vec ever), and potentially doing some prelude-specific logic (but not necessarily, I guess?).

I don't know if #73996 is based on that comment or not, but it's really cool that it's happening, and I'm sorry I didn't leave a comment here when I suggested the idea in the str librarification PR.

bors added a commit to rust-lang-ci/rust that referenced this issue Sep 3, 2020
diagnostics: shorten paths of unique symbols

This is a step towards implementing a fix for rust-lang#50310, and continuation of the discussion in [Pre-RFC: Nicer Types In Diagnostics - compiler - Rust Internals](https://internals.rust-lang.org/t/pre-rfc-nicer-types-in-diagnostics/11139). Impressed upon me from previous discussion in rust-lang#21934 that an RFC for this is not needed, and I should just come up with code.

The recent improvements to `use` suggestions that I've contributed have given rise to this implementation. Contrary to previous suggestions, it's rather simple logic, and I believe it only reduces the amount of cognitive load that a developer would need when reading type errors.

-----

If a symbol name can only be imported from one place, and as long as it was not glob-imported anywhere in the current crate, we can trim its printed path to the last component.

This has wide implications on error messages with types, for example, shortening `std::vec::Vec` to just `Vec`, as long as there is no other `Vec` importable from anywhere.
@camelid
Copy link
Member

camelid commented Mar 2, 2022

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics Area: Messages for errors, warnings, and lints A-resolve Area: Path resolution C-enhancement Category: An issue proposing an enhancement or a PR with one. D-newcomer-roadblock Diagnostics: Confusing error or lint; hard to understand for new users. E-hard Call for participation: Hard difficulty. Experience needed to fix: A lot. P-medium Medium priority T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests