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

Add diagnostics for rustc_metadata #34970

Closed
wants to merge 13 commits into from
1 change: 1 addition & 0 deletions src/librustc_driver/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1112,6 +1112,7 @@ pub fn diagnostics_registry() -> errors::registry::Registry {
all_errors.extend_from_slice(&rustc_privacy::DIAGNOSTICS);
all_errors.extend_from_slice(&rustc_trans::DIAGNOSTICS);
all_errors.extend_from_slice(&rustc_const_eval::DIAGNOSTICS);
all_errors.extend_from_slice(&rustc_metadata::DIAGNOSTICS);
Copy link
Member

Choose a reason for hiding this comment

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

Why adding this? (I'm curious)


Registry::new(&all_errors)
}
Expand Down
215 changes: 204 additions & 11 deletions src/librustc_metadata/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,14 @@ register_long_diagnostics! {
E0454: r##"
A link name was given with an empty name. Erroneous code example:

```
```ignore
Copy link
Member

Choose a reason for hiding this comment

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

It should be compile_fail,E0454 and not ignore.

#[link(name = "")] extern {} // error: #[link(name = "")] given with empty name
```

The rust compiler cannot link to an external library if you don't give it its
name. Example:

```
```ignore
#[link(name = "some_lib")] extern {} // ok!
```
"##,
Expand Down Expand Up @@ -50,7 +50,7 @@ See more: https://doc.rust-lang.org/book/conditional-compilation.html
E0458: r##"
An unknown "kind" was specified for a link attribute. Erroneous code example:

```
```ignore
Copy link
Member

Choose a reason for hiding this comment

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

It should be compile_fail,E0458 and not ignore.

#[link(kind = "wonderful_unicorn")] extern {}
// error: unknown kind: `wonderful_unicorn`
```
Expand All @@ -64,23 +64,23 @@ Please specify a valid "kind" value, from one of the following:
E0459: r##"
A link was used without a name parameter. Erroneous code example:

```
```ignore
Copy link
Member

Choose a reason for hiding this comment

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

It should be compile_fail,E0458 and not ignore.

#[link(kind = "dylib")] extern {}
// error: #[link(...)] specified without `name = "foo"`
```

Please add the name parameter to allow the rust compiler to find the library
you want. Example:

```
```ignore
#[link(kind = "dylib", name = "some_lib")] extern {} // ok!
```
"##,

E0463: r##"
A plugin/crate was declared but cannot be found. Erroneous code example:

```
```ignore
#![feature(plugin)]
#![plugin(cookie_monster)] // error: can't find crate for `cookie_monster`
extern crate cake_is_a_lie; // error: can't find crate for `cake_is_a_lie`
Expand All @@ -91,6 +91,204 @@ You need to link your code to the relevant crate in order to be able to use it
well, and you link to them the same way.
"##,

E0466: r##"
Invalid macro import declarations.
Copy link
Member

Choose a reason for hiding this comment

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

You're supposed to be a little more exhaustive on the error, not just repeat the error message.


Causes of this error:
Copy link
Member

Choose a reason for hiding this comment

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

Should be "Erroneous code example".

Copy link
Author

Choose a reason for hiding this comment

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

Is the language that critical?

Just looking at handful random error numbers, it looks like E0050, E0053, E0054, and E0055 don't say "Erroneous code example".

If we stick that strictly to the RFC it looks like some old errors need to be rewritten.

Copy link
Member

Choose a reason for hiding this comment

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

They've been written way before the RFC. They'll be updated later.


```ignore
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't be ignore.

#[macro_use(a_macro(another_macro))] // error: invalid import declaration
Copy link
Member

Choose a reason for hiding this comment

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

Just one whitespace is enough before "// error: ...".

extern crate some_crate;

#[macro_use(i_want = "some_macros")] // error: invalid import declaration
Copy link
Member

Choose a reason for hiding this comment

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

Just one whitespace is enough before "// error: ...".

extern crate another_crate;
```

This is a syntax error at the level of attribute declarations.

The proper syntax for macro imports is the following:

```ignore
// // some_crate contains:
Copy link
Member

Choose a reason for hiding this comment

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

I think you should uncomment the macros and keep the ignore flag.

// #[macro_export]
// macro_rules! get_tacos {
// ...
// }
//
// #[macro_export]
// macro_rules! bring_beer {
// ...
// }
#[macro_use(get_tacos, bring_beer)] // imports macros get_tacos and
Copy link
Member

Choose a reason for hiding this comment

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

Missing empty line before this line (and still too much whitespaces but in here I don't find it problematic).

extern crate some_crate; // bring_beer from some_crate
```

If you would like to import all exported macros, write `macro_use` with no
arguments.
"##,

E0467: r##"
Invalid or no macros listed for reexport.

Causes of this error:
Copy link
Member

Choose a reason for hiding this comment

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

Same.


```ignore
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't be ignore.

#[macro_reexport] // error: no macros listed for export
Copy link
Member

Choose a reason for hiding this comment

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

Same.

extern crate macros_for_good;
```
```ignore
Copy link
Member

Choose a reason for hiding this comment

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

Same.

#[macro_reexport(fun_macro = "foo")] // error: not a macro identifier
extern crate macros_for_good;
```

This is a syntax error at the level of attribute declarations.

Currently, `macro_reexport` requires at least one macro name to be listed.
Unlike `macro_use`, listing no names does not reexport all macros from the
given crate.

Decide which macros you would like to export and list them properly.

These are proper reexport declarations:

```ignore
#[macro_reexport(some_macro, another_macro)]
extern crate macros_for_good;
```
"##,

E0468: r##"
A non-root module attempts to import macros from another crate.

Example of erroneous code:
Copy link
Member

Choose a reason for hiding this comment

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

Same.

Copy link
Author

Choose a reason for hiding this comment

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

macros_for_good does not exist, so this example will still fail compile.

Is general consensus to just put compile_fail, or compile_fail,<errno>? Even though in this case errno is unrelated to E0467?

Copy link
Member

Choose a reason for hiding this comment

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

The point of adding compile_fail,<errno> is to avoid out of date error explanations like it happened a lot. If the error isn't thrown by the code anymore, then it means it has changed and the error explanation should be updated.


```ignore
Copy link
Member

Choose a reason for hiding this comment

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

Same.

mod foo {
#[macro_use(helpful_macro)] // error: must be at crate root to import
extern crate some_crate; // macros from another crate
helpful_macro!(...)
}

fn main() {
Copy link
Member

Choose a reason for hiding this comment

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

Why adding the main function in here?

// ...
}
```

Only `extern crate` imports at the crate root level (i.e., in lib.rs) are
allowed to import macros.

Either move the macro import to crate root or do without the foreign macros.

This will work:

```ignore
#[macro_use(helpful_macro)]
extern crate some_crate;
mod foo {
helpful_macro!(...)
}

fn main() {
Copy link
Member

Choose a reason for hiding this comment

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

Why adding the main function in here?

//...
}
```

Copy link
Member

Choose a reason for hiding this comment

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

Extra empty line.

Copy link
Author

Choose a reason for hiding this comment

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

not sure I understand?

Copy link
Member

Choose a reason for hiding this comment

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

This line shouldn't exist.

Copy link
Member

Choose a reason for hiding this comment

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

Still extra empty line.

"##,

E0469: r##"
A macro listed for import was not found.

Example of erroneous code:
Copy link
Member

Choose a reason for hiding this comment

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

Ah good! :D


```ignore
/// // crate some_crate contains:
/// #[macro_export]
/// macro_rules! eat {
/// ...
/// }
/// macro_rules! drink {
/// ...
/// }

// error: drink is a private macro of some_crate
// error: be_merry does not exist in some_crate
#[macro_use(drink, be_merry)]
extern crate some_crate;
```

Either the listed macro is not contained in the imported crate, or it is not
exported from the given crate.

This could be caused by a typo. Did you misspell the macro's name?

Double-check the names of the macros listed for import, and that the crate
in question exports them.

A working version of the above:

```ignore
/// // crate some_crate contains:
/// #[macro_export]
/// macro_rules! eat {
/// ...
/// }
/// #[macro_export]
/// macro_rules! drink {
/// ...
/// }

#[macro_use(eat, drink)]
extern crate some_crate;
```
"##,

E0470: r##"
A macro listed for reexport was not found.

Example of erroneous code:

```ignore
/// // crate some_crate contains:
/// #[macro_export]
/// macro_rules! eat {
/// ...
/// }
/// macro_rules! drink {
/// ...
/// }

// error: drink is a private macro of some_crate
// error: be_merry does not exist in some_crate
#[macro_reexport(drink, be_merry)]
extern crate some_crate;
```

Either the listed macro is not contained in the imported crate, or it is not
exported from the given crate.

This could be caused by a typo. Did you misspell the macro's name?

Double-check the names of the macros listed for reexport, and that the crate
in question exports them.

A working version of the above:

```ignore
/// // crate some_crate contains:
/// #[macro_export]
/// macro_rules! eat {
/// ...
/// }
/// #[macro_export]
/// macro_rules! drink {
/// ...
/// }

#[macro_reexport(eat, drink)]
extern crate some_crate;
```
"##,

}

register_diagnostics! {
Expand All @@ -102,11 +300,6 @@ register_diagnostics! {
E0462, // found staticlib `..` instead of rlib or dylib
E0464, // multiple matching crates for `..`
E0465, // multiple .. candidates for `..` found
E0466, // bad macro import
E0467, // bad macro reexport
E0468, // an `extern crate` loading macros must be at the crate root
E0469, // imported macro not found
E0470, // reexported macro not found
E0519, // local crate and dependency have same (crate-name, disambiguator)
E0523, // two dependencies have same (crate-name, disambiguator) but different SVH
}
2 changes: 2 additions & 0 deletions src/librustc_metadata/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,3 +62,5 @@ pub mod index;
pub mod loader;
pub mod macro_import;
pub mod tls_context;

__build_diagnostic_array! { librustc_metadata, DIAGNOSTICS }
Copy link
Member

Choose a reason for hiding this comment

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

Why adding this line? (I'm still curious haha)

Copy link
Author

Choose a reason for hiding this comment

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

I mentioned this in a comment in the #32777 thread -

The diagnostics in librustc_metadata were not registered in the first function you mentioned, so rustc --explain said there was no extended information for any of the errors in librustc_metadata.

The line in metadata/lib.rs aggregates the diagnostic info into an array, and the one in driver/lib.rs registers the information in that array.

With these changes, rustc --explain works for all of the error codes in librustc_metadata

Copy link
Author

Choose a reason for hiding this comment

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

Didn't know about make tidy! that's a quick fix