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

rustc: always suggests #[macro_use] for undefined macros #37910

Closed
wants to merge 2 commits into from

Conversation

liigo
Copy link
Contributor

@liigo liigo commented Nov 21, 2016

Work in progress. Don't merge.

If people forget adding #[macro_use] (or even don't know about it), give them a suggestion.

// missing `#[macro_use]`
extern crate log;
fn main() {
    info!("hello");
}

play link: https://is.gd/nwB6ty

@rust-highfive
Copy link
Collaborator

r? @arielb1

(rust_highfive has picked a reviewer for you, use r? to override)

@steveklabnik
Copy link
Member

it looks like @arielb1 never reviewed. Maybe someone else from @rust-lang/compiler ?

@eddyb
Copy link
Member

eddyb commented Jan 3, 2017

r? @jseyfried

@rust-highfive rust-highfive assigned jseyfried and unassigned arielb1 Jan 3, 2017
Copy link
Contributor

@jseyfried jseyfried left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@liigo Thanks for the PR!

Let me know here or on IRC if you have any questions about my comments.

@@ -385,10 +385,9 @@ impl<'a> Resolver<'a> {
if let Some(suggestion) = find_best_match_for_name(self.macro_names.iter(), name, None) {
if suggestion != name {
err.help(&format!("did you mean `{}!`?", suggestion));
} else {
err.help(&format!("have you added the `#[macro_use]` on the module/import?"));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should keep this here, but it should just say "have you added the #[macro_use] on the module?".

}
}
err.help(&format!("have you added the `#[macro_use]` on the module/import?"));
Copy link
Contributor

@jseyfried jseyfried Jan 4, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this should say "have you added the #[macro_use] on the extern crate", but only if there is a macro with that name in an extern crate.

You should be able to use resolver.lookup_import_candidates() (in librustc_resolve/lib.rs) with a filter_fn that checks for Def::Macros to see if there are any relevant macros in extern crates that the user might want to #[macro_use].

@pnkfelix
Copy link
Member

pnkfelix commented Jan 4, 2017

Hmm, with this change, if I write:

#[macro_use]
extern crate log;
fn main() {
    infq!("hello");
}

won't the compiler now suggest I put in a #[macro_use] (in addition to suggesting renaming to info!) even though no new #[macro_use] is needed?


Update: oops, @jseyfried 's feedback has already covered this; I needed to refresh my page.

@liigo
Copy link
Contributor Author

liigo commented Jan 8, 2017

I'll try to work on according to review suggestions. Please don't merge util I'm done.

@alexcrichton
Copy link
Member

Closing due to inactivity, but feel free to resubmit with comments addressed!

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

Successfully merging this pull request may close these issues.

None yet

8 participants