instance: polymorphize shims #75414

davidtwco · 2020-08-11T17:37:16Z

This PR removes the restriction of InstanceDef::Item on polymorphization, so that shims can now be polymorphized. This won't have much of an effect until polymorphization's analysis is more advanced, but it doesn't hurt.

r? @lcnr

lcnr · 2020-08-11T17:43:40Z

What was the reason why previously didn't do this?

I don't know enough about how we deal with shims, so let's have @eddyb approve this.

r? @eddyb

davidtwco · 2020-08-11T17:47:06Z

What was the reason why previously didn't do this?

As I remember it, it meant we hit fewer ICEs and got things working quicker; I've ran all of the tests locally with polymorphization enabled and everything is passing (it occurs to be that I expected #75346 to have been necessary but apparently not).

eddyb · 2020-08-16T17:10:01Z

I'm... not sure about this. This is polymorphzing based on the DefId, which for some shims is always the same DefId (of something in libcore with a builtin impl), and some shims have some types in the InstanceDef computed from the Substs.

The ones I can think of that could work well are ReifyShim and VtableShim, as they are shims around a user function, rather than builtin implementation of something in libcore. There might also be one or two that are like this but for closures.

But either way we probably want an exhaustive match to decide whether to polymorphize.

davidtwco · 2020-08-17T16:12:19Z

I'm... not sure about this. This is polymorphzing based on the DefId, which for some shims is always the same DefId (of something in libcore with a builtin impl), and some shims have some types in the InstanceDef computed from the Substs.

The ones I can think of that could work well are ReifyShim and VtableShim, as they are shims around a user function, rather than builtin implementation of something in libcore. There might also be one or two that are like this but for closures.

But either way we probably want an exhaustive match to decide whether to polymorphize.

I've pushed a pretty big change to this PR so that unused_generic_params now takes an InstanceDef and can get the correct MIR for shims.

src/librustc_middle/ty/instance.rs

oli-obk · 2020-12-28T10:59:24Z

r? @oli-obk

compiler/rustc_middle/src/ty/instance.rs

oli-obk · 2020-12-28T11:41:41Z

Except for the naming bikeshed, the implementation looks good to me

This commit removes the restriction of `InstanceDef::Item` on polymorphization, so that shims can now be polymorphized. In addition, `unused_generic_params` now takes a `ty::InstanceDef` so that `instance_mir` can be invoked and actual shim MIR can be fetched (rather than whatever you get from the `instance.def_id()`'s MIR). Signed-off-by: David Wood <david@davidtw.co>

davidtwco

@oli-obk: Left some notes here about the changes I made while rebasing - as I've come to expect from polymorphization, it's never straightforward.

In addition to the changes noted below, there's a new ICE in a test whenever polymorphization is enabled:

---- [ui] ui/impl-trait/example-calendar.rs stdout ----

error: test compilation failed although it shouldn't!
status: signal: 6
command: "/home/david/Projects/rust/rust4/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "/home/david/Projects/rust/rust4/src/test/ui/impl-trait/example-calendar.rs" "-Zthreads=1" "--target=x86_64-unknown-linux-gnu" "--error-format" "json" "-Zui-testing" "-Zdeduplicate-diagnostics=no" "-Zemit-future-incompat-report" "-C" "prefer-dynamic" "-o" "/home/david/Projects/rust/rust4/build/x86_64-unknown-linux-gnu/test/ui/impl-trait/example-calendar/a" "-Crpath" "-O" "-Cdebuginfo=0" "-Zunstable-options" "-Lnative=/home/david/Projects/rust/rust4/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "-L" "/home/david/Projects/rust/rust4/build/x86_64-unknown-linux-gnu/test/ui/impl-trait/example-calendar/auxiliary"
stdout:
------------------------------------------

------------------------------------------
stderr:
------------------------------------------
rustc: /home/david/Projects/rust/rust4/src/llvm-project/llvm/include/llvm/IR/Instructions.h:1181: void llvm::ICmpInst::AssertOK(): Assertion `getOperand(0)->getType() == getOperand(1)->getType() && "Both operands to ICmp instruction are not of the same type!"' failed.

------------------------------------------

Fortunately, my to-do list hasn't gotten any longer because this is the same ICE as in #75737 (comment) which I intend to look into already - given that it has shown up here too, I suspect that this wasn't introduced by that PR and instead by something unrelated that has changed (e.g. an LLVM upgrade).

Given that, lets leave this PR unmerged until I can work out what's happening with that (annoyingly large) test.

davidtwco · 2021-01-13T22:36:31Z

compiler/rustc_mir/src/monomorphize/polymorphize.rs

+
+    // Without available MIR, polymorphization has nothing to analyze.
+    match tcx.hir().body_const_context(def_id.expect_local()) {
+        // FIXME(davidtwco): Disable polymorphization for any constant functions which, at the time


@oli-obk: I've had to change this to avoid polymorphizing constant functions (which was changed in #78407 prompting the most recent rebase).

When changed, it results in a cycle error from one test:

---- [ui] ui/const-generics/const_evaluatable_checked/infer-too-generic.rs stdout ---- error: test compilation failed although it shouldn't! status: exit code: 1 command: "/home/david/Projects/rust/rust4/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "/home/david/Projects/rust/rust4/src/test/ui/const-generics/const_evaluatable_checked/infer-too-generic.rs" "-Zthreads=1" "--target=x86_64-unknown-linux-gnu" "--error-format" "json" "-Zui-testing" "-Zdeduplicate-diagnostics=no" "-Zemit-future-incompat-report" "-C" "prefer-dynamic" "-o" "/home/david/Projects/rust/rust4/build/x86_64-unknown-linux-gnu/test/ui/const-generics/const_evaluatable_checked/infer-too-generic/a" "-Crpath" "-O" "-Cdebuginfo=0" "-Zunstable-options" "-Lnative=/home/david/Projects/rust/rust4/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "-L" "/home/david/Projects/rust/rust4/build/x86_64-unknown-linux-gnu/test/ui/const-generics/const_evaluatable_checked/infer-too-generic/auxiliary" stdout: ------------------------------------------ ------------------------------------------ stderr: ------------------------------------------ error[E0391]: cycle detected when determining which generic parameters are unused by `split_first::{constant#0}` --> /home/david/Projects/rust/rust4/src/test/ui/const-generics/const_evaluatable_checked/infer-too-generic.rs:9:9 | LL | [T; N - 1]: Sized, | ^^^^^ | = note: ...which again requires determining which generic parameters are unused by `split_first::{constant#0}`, completing the cycle note: cycle used when determining which generic parameters are unused by `split_first::{constant#3}` --> /home/david/Projects/rust/rust4/src/test/ui/const-generics/const_evaluatable_checked/infer-too-generic.rs:14:68 | LL | let tail = ptr::read(&arr[1..] as *const [T] as *const [T; N - 1]); | ^^^^^ error: aborting due to previous error For more information about this error, try `rustc --explain E0391`.

And in another, an ICE:

---- [ui] ui/const-generics/cross_crate_complex.rs#full stdout ---- error in revision `full`: auxiliary build of "/home/david/Projects/rust/rust4/src/test/ui/const-generics/auxiliary/crayte.rs" failed to compile: status: exit code: 101 command: "/home/david/Projects/rust/rust4/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "/home/david/Projects/rust/rust4/src/test/ui/const-generics/auxiliary/crayte.rs" "-Zthreads=1" "--target=x86_64-unknown-linux-gnu" "--cfg" "full" "--error-format" "json" "-Zui-testing" "-Zdeduplicate-diagnostics=no" "-Zemit-future-incompat-report" "-C" "prefer-dynamic" "--out-dir" "/home/david/Projects/rust/rust4/build/x86_64-unknown-linux-gnu/test/ui/const-generics/cross_crate_complex.full/auxiliary" "-Crpath" "-O" "-Cdebuginfo=0" "-Zunstable-options" "-Lnative=/home/david/Projects/rust/rust4/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "--edition=2018" "--crate-type" "dylib" "-L" "/home/david/Projects/rust/rust4/build/x86_64-unknown-linux-gnu/test/ui/const-generics/cross_crate_complex.full/auxiliary" stdout: ------------------------------------------ ------------------------------------------ stderr: ------------------------------------------ error: internal compiler error: compiler/rustc_typeck/src/collect/type_of.rs:212:17: associated type missing default --> /home/david/Projects/rust/rust4/src/test/ui/const-generics/auxiliary/crayte.rs:17:5 | LL | type Assoc: Foo<N>; | ^^^^^^^^^^^^^^^^^^^ thread 'rustc' panicked at 'Box<Any>', /home/david/Projects/rust/rust4/compiler/rustc_errors/src/lib.rs:904:9 stack backtrace: 0: std::panicking::begin_panic at ./library/std/src/panicking.rs:519:12 1: rustc_errors::HandlerInner::span_bug at ./compiler/rustc_errors/src/lib.rs:904:9 2: rustc_errors::Handler::span_bug at ./compiler/rustc_errors/src/lib.rs:627:9 3: rustc_middle::util::bug::opt_span_bug_fmt::{{closure}} at ./compiler/rustc_middle/src/util/bug.rs:33:40 4: rustc_middle::ty::context::tls::with_opt::{{closure}} at ./compiler/rustc_middle/src/ty/context.rs:1797:40 5: rustc_middle::ty::context::tls::with_context_opt at ./compiler/rustc_middle/src/ty/context.rs:1749:22 6: rustc_middle::ty::context::tls::with_opt at ./compiler/rustc_middle/src/ty/context.rs:1797:9 7: rustc_middle::util::bug::opt_span_bug_fmt at ./compiler/rustc_middle/src/util/bug.rs:30:5 8: rustc_middle::util::bug::span_bug_fmt at ./compiler/rustc_middle/src/util/bug.rs:21:5 9: rustc_typeck::collect::type_of::type_of 10: rustc_middle::ty::query::<impl rustc_query_system::query::config::QueryAccessors<rustc_middle::ty::context::TyCtxt> for rustc_middle::ty::query::queries::type_of>::compute at ./compiler/rustc_middle/src/ty/query/plumbing.rs:377:17 11: rustc_query_system::dep_graph::graph::DepGraph<K>::with_task_impl at ./compiler/rustc_query_system/src/dep_graph/graph.rs:362:14 12: rustc_query_system::query::plumbing::force_query_with_job::{{closure}}::{{closure}} 13: rustc_middle::ty::query::plumbing::<impl rustc_query_system::query::QueryContext for rustc_middle::ty::context::TyCtxt>::start_query::{{closure}}::{{closure}}::{{closure}} at ./compiler/rustc_middle/src/ty/query/plumbing.rs:73:74 14: stacker::maybe_grow at /home/david/.cargo/registry/src/git.luolix.top-1ecc6299db9ec823/stacker-0.1.12/src/lib.rs:55:9 15: rustc_data_structures::stack::ensure_sufficient_stack at ./compiler/rustc_data_structures/src/stack.rs:16:5 16: rustc_middle::ty::query::plumbing::<impl rustc_query_system::query::QueryContext for rustc_middle::ty::context::TyCtxt>::start_query::{{closure}}::{{closure}} at ./compiler/rustc_middle/src/ty/query/plumbing.rs:73:17 17: rustc_middle::ty::context::tls::enter_context::{{closure}} at ./compiler/rustc_middle/src/ty/context.rs:1732:50 18: rustc_middle::ty::context::tls::set_tlv at ./compiler/rustc_middle/src/ty/context.rs:1716:9 19: rustc_middle::ty::context::tls::enter_context at ./compiler/rustc_middle/src/ty/context.rs:1732:9 20: rustc_middle::ty::query::plumbing::<impl rustc_query_system::query::QueryContext for rustc_middle::ty::context::TyCtxt>::start_query::{{closure}} at ./compiler/rustc_middle/src/ty/query/plumbing.rs:72:13 21: rustc_middle::ty::context::tls::with_related_context::{{closure}} at ./compiler/rustc_middle/src/ty/context.rs:1776:13 22: rustc_middle::ty::context::tls::with_context::{{closure}} at ./compiler/rustc_middle/src/ty/context.rs:1760:40 23: rustc_middle::ty::context::tls::with_context_opt at ./compiler/rustc_middle/src/ty/context.rs:1749:22 24: rustc_middle::ty::context::tls::with_context at ./compiler/rustc_middle/src/ty/context.rs:1760:9 25: rustc_middle::ty::context::tls::with_related_context at ./compiler/rustc_middle/src/ty/context.rs:1773:9 26: rustc_middle::ty::query::plumbing::<impl rustc_query_system::query::QueryContext for rustc_middle::ty::context::TyCtxt>::start_query at ./compiler/rustc_middle/src/ty/query/plumbing.rs:61:9 27: rustc_query_system::query::plumbing::force_query_with_job::{{closure}} at ./compiler/rustc_query_system/src/query/plumbing.rs:597:9 28: rustc_query_system::query::plumbing::with_diagnostics at ./compiler/rustc_query_system/src/query/plumbing.rs:302:18 29: rustc_query_system::query::plumbing::force_query_with_job at ./compiler/rustc_query_system/src/query/plumbing.rs:596:51 30: rustc_query_system::query::plumbing::try_execute_query at ./compiler/rustc_query_system/src/query/plumbing.rs:426:16 31: rustc_query_system::query::plumbing::get_query_impl::{{closure}} at ./compiler/rustc_query_system/src/query/plumbing.rs:644:23 32: <rustc_query_system::query::caches::DefaultCache<K,V> as rustc_query_system::query::caches::QueryCache>::lookup at ./compiler/rustc_query_system/src/query/caches.rs:114:79 33: rustc_query_system::query::plumbing::try_get_cached at ./compiler/rustc_query_system/src/query/plumbing.rs:379:5 34: rustc_query_system::query::plumbing::get_query_impl at ./compiler/rustc_query_system/src/query/plumbing.rs:636:5 35: rustc_query_system::query::plumbing::get_query at ./compiler/rustc_query_system/src/query/plumbing.rs:738:5 36: rustc_middle::ty::query::TyCtxtAt::type_of at ./compiler/rustc_middle/src/ty/query/plumbing.rs:487:17 37: rustc_middle::ty::query::<impl rustc_middle::ty::context::TyCtxt>::type_of at ./compiler/rustc_middle/src/ty/query/plumbing.rs:448:17 38: rustc_mir::monomorphize::polymorphize::mark_used_by_default_parameters at ./compiler/rustc_mir/src/monomorphize/polymorphize.rs:145:60 39: rustc_mir::monomorphize::polymorphize::mark_used_by_default_parameters at ./compiler/rustc_mir/src/monomorphize/polymorphize.rs:160:9 40: rustc_mir::monomorphize::polymorphize::unused_generic_params at ./compiler/rustc_mir/src/monomorphize/polymorphize.rs:67:5 41: rustc_middle::ty::query::<impl rustc_query_system::query::config::QueryAccessors<rustc_middle::ty::context::TyCtxt> for rustc_middle::ty::query::queries::unused_generic_params>::compute at ./compiler/rustc_middle/src/ty/query/plumbing.rs:377:17 42: rustc_query_system::dep_graph::graph::DepGraph<K>::with_task_impl at ./compiler/rustc_query_system/src/dep_graph/graph.rs:362:14 43: rustc_query_system::dep_graph::graph::DepGraph<K>::with_task at ./compiler/rustc_query_system/src/dep_graph/graph.rs:245:9 44: rustc_query_system::query::plumbing::force_query_with_job::{{closure}}::{{closure}} at ./compiler/rustc_query_system/src/query/plumbing.rs:607:17 45: rustc_middle::ty::query::plumbing::<impl rustc_query_system::query::QueryContext for rustc_middle::ty::context::TyCtxt>::start_query::{{closure}}::{{closure}}::{{closure}} at ./compiler/rustc_middle/src/ty/query/plumbing.rs:73:74 46: stacker::maybe_grow at /home/david/.cargo/registry/src/git.luolix.top-1ecc6299db9ec823/stacker-0.1.12/src/lib.rs:55:9 47: rustc_data_structures::stack::ensure_sufficient_stack at ./compiler/rustc_data_structures/src/stack.rs:16:5 48: rustc_middle::ty::query::plumbing::<impl rustc_query_system::query::QueryContext for rustc_middle::ty::context::TyCtxt>::start_query::{{closure}}::{{closure}} at ./compiler/rustc_middle/src/ty/query/plumbing.rs:73:17 49: rustc_middle::ty::context::tls::enter_context::{{closure}} at ./compiler/rustc_middle/src/ty/context.rs:1732:50 50: rustc_middle::ty::context::tls::set_tlv at ./compiler/rustc_middle/src/ty/context.rs:1716:9 51: rustc_middle::ty::context::tls::enter_context at ./compiler/rustc_middle/src/ty/context.rs:1732:9 52: rustc_middle::ty::query::plumbing::<impl rustc_query_system::query::QueryContext for rustc_middle::ty::context::TyCtxt>::start_query::{{closure}} at ./compiler/rustc_middle/src/ty/query/plumbing.rs:72:13 53: rustc_middle::ty::context::tls::with_related_context::{{closure}} at ./compiler/rustc_middle/src/ty/context.rs:1776:13 54: rustc_middle::ty::context::tls::with_context::{{closure}} at ./compiler/rustc_middle/src/ty/context.rs:1760:40 55: rustc_middle::ty::context::tls::with_context_opt at ./compiler/rustc_middle/src/ty/context.rs:1749:22 56: rustc_middle::ty::context::tls::with_context at ./compiler/rustc_middle/src/ty/context.rs:1760:9 57: rustc_middle::ty::context::tls::with_related_context at ./compiler/rustc_middle/src/ty/context.rs:1773:9 58: rustc_middle::ty::query::plumbing::<impl rustc_query_system::query::QueryContext for rustc_middle::ty::context::TyCtxt>::start_query at ./compiler/rustc_middle/src/ty/query/plumbing.rs:61:9 59: rustc_query_system::query::plumbing::force_query_with_job::{{closure}} at ./compiler/rustc_query_system/src/query/plumbing.rs:597:9 60: rustc_query_system::query::plumbing::with_diagnostics at ./compiler/rustc_query_system/src/query/plumbing.rs:302:18 61: rustc_query_system::query::plumbing::force_query_with_job at ./compiler/rustc_query_system/src/query/plumbing.rs:596:51 62: rustc_query_system::query::plumbing::try_execute_query at ./compiler/rustc_query_system/src/query/plumbing.rs:426:16 63: rustc_query_system::query::plumbing::get_query_impl::{{closure}} at ./compiler/rustc_query_system/src/query/plumbing.rs:644:23 64: <rustc_query_system::query::caches::DefaultCache<K,V> as rustc_query_system::query::caches::QueryCache>::lookup at ./compiler/rustc_query_system/src/query/caches.rs:114:79 65: rustc_query_system::query::plumbing::try_get_cached at ./compiler/rustc_query_system/src/query/plumbing.rs:379:5 66: rustc_query_system::query::plumbing::get_query_impl at ./compiler/rustc_query_system/src/query/plumbing.rs:636:5 67: rustc_query_system::query::plumbing::get_query at ./compiler/rustc_query_system/src/query/plumbing.rs:738:5 68: rustc_middle::ty::query::TyCtxtAt::unused_generic_params at ./compiler/rustc_middle/src/ty/query/plumbing.rs:487:17 69: rustc_middle::ty::query::<impl rustc_middle::ty::context::TyCtxt>::unused_generic_params at ./compiler/rustc_middle/src/ty/query/plumbing.rs:448:17 70: rustc_metadata::rmeta::encoder::EncodeContext::encode_mir_for_ctfe at ./compiler/rustc_metadata/src/rmeta/encoder.rs:1142:22 71: rustc_metadata::rmeta::encoder::EncodeContext::encode_info_for_anon_const at ./compiler/rustc_metadata/src/rmeta/encoder.rs:1524:9 72: <rustc_metadata::rmeta::encoder::EncodeContext as rustc_hir::intravisit::Visitor>::visit_anon_const at ./compiler/rustc_metadata/src/rmeta/encoder.rs:1817:9 73: rustc_hir::intravisit::walk_generic_args at ./compiler/rustc_hir/src/intravisit.rs:774:5 74: rustc_hir::intravisit::Visitor::visit_generic_args at ./compiler/rustc_hir/src/intravisit.rs:464:9 75: rustc_hir::intravisit::walk_path_segment at ./compiler/rustc_hir/src/intravisit.rs:765:9 76: rustc_hir::intravisit::Visitor::visit_path_segment at ./compiler/rustc_hir/src/intravisit.rs:461:9 77: rustc_hir::intravisit::walk_path at ./compiler/rustc_hir/src/intravisit.rs:753:9 78: rustc_hir::intravisit::Visitor::visit_path at ./compiler/rustc_hir/src/intravisit.rs:458:9 79: rustc_hir::intravisit::walk_trait_ref at ./compiler/rustc_hir/src/intravisit.rs:554:5 80: rustc_hir::intravisit::Visitor::visit_trait_ref at ./compiler/rustc_hir/src/intravisit.rs:408:9 81: rustc_hir::intravisit::walk_poly_trait_ref at ./compiler/rustc_hir/src/intravisit.rs:549:5 82: rustc_hir::intravisit::Visitor::visit_poly_trait_ref at ./compiler/rustc_hir/src/intravisit.rs:414:9 83: rustc_hir::intravisit::walk_param_bound at ./compiler/rustc_hir/src/intravisit.rs:860:13 84: rustc_hir::intravisit::walk_trait_item at ./compiler/rustc_hir/src/intravisit.rs:988:13 85: rustc_hir::intravisit::Visitor::visit_trait_item at ./compiler/rustc_hir/src/intravisit.rs:393:9 86: <rustc_hir::intravisit::DeepVisitor<V> as rustc_hir::itemlikevisit::ItemLikeVisitor>::visit_trait_item at ./compiler/rustc_hir/src/intravisit.rs:61:9 87: rustc_hir::hir::Crate::visit_all_item_likes at ./compiler/rustc_hir/src/hir.rs:686:13 88: rustc_metadata::rmeta::encoder::EncodeContext::encode_info_for_items at ./compiler/rustc_metadata/src/rmeta/encoder.rs:443:9 89: rustc_metadata::rmeta::encoder::EncodeContext::encode_crate_root at ./compiler/rustc_metadata/src/rmeta/encoder.rs:578:9 90: rustc_metadata::rmeta::encoder::encode_metadata_impl at ./compiler/rustc_metadata/src/rmeta/encoder.rs:2160:16 91: rustc_metadata::rmeta::encoder::encode_metadata::{{closure}} at ./compiler/rustc_metadata/src/rmeta/encoder.rs:2100:12 92: rustc_data_structures::sync::join at ./compiler/rustc_data_structures/src/sync.rs:159:14 93: rustc_metadata::rmeta::encoder::encode_metadata at ./compiler/rustc_metadata/src/rmeta/encoder.rs:2099:5 94: rustc_metadata::rmeta::decoder::cstore_impl::<impl rustc_middle::middle::cstore::CrateStore for rustc_metadata::creader::CStore>::encode_metadata at ./compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs:530:9 95: rustc_middle::ty::context::TyCtxt::encode_metadata at ./compiler/rustc_middle/src/ty/context.rs:1295:9 96: rustc_interface::passes::encode_and_write_metadata at ./compiler/rustc_interface/src/passes.rs:965:66 97: rustc_interface::passes::start_codegen at ./compiler/rustc_interface/src/passes.rs:1010:44 98: rustc_interface::queries::Queries::ongoing_codegen::{{closure}}::{{closure}} at ./compiler/rustc_interface/src/queries.rs:286:20 99: rustc_interface::passes::QueryContext::enter::{{closure}} at ./compiler/rustc_interface/src/passes.rs:742:42 100: rustc_middle::ty::context::tls::enter_context::{{closure}} at ./compiler/rustc_middle/src/ty/context.rs:1732:50 101: rustc_middle::ty::context::tls::set_tlv at ./compiler/rustc_middle/src/ty/context.rs:1716:9 102: rustc_middle::ty::context::tls::enter_context at ./compiler/rustc_middle/src/ty/context.rs:1732:9 103: rustc_interface::passes::QueryContext::enter at ./compiler/rustc_interface/src/passes.rs:742:9 104: rustc_interface::queries::Queries::ongoing_codegen::{{closure}} at ./compiler/rustc_interface/src/queries.rs:277:13 105: rustc_interface::queries::Query<T>::compute at ./compiler/rustc_interface/src/queries.rs:39:28 106: rustc_interface::queries::Queries::ongoing_codegen at ./compiler/rustc_interface/src/queries.rs:275:9 107: rustc_driver::run_compiler::{{closure}}::{{closure}} at ./compiler/rustc_driver/src/lib.rs:446:13 108: rustc_interface::queries::<impl rustc_interface::interface::Compiler>::enter at ./compiler/rustc_interface/src/queries.rs:418:19 109: rustc_driver::run_compiler::{{closure}} at ./compiler/rustc_driver/src/lib.rs:341:22 110: rustc_interface::interface::create_compiler_and_run::{{closure}} at ./compiler/rustc_interface/src/interface.rs:197:13 111: rustc_span::with_source_map at ./compiler/rustc_span/src/lib.rs:787:5 112: rustc_interface::interface::create_compiler_and_run at ./compiler/rustc_interface/src/interface.rs:191:5 113: rustc_interface::interface::run_compiler::{{closure}} at ./compiler/rustc_interface/src/interface.rs:213:12 114: rustc_interface::util::setup_callbacks_and_run_in_thread_pool_with_globals::{{closure}}::{{closure}} at ./compiler/rustc_interface/src/util.rs:152:13 115: scoped_tls::ScopedKey<T>::set at /home/david/.cargo/registry/src/git.luolix.top-1ecc6299db9ec823/scoped-tls-1.0.0/src/lib.rs:137:9 116: rustc_span::with_session_globals at ./compiler/rustc_span/src/lib.rs:103:5 117: rustc_interface::util::setup_callbacks_and_run_in_thread_pool_with_globals::{{closure}} at ./compiler/rustc_interface/src/util.rs:150:9 118: rustc_interface::util::scoped_thread::{{closure}} at ./compiler/rustc_interface/src/util.rs:125:24 note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace. note: the compiler unexpectedly panicked. this is a bug. note: we would appreciate a bug report: https://github.com/rust-lang/rust/issues/new?labels=C-bug%2C+I-ICE%2C+T-compiler&template=ice.md note: rustc 1.51.0-dev running on x86_64-unknown-linux-gnu note: compiler flags: -Z threads=1 -Z ui-testing -Z deduplicate-diagnostics=no -Z emit-future-incompat-report -Z unstable-options -C prefer-dynamic -C rpath -C debuginfo=0 --crate-type dylib query stack during panic: #0 [type_of] computing type of `Bar::Assoc` #1 [unused_generic_params] determining which generic parameters are unused by `Bar::Assoc::{constant#0}` end of query stack error: aborting due to previous error ------------------------------------------

I'll spend some time looking into these but given that they were essentially regressions in polymorphization introduced from #78407 (unfortunately, polymorphization has a habit of producing the strangest errors so any change to polymorphization really needs to be tested with polymorphization enabled), I've just disabled checking constants for now so that this PR can proceed :)

oh... I did touch the _with_const_arg logic, I'll run tests with polymorphization and look into fixing that.

I did some digging. The cycle error with polymorphization occurs since this test was created 5 months ago, so it's not caused by my PR. Since it works otherwise, polymorphization will need duplicate the cycle prevention logic we have for wf-checks of const_evaluatable_checked

The ICE is also unrelated to my PR, but in contrast to the cycle, I know exactly how to fix it:

rust/compiler/rustc_mir/src/monomorphize/polymorphize.rs

Line 110 in ec7f258

if !tcx.is_trait(def_id) && (tcx.is_closure(def_id) || tcx.type_of(def_id).is_generator()) {

should be a match on tcx.def_kind(def_id) instead of invoking type_of, which can fail if the item has no type (like here, an associated type in a trait declaration)

And since I already had a worktree open for this: #82765

davidtwco · 2021-01-13T22:36:52Z

compiler/rustc_mir/src/monomorphize/polymorphize.rs

+    }
+
+    // Foreign items don't have a body to analyze.
+    if tcx.is_foreign_item(def_id) {


After rebasing, this line was necessary to avoid body_const_context from causing an ICE.

davidtwco · 2021-01-13T22:39:23Z

compiler/rustc_mir/src/monomorphize/polymorphize.rs

-    // would not be invoked, and the cross-crate metadata used instead).
-    if !def_id.is_local() {
+    // Exit early if this instance should not be polymorphized.
+    let def_id = instance.def_id();


Before rebasing, should_polymorphize invoked def_id_if_not_guaranteed_local_codegen instead of using this value. It returned true whenever def_id_if_not_guaranteed_local_codegen returned None, which meant that a ClosureOnceShim (particularly, the shim for FnOnce::fn_once) wasn't being checked for available MIR, which was causing an ICE after I rebased.

bors · 2021-02-02T15:20:03Z

☔ The latest upstream changes (presumably #81660) made this pull request unmergeable. Please resolve the merge conflicts.

JohnCSimon · 2021-05-09T19:01:27Z

ping from triage:
@davidtwco can you please post the status of this PR?

davidtwco · 2021-05-10T09:33:21Z

ping from triage:
@davidtwco can you please post the status of this PR?

I haven't had time to get back to it, if you'd prefer to close this, then I'll re-open a new PR when I've been able to resume work on this.

JohnCSimon · 2021-05-31T05:12:06Z

Triage: Closing for inactivity,
@davidtwco you're free to reopen this as necessary or start a new MR.

…edicates, r=lcnr polymorphization: shims and predicates Supersedes rust-lang#75737 and rust-lang#75414. This pull request includes up some changes to polymorphization which hadn't landed previously and gets stage2 bootstrapping and the test suite passing when polymorphization is enabled. There are still issues with `type_id` and polymorphization to investigate but this should get polymorphization in a reasonable state to work on. - rust-lang#75737 and rust-lang#75414 both worked but were blocked on having the rest of the test suite pass (with polymorphization enabled) with and without the PRs. It makes more sense to just land these so that the changes are in. - rust-lang#75737's changes remove the restriction of `InstanceDef::Item` on polymorphization, so that shims can now be polymorphized. This won't have much of an effect until polymorphization's analysis is more advanced, but it doesn't hurt. - rust-lang#75414's changes remove all logic which marks parameters as used based on their presence in predicates - given rust-lang#75675, this will enable more polymorphization and avoid the symbol clashes that predicate logic previously sidestepped. - Polymorphization now explicitly checks (and skips) foreign items, this is necessary for stage2 bootstrapping to work when polymorphization is enabled. - The conditional determining the emission of a note adding context to a post-monomorphization error has been modified. Polymorphization results in `optimized_mir` running for shims during collection where that wouldn't happen previously, some errors are emitted during `optimized_mir` and these were considered post-monomorphization errors with the existing logic (more errors and shims have a `DefId` coming from the std crate, not the local crate), adding a note that resulted in tests failing. It isn't particularly feasible to change where polymorphization runs or prevent it from using `optimized_mir`, so it seemed more reasonable to not change the conditional. - `characteristic_def_id_of_type` was being invoked during partitioning for self types of impl blocks which had projections that depended on the value of unused generic parameters of a function - this caused a ICE in a debuginfo test. If partitioning is enabled and the instance needs substitution then this is skipped. That test still fails for me locally, but not with an ICE, but it fails in a fresh checkout too, so 🤷‍♂️. r? `@lcnr`

…icates, r=lcnr polymorphization: shims and predicates Supersedes rust-lang#75737 and rust-lang#75414. This pull request includes up some changes to polymorphization which hadn't landed previously and gets stage2 bootstrapping and the test suite passing when polymorphization is enabled. There are still issues with `type_id` and polymorphization to investigate but this should get polymorphization in a reasonable state to work on. - rust-lang#75737 and rust-lang#75414 both worked but were blocked on having the rest of the test suite pass (with polymorphization enabled) with and without the PRs. It makes more sense to just land these so that the changes are in. - rust-lang#75737's changes remove the restriction of `InstanceDef::Item` on polymorphization, so that shims can now be polymorphized. This won't have much of an effect until polymorphization's analysis is more advanced, but it doesn't hurt. - rust-lang#75414's changes remove all logic which marks parameters as used based on their presence in predicates - given rust-lang#75675, this will enable more polymorphization and avoid the symbol clashes that predicate logic previously sidestepped. - Polymorphization now explicitly checks (and skips) foreign items, this is necessary for stage2 bootstrapping to work when polymorphization is enabled. - The conditional determining the emission of a note adding context to a post-monomorphization error has been modified. Polymorphization results in `optimized_mir` running for shims during collection where that wouldn't happen previously, some errors are emitted during `optimized_mir` and these were considered post-monomorphization errors with the existing logic (more errors and shims have a `DefId` coming from the std crate, not the local crate), adding a note that resulted in tests failing. It isn't particularly feasible to change where polymorphization runs or prevent it from using `optimized_mir`, so it seemed more reasonable to not change the conditional. - `characteristic_def_id_of_type` was being invoked during partitioning for self types of impl blocks which had projections that depended on the value of unused generic parameters of a function - this caused a ICE in a debuginfo test. If partitioning is enabled and the instance needs substitution then this is skipped. That test still fails for me locally, but not with an ICE, but it fails in a fresh checkout too, so 🤷‍♂️. r? `@lcnr`

rust-highfive assigned lcnr Aug 11, 2020

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 11, 2020

rust-highfive assigned eddyb and unassigned lcnr Aug 11, 2020

eddyb 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 Aug 16, 2020

davidtwco force-pushed the polymorphization-shims branch 2 times, most recently from 5e53797 to 4a5848c Compare August 17, 2020 16:11

davidtwco commented Aug 17, 2020

View reviewed changes

src/librustc_middle/ty/instance.rs Outdated Show resolved Hide resolved

davidtwco added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Aug 17, 2020

This comment has been minimized.

Sign in to view

davidtwco force-pushed the polymorphization-shims branch from 4a5848c to ff2c01a Compare August 30, 2020 18:17

jyn514 added -Zpolymorphize Unstable option: Polymorphization. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Sep 15, 2020

camelid added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 7, 2020

camelid added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 23, 2020

crlf0710 added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 13, 2020

JohnCSimon added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 30, 2020

crlf0710 added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Dec 18, 2020

rust-highfive assigned oli-obk and unassigned eddyb Dec 28, 2020

oli-obk reviewed Dec 28, 2020

View reviewed changes

compiler/rustc_middle/src/ty/instance.rs Outdated Show resolved Hide resolved

davidtwco 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 Dec 31, 2020

This comment has been minimized.

Sign in to view

davidtwco force-pushed the polymorphization-shims branch from ff2c01a to 22409f0 Compare January 13, 2021 22:31

davidtwco commented Jan 13, 2021

View reviewed changes

davidtwco mentioned this pull request Jan 13, 2021

polymorphize: remove predicate logic #75737

Closed

JohnCSimon 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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Feb 22, 2021

crlf0710 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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Mar 20, 2021

JohnCSimon 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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Apr 4, 2021

camelid 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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Apr 23, 2021

JohnCSimon added the S-inactive Status: Inactive and waiting on the author. This is often applied to closed PRs. label May 31, 2021

JohnCSimon closed this May 31, 2021

davidtwco mentioned this pull request Oct 4, 2021

polymorphization: shims and predicates #89514

Merged

davidtwco deleted the polymorphization-shims branch October 5, 2021 12:53

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

instance: polymorphize shims #75414

instance: polymorphize shims #75414

davidtwco commented Aug 11, 2020

lcnr commented Aug 11, 2020

davidtwco commented Aug 11, 2020

eddyb commented Aug 16, 2020

davidtwco commented Aug 17, 2020

This comment has been minimized.

oli-obk commented Dec 28, 2020

oli-obk commented Dec 28, 2020

This comment has been minimized.

davidtwco left a comment

davidtwco Jan 13, 2021 •

edited

Loading

oli-obk Jan 14, 2021

oli-obk Mar 4, 2021

oli-obk Mar 4, 2021

oli-obk Mar 4, 2021

davidtwco Jan 13, 2021

davidtwco Jan 13, 2021

bors commented Feb 2, 2021

JohnCSimon commented May 9, 2021

davidtwco commented May 10, 2021

JohnCSimon commented May 31, 2021

instance: polymorphize shims #75414

instance: polymorphize shims #75414

Conversation

davidtwco commented Aug 11, 2020

lcnr commented Aug 11, 2020

davidtwco commented Aug 11, 2020

eddyb commented Aug 16, 2020

davidtwco commented Aug 17, 2020

This comment has been minimized.

oli-obk commented Dec 28, 2020

oli-obk commented Dec 28, 2020

This comment has been minimized.

davidtwco left a comment

Choose a reason for hiding this comment

davidtwco Jan 13, 2021 • edited Loading

Choose a reason for hiding this comment

oli-obk Jan 14, 2021

Choose a reason for hiding this comment

oli-obk Mar 4, 2021

Choose a reason for hiding this comment

oli-obk Mar 4, 2021

Choose a reason for hiding this comment

oli-obk Mar 4, 2021

Choose a reason for hiding this comment

davidtwco Jan 13, 2021

Choose a reason for hiding this comment

davidtwco Jan 13, 2021

Choose a reason for hiding this comment

bors commented Feb 2, 2021

JohnCSimon commented May 9, 2021

davidtwco commented May 10, 2021

JohnCSimon commented May 31, 2021

davidtwco Jan 13, 2021 •

edited

Loading