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

Provide more information on duplicate lang item error. #73449

Merged
merged 2 commits into from
Jul 2, 2020

Conversation

ehuss
Copy link
Contributor

@ehuss ehuss commented Jun 17, 2020

This gives some notes on the location of the files where the lang items were loaded from. Some duplicate lang item errors can be a little confusing, and this might help in diagnosing what has happened.

Here's an example when hitting a bug with Cargo's build-std:

error: duplicate lang item in crate `core` (which `rustc_std_workspace_core` depends on): `try`.
  |
  = note: the lang item is first defined in crate `core` (which `z10` depends on)
  = note: first definition in `core` loaded from /Users/eric/Proj/rust/cargo/scratch/z10/target/target/debug/deps/libcore-a764da499c7385f4.rmeta
  = note: second definition in `core` loaded from /Users/eric/Proj/rust/cargo/scratch/z10/target/target/debug/deps/libcore-5b082675aea34986.rmeta

@rust-highfive
Copy link
Collaborator

r? @matthewjasper

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

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 17, 2020
@ehuss
Copy link
Contributor Author

ehuss commented Jun 17, 2020

This is somewhat hacky, as I'm not too familiar with the compiler code (I never know the proper ways to get information from one place to another). I'm not sure if this will actually be helpful to anyone besides myself, so I wanted to open this PR to see if anyone might see some use in it.

cc @alexcrichton, you may or may not have an opinion on the usefulness here.

@alexcrichton
Copy link
Member

👍 from a usefulness perspective, seems handy to have!

Copy link
Contributor

@matthewjasper matthewjasper left a comment

Choose a reason for hiding this comment

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

crate_extern_paths should be a query. I've left a couple of comments to give some explanation on how to do that.
I also have a nit about the error message.

paths.join(", ")
};
err.note(&format!(
"{} definition in `{}` loaded from {}",
Copy link
Contributor

Choose a reason for hiding this comment

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

For the local crate the note should be "{} definition in the local crate".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, tweaked the wording. I kept the crate name, since I find that can be useful at times.

@@ -513,4 +514,8 @@ impl CrateStore for CStore {
fn allocator_kind(&self) -> Option<AllocatorKind> {
self.allocator_kind()
}

fn crate_extern_paths(&self, cnum: CrateNum) -> Vec<PathBuf> {
self.get_crate_data(cnum).source().paths().cloned().collect()
Copy link
Contributor

Choose a reason for hiding this comment

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

Move this into the provide macro above.

@@ -1207,6 +1208,14 @@ impl<'tcx> TyCtxt<'tcx> {
if cnum == LOCAL_CRATE { false } else { self.cstore.crate_is_private_dep_untracked(cnum) }
}

pub fn crate_extern_paths(&self, cnum: CrateNum) -> Vec<PathBuf> {
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be a query: you can declare it be copying this example and updating the signature and description

query extra_filename(_: CrateNum) -> String {
eval_always
desc { "looking up the extra filename for a crate" }
}

@matthewjasper matthewjasper added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 21, 2020
@ehuss
Copy link
Contributor Author

ehuss commented Jun 26, 2020

@matthewjasper Thanks for taking a look!

Is there some guidance on what should be a query and what should be a function? I assumed queries were mainly used for caching, and I assumed in this case that it wouldn't be useful since this only appears in an error.

I'm also a bit confused on how to define the query. I'm getting all sorts of errors that I do not understand how to resolve. In particular:

error[E0412]: cannot find type `PathBuf` in this scope
    --> src/librustc_middle/query/mod.rs:1046:54
     |
38   | / rustc_queries! {
39   | |     Other {
40   | |         query trigger_delay_span_bug(key: DefId) -> () {
41   | |             desc { "trigger a delay span bug" }
...    |
1046 | |         query crate_extern_paths(_: CrateNum) -> Vec<PathBuf> {
     | |                                                      ^^^^^^^ not found in this scope
...    |
1446 | |     }
1447 | | }
     | |_- in this expansion of `rustc_query_append!`
     |
    ::: src/librustc_middle/ty/query/mod.rs:104:1
     |
104  |   rustc_query_append! { [define_queries!][<'tcx>] }
     |   ------------------------------------------------- in this macro invocation
     |
help: consider importing this struct
     |
1    | use std::path::PathBuf;
     |

warning: unused import: `std::path::PathBuf`
  --> src/librustc_middle/query/mod.rs:17:5
   |
17 | use std::path::PathBuf;
   |     ^^^^^^^^^^^^^^^^^^
   |
   = note: `#[warn(unused_imports)]` on by default

error[E0283]: type annotations needed
    --> src/librustc_middle/ty/query/plumbing.rs:516:30
     |
237  |  /     macro_rules! define_queries {
238  |  |         (<$tcx:tt> $($category:tt {
239  |  |             $($(#[$attr:meta])* [$($modifiers:tt)*] fn $name:ident: $node:ident($($K:tt)*) -> $V:ty,)*
240  |  |         },)*) => {
241  | /|             define_queries_inner! { <$tcx>
242  | ||                 $($( $(#[$attr])* category<$category> [$($modifiers)*] fn $name: $node($($K)*) -> $V,)*)*
243  | ||             }
     | ||_____________- in this macro invocation (#3)
244  |  |         }
245  |  |     }
     |  |_____- in this expansion of `define_queries!` (#2)
...
252  | /      macro_rules! define_queries_inner {
253  | |          (<$tcx:tt>
254  | |           $($(#[$attr:meta])* category<$category:tt>
255  | |              [$($modifiers:tt)*] fn $name:ident: $node:ident($($K:tt)*) -> $V:ty,)*) => {
...    |
265  | /              define_queries_struct! {
266  |                    tcx: $tcx,
267  |                    input: ($(([$($modifiers)*] [$($attr)*] [$name]))*)
268  | |              }
     | |______________- in this macro invocation (#4)
...
485  | |          }
486  | |      }
     | |______- in this expansion of `define_queries_inner!` (#3)
487  |
488  | /      macro_rules! define_queries_struct {
489  | |          (tcx: $tcx:tt,
490  | |           input: ($(([$($modifiers:tt)*] [$($attr:tt)*] [$name:ident]))*)) => {
491  | |              pub struct Queries<$tcx> {
...    |
516  | |                          $($name: Default::default()),*
     | |                                   ^^^^^^^^^^^^^^^^ cannot infer type
...    |
536  | |          };
537  | |      }
     | |______- in this expansion of `define_queries_struct!` (#4)
     |
    ::: src/librustc_middle/ty/query/mod.rs:104:1
     |
104  |        rustc_query_append! { [define_queries!][<'tcx>] }
     |        ------------------------------------------------- in this macro invocation (#1)
     |
    ::: src/librustc_middle/query/mod.rs:38:1
     |
38   |      / rustc_queries! {
39   |      |     Other {
40   |      |         query trigger_delay_span_bug(key: DefId) -> () {
41   |      |             desc { "trigger a delay span bug" }
...         |
1446 |      |     }
1447 |      | }
     |      | -
     |      | |
     |      |_in this expansion of `rustc_query_append!` (#1)
     |        in this macro invocation (#2)
     |
     = note: cannot satisfy `_: std::default::Default`
     = note: required by `std::default::Default::default`

The first error is very strange, since I am using PathBuf in the same file, I don't know why it can't find it.

The second one is completely opaque to me, perhaps it is a consequence of the first error?

@matthewjasper
Copy link
Contributor

The use needs to be in src/librustc_middle/ty/query/mod.rs, not src/librustc_middle/query/mod.rs.

I couldn't find an explanation of what should be a query. Queries are used both for caching and tracking dependencies for incremental: queries are expected to be depend only on their inputs and the results of any queries that they call. For data that exists prior to the query system, the current approach is to make make the public method to access the data an eval_always query.

@ehuss ehuss force-pushed the duplicate-lang-item branch from c2d8966 to 1b3ef66 Compare June 30, 2020 16:12
@ehuss
Copy link
Contributor Author

ehuss commented Jun 30, 2020

Oh, thanks! In hindsight that was obvious, I'm not sure how I missed that.

@matthewjasper
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented Jul 1, 2020

📌 Commit 1b3ef66 has been approved by matthewjasper

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jul 1, 2020
Manishearth added a commit to Manishearth/rust that referenced this pull request Jul 1, 2020
…wjasper

Provide more information on duplicate lang item error.

This gives some notes on the location of the files where the lang items were loaded from. Some duplicate lang item errors can be a little confusing, and this might help in diagnosing what has happened.

Here's an example when hitting a bug with Cargo's build-std:

```
error: duplicate lang item in crate `core` (which `rustc_std_workspace_core` depends on): `try`.
  |
  = note: the lang item is first defined in crate `core` (which `z10` depends on)
  = note: first definition in `core` loaded from /Users/eric/Proj/rust/cargo/scratch/z10/target/target/debug/deps/libcore-a764da499c7385f4.rmeta
  = note: second definition in `core` loaded from /Users/eric/Proj/rust/cargo/scratch/z10/target/target/debug/deps/libcore-5b082675aea34986.rmeta
```
Manishearth added a commit to Manishearth/rust that referenced this pull request Jul 2, 2020
…wjasper

Provide more information on duplicate lang item error.

This gives some notes on the location of the files where the lang items were loaded from. Some duplicate lang item errors can be a little confusing, and this might help in diagnosing what has happened.

Here's an example when hitting a bug with Cargo's build-std:

```
error: duplicate lang item in crate `core` (which `rustc_std_workspace_core` depends on): `try`.
  |
  = note: the lang item is first defined in crate `core` (which `z10` depends on)
  = note: first definition in `core` loaded from /Users/eric/Proj/rust/cargo/scratch/z10/target/target/debug/deps/libcore-a764da499c7385f4.rmeta
  = note: second definition in `core` loaded from /Users/eric/Proj/rust/cargo/scratch/z10/target/target/debug/deps/libcore-5b082675aea34986.rmeta
```
Manishearth added a commit to Manishearth/rust that referenced this pull request Jul 2, 2020
…wjasper

Provide more information on duplicate lang item error.

This gives some notes on the location of the files where the lang items were loaded from. Some duplicate lang item errors can be a little confusing, and this might help in diagnosing what has happened.

Here's an example when hitting a bug with Cargo's build-std:

```
error: duplicate lang item in crate `core` (which `rustc_std_workspace_core` depends on): `try`.
  |
  = note: the lang item is first defined in crate `core` (which `z10` depends on)
  = note: first definition in `core` loaded from /Users/eric/Proj/rust/cargo/scratch/z10/target/target/debug/deps/libcore-a764da499c7385f4.rmeta
  = note: second definition in `core` loaded from /Users/eric/Proj/rust/cargo/scratch/z10/target/target/debug/deps/libcore-5b082675aea34986.rmeta
```
Manishearth added a commit to Manishearth/rust that referenced this pull request Jul 2, 2020
…wjasper

Provide more information on duplicate lang item error.

This gives some notes on the location of the files where the lang items were loaded from. Some duplicate lang item errors can be a little confusing, and this might help in diagnosing what has happened.

Here's an example when hitting a bug with Cargo's build-std:

```
error: duplicate lang item in crate `core` (which `rustc_std_workspace_core` depends on): `try`.
  |
  = note: the lang item is first defined in crate `core` (which `z10` depends on)
  = note: first definition in `core` loaded from /Users/eric/Proj/rust/cargo/scratch/z10/target/target/debug/deps/libcore-a764da499c7385f4.rmeta
  = note: second definition in `core` loaded from /Users/eric/Proj/rust/cargo/scratch/z10/target/target/debug/deps/libcore-5b082675aea34986.rmeta
```
bors added a commit to rust-lang-ci/rust that referenced this pull request Jul 2, 2020
…arth

Rollup of 16 pull requests

Successful merges:

 - rust-lang#72569 (Remove legacy InnoSetup GUI installer)
 - rust-lang#73306 (Don't implement Fn* traits for #[target_feature] functions)
 - rust-lang#73345 (expand: Stop using nonterminals for passing tokens to attribute and derive macros)
 - rust-lang#73449 (Provide more information on duplicate lang item error.)
 - rust-lang#73569 (Handle `macro_rules!` tokens consistently across crates)
 - rust-lang#73803 (Recover extra trailing angle brackets in struct definition)
 - rust-lang#73839 (Split and expand nonstandard-style lints unicode unit test.)
 - rust-lang#73841 (Remove defunct `-Z print-region-graph`)
 - rust-lang#73848 (Fix markdown rendering in librustc_lexer docs)
 - rust-lang#73865 (Fix Zulip topic format)
 - rust-lang#73892 (Clean up E0712 explanation)
 - rust-lang#73898 (remove duplicate test for rust-lang#61935)
 - rust-lang#73906 (Add missing backtick in `ty_error_with_message`)
 - rust-lang#73909 (`#[deny(unsafe_op_in_unsafe_fn)]` in libstd/fs.rs)
 - rust-lang#73910 (Rewrite a few manual index loops with while-let)
 - rust-lang#73929 (Fix comment typo)

Failed merges:

r? @ghost
@bors bors merged commit 6b57050 into rust-lang:master Jul 2, 2020
@cuviper cuviper added this to the 1.46 milestone May 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants