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

Assertion failed in erroneous recursive types with methods returning a impl Trait type

Closed
hiro4bbh-ntt opened this issue May 25, 2020 · 10 comments · Fixed by #72718
Closed
Labels
A-codegen A-impl-trait C-bug E-needs-test glacier I-ICE T-compiler

Comments

@hiro4bbh-ntt
Copy link

hiro4bbh-ntt commented May 25, 2020

Summary

  • erroneous recursive types (without being wrapped with Box<_>) having methods whose return type is impl Trait causes the assertion failure.

I tried this code:

use std::collections::BTreeSet;

#[derive(Hash)]
pub enum ElemDerived {
    A(ElemDerived)
}

pub enum Elem {
    Derived(ElemDerived)
}

pub struct Set(BTreeSet<Elem>);

impl Set {
    pub fn into_iter(self) -> impl Iterator<Item = Elem> {
        self.0.into_iter()
    }
}

fn main() {}

I expected to see this happen:

  • I expected the error message "recursive type ElemDerived has infinite size."
  • Indeed, if there is no method with return type impl Trait, we got the following normal error messages:
    error[E0072]: recursive type `ElemDerived` has infinite size
    --> ./bug.rs:4:1
      |
    4 | pub enum ElemDerived {
      | ^^^^^^^^^^^^^^^^^^^^ recursive type has infinite size
    5 |     A(ElemDerived)
      |       ----------- recursive without indirection
      |
      = help: insert indirection (e.g., a `Box`, `Rc`, or `&`) at some point to make `ElemDerived` representable
    
    error: aborting due to previous error
    

Instead, this happened:

  • rustc aborted with the following assertion failure.
    % RUST_BACKTRACE=1 rustc ./bug.rs
    error: internal compiler error: src/librustc_infer/traits/codegen/mod.rs:108: Encountered errors `[FulfillmentError(Obligation(predicate=Binder(TraitPredicate(<[type error] as std::marker::Sized>)), depth=2),Ambiguity)]` resolving bounds after type-checking
    
    thread 'rustc' panicked at 'Box<Any>', src/librustc_errors/lib.rs:875:9
    stack backtrace:
      0: <std::sys_common::backtrace::_print::DisplayBacktrace as core::fmt::Display>::fmt
      1: core::fmt::write
      2: std::io::Write::write_fmt
      3: std::panicking::default_hook::{{closure}}
      4: std::panicking::default_hook
      5: rustc_driver::report_ice
      6: std::panicking::rust_panic_with_hook
      7: std::panicking::begin_panic
      8: rustc_errors::HandlerInner::bug
      9: rustc_errors::Handler::bug
      10: rustc::util::bug::opt_span_bug_fmt::{{closure}}
      11: rustc::ty::context::tls::with_opt::{{closure}}
      12: rustc::ty::context::tls::with_opt
      13: rustc::util::bug::opt_span_bug_fmt
      14: rustc::util::bug::bug_fmt
      15: rustc_infer::traits::codegen::<impl rustc_infer::infer::InferCtxt>::drain_fulfillment_cx_or_panic
      16: rustc::ty::context::GlobalCtxt::enter_local
      17: rustc_infer::traits::codegen::codegen_fulfill_obligation
      18: rustc::ty::query::__query_compute::codegen_fulfill_obligation
      19: rustc::ty::query::<impl rustc::ty::query::config::QueryAccessors for rustc::ty::query::queries::codegen_fulfill_obligation>::compute
      20: rustc::dep_graph::graph::DepGraph::with_task_impl
      21: rustc::ty::query::plumbing::<impl rustc::ty::context::TyCtxt>::get_query
      22: rustc_ty::instance::resolve_instance
      23: rustc::ty::instance::Instance::resolve
      24: rustc_mir_build::lints::check
      25: rustc::ty::context::GlobalCtxt::enter_local
      26: rustc_mir_build::build::mir_built
      27: rustc::ty::query::__query_compute::mir_built
      28: rustc::ty::query::<impl rustc::ty::query::config::QueryAccessors for rustc::ty::query::queries::mir_built>::compute
      29: rustc::dep_graph::graph::DepGraph::with_task_impl
      30: rustc::ty::query::plumbing::<impl rustc::ty::context::TyCtxt>::get_query
      31: rustc_mir::transform::check_unsafety::unsafety_check_result
      32: rustc::ty::query::__query_compute::unsafety_check_result
      33: rustc::ty::query::<impl rustc::ty::query::config::QueryAccessors for rustc::ty::query::queries::unsafety_check_result>::compute
      34: rustc::dep_graph::graph::DepGraph::with_task_impl
      35: rustc::ty::query::plumbing::<impl rustc::ty::context::TyCtxt>::get_query
      36: rustc_mir::transform::mir_const
      37: rustc::ty::query::__query_compute::mir_const
      38: rustc::ty::query::<impl rustc::ty::query::config::QueryAccessors for rustc::ty::query::queries::mir_const>::compute
      39: rustc::dep_graph::graph::DepGraph::with_task_impl
      40: rustc::ty::query::plumbing::<impl rustc::ty::context::TyCtxt>::get_query
      41: rustc_mir::transform::mir_validated
      42: rustc::ty::query::__query_compute::mir_validated
      43: rustc::ty::query::<impl rustc::ty::query::config::QueryAccessors for rustc::ty::query::queries::mir_validated>::compute
      44: rustc::dep_graph::graph::DepGraph::with_task_impl
      45: rustc::ty::query::plumbing::<impl rustc::ty::context::TyCtxt>::get_query
      46: rustc_mir::borrow_check::mir_borrowck
      47: rustc::ty::query::__query_compute::mir_borrowck
      48: rustc::ty::query::<impl rustc::ty::query::config::QueryAccessors for rustc::ty::query::queries::mir_borrowck>::compute
      49: rustc::dep_graph::graph::DepGraph::with_task_impl
      50: rustc::ty::query::plumbing::<impl rustc::ty::context::TyCtxt>::get_query
      51: rustc_typeck::collect::type_of::type_of
      52: rustc::ty::query::<impl rustc::ty::query::config::QueryAccessors for rustc::ty::query::queries::type_of>::compute
      53: rustc::dep_graph::graph::DepGraph::with_task_impl
      54: rustc::ty::query::plumbing::<impl rustc::ty::context::TyCtxt>::get_query
      55: rustc::ty::util::<impl rustc::ty::context::TyCtxt>::try_expand_impl_trait_type::OpaqueTypeExpander::expand_opaque_ty
      56: rustc::ty::util::<impl rustc::ty::context::TyCtxt>::try_expand_impl_trait_type
      57: rustc_typeck::check::check_item_type
      58: rustc::hir::map::Map::visit_item_likes_in_module
      59: rustc_typeck::check::check_mod_item_types
      60: rustc::ty::query::__query_compute::check_mod_item_types
      61: rustc::ty::query::<impl rustc::ty::query::config::QueryAccessors for rustc::ty::query::queries::check_mod_item_types>::compute
      62: rustc::dep_graph::graph::DepGraph::with_task_impl
      63: rustc::ty::query::plumbing::<impl rustc::ty::context::TyCtxt>::get_query
      64: rustc::ty::query::plumbing::<impl rustc::ty::context::TyCtxt>::ensure_query
      65: rustc_session::utils::<impl rustc_session::session::Session>::time
      66: rustc_typeck::check_crate
      67: rustc_interface::passes::analysis
      68: rustc::ty::query::__query_compute::analysis
      69: rustc::dep_graph::graph::DepGraph::with_task_impl
      70: rustc::ty::query::plumbing::<impl rustc::ty::context::TyCtxt>::get_query
      71: rustc::ty::context::tls::enter_global
      72: rustc_interface::interface::run_compiler_in_existing_thread_pool
    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/blob/master/CONTRIBUTING.md#bug-reports
    
    note: rustc 1.43.1 (8d69840ab 2020-05-04) running on x86_64-apple-darwin
    
    query stack during panic:
    #0 [codegen_fulfill_obligation] checking if `std::iter::IntoIterator` fulfills its obligations
    #1 [mir_built] building MIR for
    #2 [unsafety_check_result] unsafety-checking `Set::into_iter`
    #3 [mir_const] processing `Set::into_iter`
    #4 [mir_validated] processing `Set::into_iter`
    #5 [mir_borrowck] borrow-checking `Set::into_iter`
    #6 [type_of] processing `Set::into_iter::{{opaque}}#0`
    #7 [check_mod_item_types] checking item types in top-level module
    #8 [analysis] running analysis passes on this crate
    end of query stack
    error: aborting due to previous error
    

Meta

  • rustc --version --verbose:
    % rustc --version --verbose
    rustc 1.43.1 (8d69840ab 2020-05-04)
    binary: rustc
    commit-hash: 8d69840ab92ea7f4d323420088dd8c9775f180cd
    commit-date: 2020-05-04
    host: x86_64-apple-darwin
    release: 1.43.1
    LLVM version: 9.0
    
@JohnTitor JohnTitor added A-impl-trait C-bug I-ICE T-compiler A-codegen labels May 25, 2020
@JohnTitor

This comment has been minimized.

@rustbot

This comment has been minimized.

@JohnTitor

This comment has been minimized.

@rustbot

This comment has been minimized.

@JohnTitor

This comment has been minimized.

@rustbot
Copy link
Collaborator

rustbot commented May 25, 2020

Error: The feature glacier is not enabled in this repository.
To enable it add its section in the triagebot.toml in the root of the repository.

Please let @rust-lang/release know if you're having trouble with this bot.

@JohnTitor
Copy link
Member

JohnTitor commented May 25, 2020

The feature glacier is not enabled in this repository.

Oh, we must have enabled this... Sorry for the noise!

Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue May 25, 2020
Enable `glacier` command via triagebot

I noticed this was required by rust-lang#72554 (comment).
@JohnTitor
Copy link
Member

JohnTitor commented May 26, 2020

I think now we could...
@rustbot glacier "https://gist.github.com/rust-play/e9a2a667c7f9015fdf681a7e6a542d63"

@estebank estebank self-assigned this May 29, 2020
@estebank
Copy link
Contributor

estebank commented May 29, 2020

Making the following change:

fn drain_fulfillment_cx_or_panic<T>(
    infcx: &InferCtxt<'_, 'tcx>,
    fulfill_cx: &mut FulfillmentContext<'tcx>,
    result: &T,
) -> T
where
    T: TypeFoldable<'tcx>,
{
    debug!("drain_fulfillment_cx_or_panic()");

    // In principle, we only need to do this so long as `result`
    // contains unbound type parameters. It could be a slight
    // optimization to stop iterating early.
    if let Err(errors) = fulfill_cx.select_all_or_error(infcx) {
+        // `select_all_or_error` can return an empty `Vec<FulfillmentError>` when you have a bad
+        // interation of `impl Trait` in return position and a recursive ADT without indirection
+        // (#72554).
+        if !errors.is_empty() {
+            infcx.tcx.sess.delay_span_bug(
+                rustc_span::DUMMY_SP,
+                "`select_all_or_error` failed but no errors where present",
+            );
+        } else {
            bug!("encountered errors `{:?}` resolving bounds after type-checking", errors);
+        }
    }

    let result = infcx.resolve_vars_if_possible(result);
    infcx.tcx.erase_regions(&result)
}

makes the output look correct, but I am not sure of whether that should be the fix, making it always a delay_span_bug and continue returning the Ok<VTable> or make the caller return Err(ErrorReported) instead (which seems to me to be the correct behavior). I do not understand why this has to panic in the first place.

@estebank estebank removed their assignment May 29, 2020
estebank added a commit to estebank/rust that referenced this issue May 29, 2020
@rust-lang-glacier-bot rust-lang-glacier-bot added the glacier label May 29, 2020
@JohnTitor
Copy link
Member

JohnTitor commented May 31, 2020

Triage: Seems the ICE is fixed with the latest nightly, now recursive_type_with_infinite_size_error catches this correctly (not bisected yet). Marking as E-needs-test, cc @estebank for awareness.

@JohnTitor JohnTitor added the E-needs-test label May 31, 2020
estebank added a commit to estebank/rust that referenced this issue Jun 1, 2020
estebank added a commit to estebank/rust that referenced this issue Jun 1, 2020
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue Jun 3, 2020
bors added a commit to rust-lang-ci/rust that referenced this issue Jun 3, 2020
Rollup of 6 pull requests

Successful merges:

 - rust-lang#70946 (Add a lint to catch clashing `extern` fn declarations.)
 - rust-lang#72718 (Add regression test for rust-lang#72554)
 - rust-lang#72754 (remove trivial `mk_predicate`s)
 - rust-lang#72904 (Order the Rust and C ABIs first to reduce test churn)
 - rust-lang#72950 (fix `AdtDef` docs)
 - rust-lang#72951 (Add Camelid per request)

Failed merges:

r? @ghost
bors added a commit to rust-lang-ci/rust that referenced this issue Jun 4, 2020
Rollup of 6 pull requests

Successful merges:

 - rust-lang#72718 (Add regression test for rust-lang#72554)
 - rust-lang#72782 (rustc_target: Remove `pre_link_args_crt`)
 - rust-lang#72923 (Improve E0433, so that it suggests missing imports)
 - rust-lang#72950 (fix `AdtDef` docs)
 - rust-lang#72951 (Add Camelid per request)
 - rust-lang#72964 (Bump libc dependency to latest version (0.2.71))

Failed merges:

r? @ghost
@bors bors closed this as completed in 21ac561 Jun 4, 2020
ayushmishra2005 pushed a commit to ayushmishra2005/rust that referenced this issue Jun 5, 2020
Update src/librustc_resolve/diagnostics.rs

Co-authored-by: David Wood <Q0KPU0H1YOEPHRY1R2SN5B5RL@david.davidtw.co>

Minor refactoring rust-lang#72642

Fixed failing test-cases

remove trivial `mk_predicate`s

Make `SourceMap` available for early debug-printing of `Span`s

Normally, we debug-print `Spans` using the `SourceMap` retrieved from
the global `TyCtxt`. However, we fall back to printing out the `Span`'s
raw fields (instead of a file and line number) when we try to print a
`Span` before a `TyCtxt` is available. This makes debugging early phases
of the compile, such as parsing, much more difficult.

This commit stores a `SourceMap` in `rustc_span::GlOBALS` as a fallback.
When a `TyCtxt` is not available, we try to retrieve one from `GLOBALS`
- only if this is not available do we fall back to the raw field output.

I'm not sure how to write a test for this - however, this can be
verified locally by setting `RUSTC_LOG="rustc_parse=debug"`, and
verifying that the output contains filenames and line numbers.

Add test for rust-lang#72554.

rustc_target: Remove `pre_link_args_crt`

Improve E0433, so that it suggests missing imports

Fix a typo in `late.rs`

Co-authored-by: Esteban Kuber <estebank@users.noreply.github.com>

fix `AdtDef` docs

Add Camelid

Email from @camelid:

> HI there,
>
> I’m a new contributor and I just looked at Rust Thanks and noticed that my contributions are listed under two different capitalizations of my name: “Camelid" and “camelid". Could you make them both “Camelid"?
>
> Thanks!
>
> Camelid

save_analysis: work on HIR tree instead of AST

Update `rls` submodule

remove outdated fixme

Hexagon libstd: fix typo for c_ulonglong

Add more assert to Vec with_capacity docs

Show assertion on len too to show them how adding new items will affect both the
length and capacity, before and after.

Add Kyle Strand to mailmap

Fix missing word in RELEASES.md

Update cargo

Enable lld for Cargo tests on Windows.

Fixed failing test-cases rust-lang#72642
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-codegen A-impl-trait C-bug E-needs-test glacier I-ICE T-compiler
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants