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

Missing tcx query provider is hard to diagnose - error message is not intuitive #83122

Closed
richkadel opened this issue Mar 14, 2021 · 9 comments · Fixed by #83367
Closed

Missing tcx query provider is hard to diagnose - error message is not intuitive #83122

richkadel opened this issue Mar 14, 2021 · 9 comments · Fixed by #83367
Labels
A-contributor-roadblock Area: Makes things more difficult for new contributors to rust itself A-query-system Area: The rustc query system (https://rustc-dev-guide.rust-lang.org/query.html)

Comments

@richkadel
Copy link
Contributor

richkadel commented Mar 14, 2021

I inadvertently deleted a rustc_middle::ty::query::Providers assignment (which meant that any tcx.some_unassigned_query(def_id) would not be resolved.

Since tcx query functions are assigned at runtime, this is not a build-time error.

The best that rustc can do is generate a runtime error, but the error message is non-intuitive, and it took quite a while for me to realize my mistake:

error: internal compiler error: compiler/rustc_middle/src/ty/query/mod.rs:279:1:
   `tcx.coverageinfo(DefId(0:3 ~ abort[317d]::might_abort))` unsupported by its crate

Is there a better way to word this message to make it more obvious that there is a query defined, but a provider was never assigned?

@jyn514 jyn514 added A-contributor-roadblock Area: Makes things more difficult for new contributors to rust itself A-query-system Area: The rustc query system (https://rustc-dev-guide.rust-lang.org/query.html) labels Mar 14, 2021
@jyn514
Copy link
Member

jyn514 commented Mar 14, 2021

Maybe something like "no provider available for local/external crate" (depending on whether it's local or not)?

@richkadel
Copy link
Contributor Author

I think the reference to "crate" is what threw me off actually. To me, it implies the missing provider is missing in the code being compiled, but the problem was the provider should have been assigned in the compiler code itself.

@jyn514
Copy link
Member

jyn514 commented Mar 14, 2021

The distinction matters though - there can be a provider for local crates but not external ones, or vice versa.

@richkadel
Copy link
Contributor Author

That's fine. I don't fully understand the significance of the crate, but if it helps others debug certain types of issues, it's fine to replace "unsupported by its crate" to "no provider available for local/external crate" (if the information is available).

But the problem I had didn't have anything to do with the crate distinction, or at least, if it did, the message wouldn't have helped me diagnose the actual problem:

In my case, there was a query defined in rustc_middle/src/query/mod.rs for coverageinfo(), and there should have been a Provider assigned to that query, defined in the compiler code, in rustc_mir/src/transform/coverage/query.rs (except I mistakenly removed that query's assignment to Providers.

Removing that query assignment was a bug in the compiler code itself, regardless of what was happening with local vs. external crates.

So how about just adding to either the current message or your revised message?

I'm not sure how to tell what is local or external here, from just the Providers or the tcx, but I added a placeholder. (AFAIK, the key type is not necessarily a DefId at this point.):

impl Default for Providers {

        impl Default for Providers {
            fn default() -> Self {
                Providers {
                    $($name: |tcx, key| bug!(
                        "`tcx.{}({:?})` no provider available for {} crate; perhaps the `{}` query was never assigned a provider function",
                         stringify!($name),
                         key,
                         if tcx.is_local() { "local" } else { "external" },
                         stringify!($name),
                    ),)*
                }
            }
        }

@jyn514
Copy link
Member

jyn514 commented Mar 15, 2021

That's fine. I don't fully understand the significance of the crate, but if it helps others debug certain types of issues, it's fine to replace "unsupported by its crate" to "no provider available for local/external crate" (if the information is available).

But the problem I had didn't have anything to do with the crate distinction, or at least, if it did, the message wouldn't have helped me diagnose the actual problem:

In my case, there was a query defined in rustc_middle/src/query/mod.rs for coverageinfo(), and there should have been a Provider assigned to that query, defined in the compiler code, in rustc_mir/src/transform/coverage/query.rs (except I mistakenly removed that query's assignment to Providers.

Removing that query assignment was a bug in the compiler code itself, regardless of what was happening with local vs. external crates.

So how about just adding to either the current message or your revised message?

I'm not sure how to tell what is local or external here, from just the Providers or the tcx, but I added a placeholder. (AFAIK, the key type is not necessarily a DefId at this point.):

impl Default for Providers {

        impl Default for Providers {
            fn default() -> Self {
                Providers {
                    $($name: |tcx, key| bug!(
                        "`tcx.{}({:?})` no provider available for {} crate; perhaps the `{}` query was never assigned a provider function",
                         stringify!($name),
                         key,
                         if tcx.is_local() { "local" } else { "external" },
                         stringify!($name),
                    ),)*
                }
            }
        }

That looks good! Could you make a pr with that change? :)

@richkadel
Copy link
Contributor Author

@jyn514 - Happy to submit a PR, but I don't think tcx.is_local() exists.

Is there a tcx method that will give me something equivalent?

If not, I can just use the original message, and add the ; perhaps... to the end of it.

@jyn514
Copy link
Member

jyn514 commented Mar 15, 2021

@jyn514 - Happy to submit a PR, but I don't think tcx.is_local() exists.

It's the other way around - there's only ever one tcx, but Providers may be local or non-local. You could add an is_local field to Providers, and set it in

pub static DEFAULT_EXTERN_QUERY_PROVIDERS: SyncLazy<Providers> = SyncLazy::new(|| {
let mut extern_providers = *DEFAULT_QUERY_PROVIDERS;
rustc_metadata::provide_extern(&mut extern_providers);
rustc_codegen_ssa::provide_extern(&mut extern_providers);
extern_providers
});
.

@jyn514
Copy link
Member

jyn514 commented Mar 15, 2021

Hmm, that actually won't work because you have to have a function pointer, not a closure - you don't have access to the current state of the Providers struct. Using the original message seems ok; I don't know if there's a better way.

@richkadel
Copy link
Contributor Author

OK, will do then. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-contributor-roadblock Area: Makes things more difficult for new contributors to rust itself A-query-system Area: The rustc query system (https://rustc-dev-guide.rust-lang.org/query.html)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants