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

ICE with the (foo @ ..,) pattern #74702

Closed
namibj opened this issue Jul 23, 2020 · 5 comments
Closed

ICE with the (foo @ ..,) pattern #74702

namibj opened this issue Jul 23, 2020 · 5 comments
Labels
C-bug Category: This is a bug. glacier ICE tracked in rust-lang/glacier. I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️ P-medium Medium priority T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@namibj
Copy link

namibj commented Jul 23, 2020

Closely related to #74539, where the foo @ .. was inside an EnumItemTuple pattern, instead of a plain Tuple pattern.

fn main() {
    let ( foo @ .. ,) = (0, 0);
    dbg!(foo);
}

(Playground)

rustc --version:

rustc 1.47.0-nightly (bbebe7351fcd29af1eb9 2020-07-22)

Errors:

  Compiling playground v0.0.1 (/playground)
error: `foo @` is not allowed in a tuple
 --> src/main.rs:2:11
  |
2 |     let ( foo @ .. ,) = (0, 0);
  |           ^^^^^^^^ this is only allowed in slice patterns
  |
  = help: remove this and bind each tuple field independently
help: if you don't need to use the contents of foo, discard the tuple's remaining fields
  |
2 |     let ( .. ,) = (0, 0);
  |           ^^

error: internal compiler error: src/librustc_typeck/check/mod.rs:3210:13: no type for local variable unknown node (hir_id=HirId { owner: DefId(0:3 ~ playground[9c6c]::main[0]), local_id: 2 })
 --> src/main.rs:3:10
  |
3 |     dbg!(foo);
  |          ^^^
Backtrace

thread 'rustc' panicked at 'Box<Any>', /rustc/bbebe7351fcd29af1eb9a35e315369b15887ea09/src/libstd/macros.rs:13:23
stack backtrace:
   0: std::backtrace_rs::backtrace::libunwind::trace
             at /rustc/bbebe7351fcd29af1eb9a35e315369b15887ea09/src/libstd/../backtrace/src/backtrace/libunwind.rs:96
   1: std::backtrace_rs::backtrace::trace_unsynchronized
             at /rustc/bbebe7351fcd29af1eb9a35e315369b15887ea09/src/libstd/../backtrace/src/backtrace/mod.rs:66
   2: std::sys_common::backtrace::_print_fmt
             at /rustc/bbebe7351fcd29af1eb9a35e315369b15887ea09/src/libstd/sys_common/backtrace.rs:77
   3: <std::sys_common::backtrace::_print::DisplayBacktrace as core::fmt::Display>::fmt
             at /rustc/bbebe7351fcd29af1eb9a35e315369b15887ea09/src/libstd/sys_common/backtrace.rs:58
   4: core::fmt::write
             at /rustc/bbebe7351fcd29af1eb9a35e315369b15887ea09/src/libcore/fmt/mod.rs:1117
   5: std::io::Write::write_fmt
             at /rustc/bbebe7351fcd29af1eb9a35e315369b15887ea09/src/libstd/io/mod.rs:1508
   6: std::sys_common::backtrace::_print
             at /rustc/bbebe7351fcd29af1eb9a35e315369b15887ea09/src/libstd/sys_common/backtrace.rs:61
   7: std::sys_common::backtrace::print
             at /rustc/bbebe7351fcd29af1eb9a35e315369b15887ea09/src/libstd/sys_common/backtrace.rs:48
   8: std::panicking::default_hook::{{closure}}
             at /rustc/bbebe7351fcd29af1eb9a35e315369b15887ea09/src/libstd/panicking.rs:198
   9: std::panicking::default_hook
             at /rustc/bbebe7351fcd29af1eb9a35e315369b15887ea09/src/libstd/panicking.rs:217
  10: rustc_driver::report_ice
  11: std::panicking::rust_panic_with_hook
             at /rustc/bbebe7351fcd29af1eb9a35e315369b15887ea09/src/libstd/panicking.rs:530
  12: std::panicking::begin_panic
  13: rustc_errors::HandlerInner::span_bug
  14: rustc_errors::Handler::span_bug
  15: rustc_middle::util::bug::opt_span_bug_fmt::{{closure}}
  16: rustc_middle::ty::context::tls::with_opt::{{closure}}
  17: rustc_middle::ty::context::tls::with_opt
  18: rustc_middle::util::bug::opt_span_bug_fmt
  19: rustc_middle::util::bug::span_bug_fmt
  20: rustc_typeck::check::FnCtxt::local_ty::{{closure}}
  21: rustc_typeck::check::FnCtxt::local_ty
  22: rustc_typeck::check::FnCtxt::instantiate_value_path
  23: rustc_typeck::check::expr::<impl rustc_typeck::check::FnCtxt>::check_expr_kind
  24: rustc_typeck::check::expr::<impl rustc_typeck::check::FnCtxt>::check_expr_with_expectation
  25: rustc_typeck::check::_match::<impl rustc_typeck::check::FnCtxt>::check_match
  26: rustc_typeck::check::expr::<impl rustc_typeck::check::FnCtxt>::check_expr_kind
  27: rustc_typeck::check::expr::<impl rustc_typeck::check::FnCtxt>::check_expr_with_expectation
  28: rustc_typeck::check::FnCtxt::check_stmt
  29: rustc_typeck::check::FnCtxt::check_block_with_expected
  30: rustc_typeck::check::expr::<impl rustc_typeck::check::FnCtxt>::check_expr_kind
  31: rustc_typeck::check::expr::<impl rustc_typeck::check::FnCtxt>::check_expr_with_expectation
  32: rustc_typeck::check::expr::<impl rustc_typeck::check::FnCtxt>::check_return_expr
  33: rustc_typeck::check::check_fn
  34: rustc_infer::infer::InferCtxtBuilder::enter
  35: rustc_typeck::check::typeck
  36: rustc_middle::ty::query::<impl rustc_query_system::query::config::QueryAccessors<rustc_middle::ty::context::TyCtxt> for rustc_middle::ty::query::queries::typeck>::compute
  37: rustc_query_system::dep_graph::graph::DepGraph<K>::with_task_impl
  38: rustc_data_structures::stack::ensure_sufficient_stack
  39: rustc_query_system::query::plumbing::get_query_impl
  40: rustc_query_system::query::plumbing::ensure_query_impl
  41: rustc_typeck::check::typeck_item_bodies
  42: rustc_middle::ty::query::<impl rustc_query_system::query::config::QueryAccessors<rustc_middle::ty::context::TyCtxt> for rustc_middle::ty::query::queries::typeck_item_bodies>::compute
  43: rustc_query_system::dep_graph::graph::DepGraph<K>::with_task_impl
  44: rustc_data_structures::stack::ensure_sufficient_stack
  45: rustc_query_system::query::plumbing::get_query_impl
  46: rustc_typeck::check_crate
  47: rustc_interface::passes::analysis
  48: rustc_middle::ty::query::<impl rustc_query_system::query::config::QueryAccessors<rustc_middle::ty::context::TyCtxt> for rustc_middle::ty::query::queries::analysis>::compute
  49: rustc_query_system::dep_graph::graph::DepGraph<K>::with_task_impl
  50: rustc_query_system::query::plumbing::get_query_impl
  51: rustc_middle::ty::context::tls::enter_global
  52: rustc_interface::queries::<impl rustc_interface::interface::Compiler>::enter
  53: rustc_span::with_source_map
  54: rustc_interface::interface::create_compiler_and_run
  55: scoped_tls::ScopedKey<T>::set
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.47.0-nightly (bbebe7351 2020-07-22) running on x86_64-unknown-linux-gnu

note: compiler flags: -C embed-bitcode=no -C codegen-units=1 -C debuginfo=2 --crate-type bin

note: some of the compiler flags provided by cargo are hidden

query stack during panic:
#0 [typeck] type-checking `main`
#1 [typeck_item_bodies] type-checking all item bodies
#2 [analysis] running analysis passes on this crate
end of query stack

@jonas-schievink jonas-schievink added C-bug Category: This is a bug. I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️ I-prioritize Issue: Indicates that prioritization has been requested for this issue. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jul 23, 2020
@petrochenkov
Copy link
Contributor

petrochenkov commented Jul 24, 2020

To the future fixer - resolve is not a good place to fix this issue, #74557 should probably be reverted and the fix should be applied to some other place (e.g. AST lowering).

UPD: #74692 will need to be reverted as well since it's based on #74557.

@jakubadamw
Copy link
Contributor

jakubadamw commented Jul 24, 2020

@petrochenkov, this would normally be invalid syntax, much like in this case. The only reason it is accepted as far as parsing, resolve and/or AST lowering is so that the diagnostics are more informative. Therefore, while the syntactic form is carried over further into the compiler pipeline for this very purpose, it seems reasonable to invalidate/ignore some of its effects (like, indeed, not resolve the binding it introduces) as early as possible. Or perhaps I'm missing something?

This isn't to say the fix was ideal, which #74692 demonstrates. 🙂 Sorry for that!

@rust-lang-glacier-bot rust-lang-glacier-bot added the glacier ICE tracked in rust-lang/glacier. label Jul 24, 2020
@petrochenkov
Copy link
Contributor

petrochenkov commented Jul 24, 2020

@jakubadamw
There are multiple reasons to catch this case later.

  • First, .. is a valid pattern syntactically so it should get through the parser without errors (RFC 2707).
  • Second, (head, tail @ ..) has a quite reasonable meaning - tail is the sub-tuple, even if it's not currently supported by the language (but see future possibilities in the RFC 2707), so from the error recovery point of view we could actually fully implement the reasonable semantics after formally reporting the error. For maximum recovery we should proceed as far as possible as if nothing erroneous happened (looks like now we proceed until AST lowering, if I'm not mistaken).
  • Third, when "foo @ is not allowed in a tuple (struct)" is reported in one place (AST lowering) and then this case is also treated specially in another place (resolve) it's easy for these two places to go out of sync (as this issue shows).
  • Finally, resolve doesn't care about semantic restrictions like this - it sees a name, it resolves the name, whether it's inside of a tuple or not is not resolve's concern or area of responsibility, so this check just looks out of place when put there.

@spastorino
Copy link
Member

Assigning P-medium as discussed as part of the Prioritization Working Group procedure and removing I-prioritize.

Also nominating for discussion given @petrochenkov's comments here and here.

@spastorino spastorino added I-nominated P-medium Medium priority and removed I-prioritize Issue: Indicates that prioritization has been requested for this issue. labels Jul 29, 2020
bors added a commit to rust-lang-ci/rust that referenced this issue Aug 2, 2020
Fix ICEs with `@ ..` binding

This reverts rust-lang#74557 and introduces an alternative fix while ensuring that rust-lang#74954 is not broken.
The diagnostics are verbose though, it fixes three related issues.
cc rust-lang#74954, rust-lang#74539, and rust-lang#74702
@JohnTitor
Copy link
Member

It's fixed by #74963. I'm going just to close this as fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: This is a bug. glacier ICE tracked in rust-lang/glacier. I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️ P-medium Medium priority T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

7 participants