Use unused_generic_params from crate metadata
#109109
Conversation
|
r? @Nilstrieb (rustbot has picked a reviewer for you, use r? to override) |
|
cc @davidtwco |
| @@ -226,7 +226,14 @@ provide! { tcx, def_id, other, cdata, | |||
| lookup_default_body_stability => { table } | |||
| lookup_deprecation_entry => { table } | |||
| params_in_repr => { table } | |||
| unused_generic_params => { table } | |||
| unused_generic_params => { | |||
There was a problem hiding this comment.
You had a FIXME in the other PR, could you add this here as well? Would be nice to just default
There was a problem hiding this comment.
Oh also, is there a reason why the type doesn't implement that trait? Could you just add the impl? Or is there something to it
There was a problem hiding this comment.
For us to have a defaulted table, the value must implement FixedSizeEncoding and IsDefault. Lazy<T> can be FixedSizeEncoding, but can't be checked for defaultness because they're just a file offset.
I could instead encode the table value as Option<LazyValue<UnusedGenericParams>>, but then ProcessQueryValue<T> for Option<T> panics if the value decoded is None -- we could use specializtion to do:
impl ProcessQueryValue<'_, UnusedGenericParams> for Option<UnusedGenericParams> {
#[inline(always)]
fn process_decoded(self, _tcx: TyCtxt<'_>, _err: impl Fn() -> !) -> UnusedGenericParams {
self.unwrap_or_else(|| UnusedGenericParams::new_all_used())
}
}but I don't feel like using specialization here is compelling enough to save a few lines in the decoder rather than providing an explicit body here.
92d4290 to
ee2d428
Compare
|
r=me |
|
@bors r+ |
Rollup of 7 pull requests Successful merges: - rust-lang#108991 (add `enable-warnings` flag for llvm, and disable it by default.) - rust-lang#109109 (Use `unused_generic_params` from crate metadata) - rust-lang#109111 (Create dirs for build_triple) - rust-lang#109136 (Simplify proc macro signature validity check) - rust-lang#109150 (Update cargo) - rust-lang#109154 (Fix MappingToUnit to support no span of arg_ty) - rust-lang#109157 (Remove mw from review rotation for a while) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
|
Thanks for catching this - it seems like it could make a significant impact on the effectiveness of polymorphization in cross-crate scenarios. |
Due to the way that
separate_provide_externinteracted with the implementation of<ty::InstanceDef<'tcx> as Key>::query_crate_is_local, we actually never hit the foreign provider forunused_generic_params.Additionally, since the local provider of
unused_generic_paramscallsshould_polymorphize, which always returns false if the def-id is foreign, this means that we never actually polymorphize monomorphic instances originating from foreign crates.We don't actually encode
unused_generic_paramsfor items where all generics are used, so I had to tweak the foreign provider to fall back toty::UnusedGenericParams::new_all_used()to avoid more ICEs when the above bugs were fixed.