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

Compiler panics unexpectedly in 1.31 when constructing struct with wrong syntax #56611

Closed
chridou opened this issue Dec 7, 2018 · 13 comments
Closed
Assignees
Labels
A-typesystem Area: The type system I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️ P-high High priority regression-from-stable-to-stable Performance or correctness regression from one stable version to another. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@chridou
Copy link

chridou commented Dec 7, 2018

The compiler crashes when constructing a struct syntactically wrong.

Simply paste the code below into the playground.

I tried this code:

struct Foo {
    t: u32,
}

impl Foo {
    fn new(t: u32) -> Self {
        Self(t) // wrong. Should be Self{t}
    }
}

fn main() {
    println!("Hello, world!");
}

This is the compiler output:

Compiling playground v0.0.1 (/playground)
error[E0658]: `Self` struct constructors are unstable (see issue #51994)
 --> src/main.rs:7:9
  |
7 |         Self(t) // wrong. Should be Self{t}
  |         ^^^^

error: internal compiler error: librustc/hir/def.rs:265: attempted .def_id() on invalid def: SelfCtor(DefId(0/0:4 ~ playground[8d9a]::{{impl}}[0]))

thread 'main' panicked at 'Box<Any>', librustc_errors/lib.rs:600:9
note: Run with `RUST_BACKTRACE=1` for a backtrace.
error: aborting due to 2 previous errors

For more information about this error, try `rustc --explain E0658`.

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.31.0 (abe02cefd 2018-12-04) running on x86_64-unknown-linux-gnu

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

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

error: Could not compile `playground`.

To learn more, run the command again with --verbose.
@estebank estebank added A-typesystem Area: The type system I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️ regression-from-stable-to-stable Performance or correctness regression from one stable version to another. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. and removed regression-from-stable-to-stable Performance or correctness regression from one stable version to another. labels Dec 7, 2018
@estebank
Copy link
Contributor

estebank commented Dec 7, 2018

error: internal compiler error: src/librustc/hir/def.rs:265: attempted .def_id() on invalid def: SelfCtor(DefId(0/0:4 ~ playground[8317]::{{impl}}[0]))

thread 'main' panicked at 'Box<Any>', src/librustc_errors/lib.rs:600:9
stack backtrace:
   0: std::sys::unix::backtrace::tracing::imp::unwind_backtrace
             at src/libstd/sys/unix/backtrace/tracing/gcc_s.rs:49
   1: std::sys_common::backtrace::_print
             at src/libstd/sys_common/backtrace.rs:71
   2: std::panicking::default_hook::{{closure}}
             at src/libstd/sys_common/backtrace.rs:59
             at src/libstd/panicking.rs:211
   3: std::panicking::default_hook
             at src/libstd/panicking.rs:227
   4: rustc::util::common::panic_hook
   5: std::panicking::rust_panic_with_hook
             at src/libstd/panicking.rs:495
   6: std::panicking::begin_panic
   7: rustc_errors::Handler::bug
   8: rustc::util::bug::opt_span_bug_fmt::{{closure}}
   9: rustc::ty::context::tls::with_opt::{{closure}}
  10: rustc::ty::context::tls::with_context_opt
  11: rustc::ty::context::tls::with_opt
  12: rustc::util::bug::opt_span_bug_fmt
  13: rustc::util::bug::bug_fmt
  14: rustc::hir::def::Def::def_id::{{closure}}
  15: rustc::hir::def::Def::def_id
  16: rustc_typeck::check::callee::<impl rustc_typeck::check::FnCtxt<'a, 'gcx, 'tcx>>::confirm_builtin_call
  17: rustc_typeck::check::FnCtxt::check_expr_kind
  18: rustc_typeck::check::FnCtxt::check_expr_with_expectation_and_needs
  19: rustc_typeck::check::FnCtxt::check_block_with_expected
  20: rustc_typeck::check::FnCtxt::check_expr_kind
  21: rustc_typeck::check::FnCtxt::check_expr_with_expectation_and_needs
  22: rustc_typeck::check::FnCtxt::check_return_expr
  23: rustc_typeck::check::check_fn
  24: rustc::ty::context::tls::with_related_context
  25: rustc::infer::InferCtxtBuilder::enter
  26: rustc_typeck::check::typeck_tables_of
  27: rustc::ty::query::<impl rustc::ty::query::config::QueryAccessors<'tcx> for rustc::ty::query::queries::typeck_tables_of<'tcx>>::compute
  28: rustc::dep_graph::graph::DepGraph::with_task_impl
  29: <rustc::ty::query::plumbing::JobOwner<'a, 'tcx, Q>>::start
  30: rustc::ty::query::plumbing::<impl rustc::ty::context::TyCtxt<'a, 'gcx, 'tcx>>::force_query_with_job
  31: rustc::ty::query::plumbing::<impl rustc::ty::context::TyCtxt<'a, 'gcx, 'tcx>>::get_query
  32: rustc::ty::query::plumbing::<impl rustc::ty::context::TyCtxt<'a, 'gcx, 'tcx>>::ensure_query
  33: rustc::session::Session::track_errors
  34: rustc_typeck::check::typeck_item_bodies
  35: rustc::ty::query::<impl rustc::ty::query::config::QueryAccessors<'tcx> for rustc::ty::query::queries::typeck_item_bodies<'tcx>>::compute
  36: rustc::dep_graph::graph::DepGraph::with_task_impl
  37: <rustc::ty::query::plumbing::JobOwner<'a, 'tcx, Q>>::start
  38: rustc::ty::query::plumbing::<impl rustc::ty::context::TyCtxt<'a, 'gcx, 'tcx>>::force_query_with_job
  39: rustc::ty::query::plumbing::<impl rustc::ty::context::TyCtxt<'a, 'gcx, 'tcx>>::get_query
  40: rustc::util::common::time
  41: rustc_typeck::check_crate
  42: rustc::ty::context::tls::enter_context
  43: <std::thread::local::LocalKey<T>>::with
  44: rustc::ty::context::TyCtxt::create_and_enter
  45: rustc_driver::driver::compile_input
  46: rustc_driver::run_compiler_with_pool
  47: <scoped_tls::ScopedKey<T>>::set
  48: rustc_driver::run_compiler
  49: rustc_driver::monitor::{{closure}}
  50: __rust_maybe_catch_panic
             at src/libpanic_unwind/lib.rs:102
  51: rustc_driver::run
  52: rustc_driver::main
  53: std::rt::lang_start::{{closure}}
  54: std::panicking::try::do_call
             at src/libstd/rt.rs:59
             at src/libstd/panicking.rs:310
  55: __rust_maybe_catch_panic
             at src/libpanic_unwind/lib.rs:102
  56: std::rt::lang_start_internal
             at src/libstd/panicking.rs:289
             at src/libstd/panic.rs:398
             at src/libstd/rt.rs:58
  57: main
  58: __libc_start_main
  59: <unknown>
query stack during panic:
#0 [typeck_tables_of] processing `Foo::new`
#1 [typeck_item_bodies] type-checking all item bodies
end of query stack
error: aborting due to previous error

@alexreg
Copy link
Contributor

alexreg commented Dec 10, 2018

I'm presently looking into this. Let me know if you have thoughts please, @estebank.

@estebank
Copy link
Contributor

@alexreg the ICE happens because of the unconditional attempt to get the span for the given defid:

_ => self.tcx.hir().span_if_local(def.def_id())

You will need to add a check for SelfCtor and either forego the convenient span or find a way to get the defid (and subsequent span) of the current type (which might be hard to do, but should be something along the lines of impl <Trait> for <UNDERLINE_THIS> {).

I don't think this existing code already handles the "used parens instead of braces" case, but there's code to account for it which might have to also be adjusted.

@estebank estebank added the regression-from-stable-to-stable Performance or correctness regression from one stable version to another. label Dec 10, 2018
@estebank
Copy link
Contributor

Marking as regression because it affects stable (even though the feature that caused this can't be used, it still can be triggered).

@alexreg
Copy link
Contributor

alexreg commented Dec 10, 2018

@estebank Yep, after looking into this more myself, this was my conclusion too. That said, I found out that adding

tcx.sess.span_err(span, "the `Self` constructor can only be used with \
                         tuple structs");

above the line (impl_def_id, tcx.types.err) in the instantiate_value_path fn prevents that code path from even being hit. Maybe this is enough, or maybe I should add the SelfCtor branch too?

@alexreg
Copy link
Contributor

alexreg commented Dec 10, 2018

Okay, I think I've figured this out. Added a did you mean ...? label too.

bors added a commit that referenced this issue Dec 11, 2018
Implement RFC 2338, "Type alias enum variants"

This PR implements [RFC 2338](rust-lang/rfcs#2338), allowing one to write code like the following.

```rust
#![feature(type_alias_enum_variants)]

enum Foo {
    Bar(i32),
    Baz { i: i32 },
}

type Alias = Foo;

fn main() {
    let t = Alias::Bar(0);
    let t = Alias::Baz { i: 0 };
    match t {
        Alias::Bar(_i) => {}
        Alias::Baz { i: _i } => {}
    }
}
```

Since `Self` can be considered a type alias in this context, it also enables using `Self::Variant` as both a constructor and pattern.

Fixes issues #56199 and #56611.

N.B., after discussing the syntax for type arguments on enum variants with @petrochenkov and @eddyb (there are also a few comments on the [tracking issue](#49683)), the consensus seems to be treat the syntax as follows, which ought to be backwards-compatible.

```rust
Option::<u8>::None; // OK
Option::None::<u8>; // OK, but lint in near future (hard error next edition?)
Alias::<u8>::None; // OK
Alias::None::<u8>; // Error
```

I do not know if this will need an FCP, but let's start one if so.

r? @petrochenkov
@pnkfelix
Copy link
Member

compiler triage, marking P-high.

@alexreg, did you put up a PR for this? In any case, I will assign to @alexreg since it sounds like they made progress on this.

@pnkfelix pnkfelix added the P-high High priority label Dec 13, 2018
@pnkfelix
Copy link
Member

(hmm apparently I cannot assign to @alexreg. I'll assign to self in that case so I am reminded to either poke @alexreg or pick up the torch.)

@pnkfelix pnkfelix self-assigned this Dec 13, 2018
@alexreg
Copy link
Contributor

alexreg commented Dec 13, 2018

@pnkfelix Hmm, maybe because I'm not a team member? (Although I am an organisation member of course.) Anyway, the fix is in #55994 -- all ready & passing CI tests, just waiting for final review & merge.

@pnkfelix
Copy link
Member

pnkfelix commented Dec 20, 2018

visiting for triage. No change necessary to meta-data (still P-high, still fixed by PR #55994, which has not yet landed). But if anyone wants to have a go at factoring out a PR focused just on just fixing the ICE, the compiler team continues to be unopposed to such effort.

@pnkfelix
Copy link
Member

okay, in fact, this was fixed in nightly by the factored out PR #56850. The only questions are about how far to backport #56850. Leaving this open until any backporting questions are resolved.

bors added a commit that referenced this issue Dec 29, 2018
Implement RFC 2338, "Type alias enum variants"

This PR implements [RFC 2338](rust-lang/rfcs#2338), allowing one to write code like the following.

```rust
#![feature(type_alias_enum_variants)]

enum Foo {
    Bar(i32),
    Baz { i: i32 },
}

type Alias = Foo;

fn main() {
    let t = Alias::Bar(0);
    let t = Alias::Baz { i: 0 };
    match t {
        Alias::Bar(_i) => {}
        Alias::Baz { i: _i } => {}
    }
}
```

Since `Self` can be considered a type alias in this context, it also enables using `Self::Variant` as both a constructor and pattern.

Fixes issues #56199 and #56611.

N.B., after discussing the syntax for type arguments on enum variants with @petrochenkov and @eddyb (there are also a few comments on the [tracking issue](#49683)), the consensus seems to be treat the syntax as follows, which ought to be backwards-compatible.

```rust
Option::<u8>::None; // OK
Option::None::<u8>; // OK, but lint in near future (hard error next edition?)
Alias::<u8>::None; // OK
Alias::None::<u8>; // Error
```

I do not know if this will need an FCP, but let's start one if so.
@nagisa
Copy link
Member

nagisa commented Jan 3, 2019

@alexreg could a backport against beta be prepared?

@nagisa
Copy link
Member

nagisa commented Jan 3, 2019

Seems to have been backported via #57236.

@nagisa nagisa closed this as completed Jan 3, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-typesystem Area: The type system I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️ P-high High priority regression-from-stable-to-stable Performance or correctness regression from one stable version to another. 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

5 participants