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

Internal Compiler Error while compiling diesel #79560

Closed
weiznich opened this issue Nov 30, 2020 · 30 comments
Closed

Internal Compiler Error while compiling diesel #79560

weiznich opened this issue Nov 30, 2020 · 30 comments
Assignees
Labels
A-incr-comp Area: Incremental compilation A-query-system Area: The rustc query system (https://rustc-dev-guide.rust-lang.org/query.html) C-bug Category: This is a bug. E-needs-test Call for participation: An issue has been fixed and does not reproduce, but no test has been added. 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. WG-incr-comp Working group: Incremental compilation

Comments

@weiznich
Copy link
Contributor

Code

Diesel master

Meta

rustc --version --verbose:

rustc 1.50.0-nightly (349b3b324 2020-11-29)
binary: rustc
commit-hash: 349b3b324dade7ca638091db93ba08bbc443c63d
commit-date: 2020-11-29
host: x86_64-unknown-linux-gnu
release: 1.50.0-nightly

Last known working version: rust 1.50.0-nightly (b48cafd9e 2020-11-25) (Haven't tried the nightlies in between yet)

Error output

thread 'rustc' panicked at 'forcing query with already existing `DepNode`
- query-key: (DefId(0:1086 ~ diesel[f0f0]::expression::ValidGrouping), Some(IsAggregate#1483))
- dep-node: super_predicates_that_define_assoc_type(67abfa33ab7d90da-56b8cbe8d1ca79a9)', /rustc/349b3b324dade7ca638091db93ba08bbc443c63d\compiler\rustc_query_system\src\query\plumbing.rs:586:5
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

error: internal compiler error: unexpected panic
Error: internal compiler error: unexpected panic
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.50.0-nightly (349b3b324 2020-11-29) running on x86_64-pc-windows-msvc

note: compiler flags: -C embed-bitcode=no -C debuginfo=2 -C incremental --crate-type lib

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

query stack during panic:
#0 [explicit_predicates_of] computing explicit predicates of `type_impls::tuples::_::_::<impl expression::ValidGrouping<__GroupByClause> for (A, B, C)>`
#1 [predicates_defined_on] computing predicates of `type_impls::tuples::_::_::<impl expression::ValidGrouping<__GroupByClause> for (A, B, C)>`
end of query stack
error: could not compile `diesel`
Error: could not compile `diesel`
To learn more, run the command again with --verbose.
warning: build failed, waiting for other jobs to finish...
Warning: thread 'rustc' panicked at 'forcing query with already existing `DepNode`
- query-key: (DefId(0:1098 ~ diesel[8909]::expression::ValidGrouping), Some(IsAggregate#3574))
- dep-node: super_predicates_that_define_assoc_type(10b7a81c445a7cf0-f4bea10f7f9b919e)', /rustc/349b3b324dade7ca638091db93ba08bbc443c63d\compiler\rustc_query_system\src\query\plumbing.rs:586:5
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

As this is the second regression for diesel in less then a week: Would it possible to add diesel to the compiler test suite to prevent breaking diesel again and again. I would like to use my free time for something else than reporting regression bugs every day 😉

@rustbot modify labels: +regression-from-beta-to-nightly

@weiznich weiznich added C-bug Category: This is a bug. I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️ T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Nov 30, 2020
@matthiaskrgr
Copy link
Member

matthiaskrgr commented Nov 30, 2020

I would like to use my free time for something else than reporting regression bugs every day

Have you considered switching to the beta toolchain?

@weiznich
Copy link
Contributor Author

@matthiaskrgr As I'm the maintainer of diesel that's not an option for me. I think it's obvious that our users expect that diesel will work on any version of rustc + issues occurring on nightly will be in beta in max. 6 weeks + in stable in max. 12 weeks, therefore that wouldn't help in any way.

@matthiaskrgr
Copy link
Member

matthiaskrgr commented Nov 30, 2020

According to my bisection, 349b3b3 is the cause. cc @spastorino

searched toolchains 1c389ffeff814726dec325f0f2b0c99107df2673 through 28b86e0860f0593b85cda6c2c7b03ae8a582962f

********************************************************************************
Regression in 349b3b324dade7ca638091db93ba08bbc443c63d
********************************************************************************

@matthiaskrgr
Copy link
Member

@rustbot prioritize

@rustbot rustbot added the I-prioritize Issue: Indicates that prioritization has been requested for this issue. label Nov 30, 2020
@LeSeulArtichaut LeSeulArtichaut added the regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. label Nov 30, 2020
@hameerabbasi
Copy link
Contributor

hameerabbasi commented Nov 30, 2020

Introduced in PR #79209.

@hameerabbasi hameerabbasi added P-critical Critical priority and removed I-prioritize Issue: Indicates that prioritization has been requested for this issue. labels Nov 30, 2020
@hameerabbasi
Copy link
Contributor

Labelling P-critical according to the Zulip discussion. Many of the reasons were already outlined above.

@jyn514
Copy link
Member

jyn514 commented Nov 30, 2020

@weiznich you could make a PR to cargotest adding diesel. I don't think it would be a very controversial add, it exercises a lot more of the compiler than something like regex.

@jyn514 jyn514 added the A-query-system Area: The rustc query system (https://rustc-dev-guide.rust-lang.org/query.html) label Nov 30, 2020
@spastorino spastorino self-assigned this Nov 30, 2020
@spastorino
Copy link
Member

@rustbot ping cleanup

Would be nice to get an MCVE for this one.

@rustbot rustbot added the ICEBreaker-Cleanup-Crew Helping to "clean up" bugs with minimal examples and bisections label Nov 30, 2020
@rustbot
Copy link
Collaborator

rustbot commented Nov 30, 2020

Hey Cleanup Crew ICE-breakers! This bug has been identified as a good
"Cleanup ICE-breaking candidate". In case it's useful, here are some
instructions for tackling these sorts of bugs. Maybe take a look?
Thanks! <3

cc @AminArria @camelid @chrissimpkins @contrun @DutchGhost @elshize @ethanboxx @h-michael @HallerPatrick @hdhoang @hellow554 @imtsuki @JamesPatrickGill @kanru @KarlK90 @LeSeulArtichaut @MAdrianMattocks @matheus-consoli @mental32 @nmccarty @Noah-Kennedy @pard68 @PeytonT @pierreN @Redblueflame @RobbieClarken @RobertoSnap @robjtede @SarthakSingh31 @shekohex @sinato @smmalis37 @steffahn @Stupremee @tamuhey @turboladen @woshilapin @yerke

@spastorino spastorino added the E-needs-mcve Call for participation: This issue has a repro, but needs a Minimal Complete and Verifiable Example label Nov 30, 2020
@kvark
Copy link
Contributor

kvark commented Nov 30, 2020

Same issue affects gfx-rs repo. Looking forward to a fix!

@SNCPlay42
Copy link
Contributor

Small repro crate:

#[macro_use]
extern crate serde; //1.0.117
macro_rules! formats {
    {
        $($name:ident,)*
    } => {
        $(
            #[derive(Deserialize)]
            pub struct $name;
        )*
    }
}
formats! { Foo, Bar, }

Expanding the formats! macro doesn't repro the ICE, nor does expanding the Deserialize derive. Also I can't repro on playground.

@jyn514
Copy link
Member

jyn514 commented Nov 30, 2020

thread 'rustc' panicked at 'forcing query with already existing `DepNode`
- query-key: (DefId(18:1213 ~ serde[392b]::de::Deserializer), Some(Error#12))
- dep-node: super_predicates_that_define_assoc_type(f061438f2a3ccf95-d48f0f966cfa452)', /rustc/349b3b324dade7ca638091db93ba08bbc443c63d/compiler/rustc_query_system/src/query/plumbing.rs:586:5
Backtrace
stack backtrace:
   0: rust_begin_unwind
             at /rustc/349b3b324dade7ca638091db93ba08bbc443c63d/library/std/src/panicking.rs:493:5
   1: std::panicking::begin_panic_fmt
             at /rustc/349b3b324dade7ca638091db93ba08bbc443c63d/library/std/src/panicking.rs:435:5
   2: rustc_query_system::query::plumbing::get_query_impl
   3: rustc_infer::traits::util::transitive_bounds_that_define_assoc_type
   4: <dyn rustc_typeck::astconv::AstConv>::associated_path_to_ty
   5: <dyn rustc_typeck::astconv::AstConv>::ast_ty_to_ty_inner
   6: <<dyn rustc_typeck::astconv::AstConv>::create_substs_for_ast_path::SubstsForAstPathCtxt as rustc_typeck::astconv::CreateSubstsForGenericArgsCtxt>::provided_kind
   7: <dyn rustc_typeck::astconv::AstConv>::create_substs_for_ast_path
   8: <dyn rustc_typeck::astconv::AstConv>::ast_path_substs_for_ty
   9: <dyn rustc_typeck::astconv::AstConv>::res_to_ty
  10: <dyn rustc_typeck::astconv::AstConv>::ast_ty_to_ty_inner
  11: <dyn rustc_typeck::astconv::AstConv>::ty_of_fn
  12: rustc_typeck::collect::fn_sig
  13: rustc_middle::ty::query::<impl rustc_query_system::query::config::QueryAccessors<rustc_middle::ty::context::TyCtxt> for rustc_middle::ty::query::queries::fn_sig>::compute
  14: rustc_middle::dep_graph::<impl rustc_query_system::dep_graph::DepKind for rustc_middle::dep_graph::dep_node::DepKind>::with_deps
  15: rustc_query_system::dep_graph::graph::DepGraph<K>::with_task
  16: rustc_data_structures::stack::ensure_sufficient_stack
  17: rustc_query_system::query::plumbing::get_query_impl
  18: rustc_query_system::query::plumbing::ensure_query_impl
  19: <rustc_typeck::collect::CollectItemTypesVisitor as rustc_hir::intravisit::Visitor>::visit_impl_item
  20: rustc_middle::hir::map::Map::visit_item_likes_in_module
  21: rustc_typeck::collect::collect_mod_item_types
  22: rustc_middle::ty::query::<impl rustc_query_system::query::config::QueryAccessors<rustc_middle::ty::context::TyCtxt> for rustc_middle::ty::query::queries::collect_mod_item_types>::compute
  23: rustc_middle::dep_graph::<impl rustc_query_system::dep_graph::DepKind for rustc_middle::dep_graph::dep_node::DepKind>::with_deps
  24: rustc_query_system::dep_graph::graph::DepGraph<K>::with_task
  25: rustc_data_structures::stack::ensure_sufficient_stack
  26: rustc_query_system::query::plumbing::get_query_impl
  27: rustc_query_system::query::plumbing::ensure_query_impl
  28: rustc_typeck::check_crate
  29: rustc_interface::passes::analysis
  30: rustc_middle::ty::query::<impl rustc_query_system::query::config::QueryAccessors<rustc_middle::ty::context::TyCtxt> for rustc_middle::ty::query::queries::analysis>::compute
  31: rustc_middle::dep_graph::<impl rustc_query_system::dep_graph::DepKind for rustc_middle::dep_graph::dep_node::DepKind>::with_deps
  32: rustc_query_system::dep_graph::graph::DepGraph<K>::with_eval_always_task
  33: rustc_data_structures::stack::ensure_sufficient_stack
  34: rustc_query_system::query::plumbing::get_query_impl
  35: rustc_interface::passes::QueryContext::enter
  36: rustc_interface::queries::<impl rustc_interface::interface::Compiler>::enter
  37: rustc_span::with_source_map
  38: scoped_tls::ScopedKey<T>::set
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

error: internal compiler error: unexpected panic

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.50.0-nightly (349b3b324 2020-11-29) running on x86_64-unknown-linux-gnu

note: compiler flags: -C embed-bitcode=no -C debuginfo=2 -C incremental -C link-arg=-fuse-ld=lld --crate-type lib

note: some of the compiler flags provided by cargo are hidden
query stack during panic:
#0 [fn_sig] computing function signature of `_::<impl serde::Deserialize<'de> for Bar>::deserialize`
#1 [collect_mod_item_types] collecting item types in top-level module
#2 [analysis] running analysis passes on this crate
end of query stack

@jyn514 jyn514 removed E-needs-mcve Call for participation: This issue has a repro, but needs a Minimal Complete and Verifiable Example ICEBreaker-Cleanup-Crew Helping to "clean up" bugs with minimal examples and bisections labels Nov 30, 2020
@spastorino
Copy link
Member

@SNCPlay42 nice!, it would be nice to reduce this further without any external deps.

@SNCPlay42
Copy link
Contributor

Single crate repro:

trait Deserializer {
    type Error;
}

trait Deserialize {
    fn deserialize<D>(_: D) -> D::Error
    where D: Deserializer;
}

macro_rules! impl_deserialize {
    ($name:ident) => {
        impl Deserialize for $name {
            fn deserialize<D>(_: D) -> D::Error
            where D: Deserializer
            {
                loop {}
            }
        }
    }
}

macro_rules! formats {
    {
        $($name:ident,)*
    } => {
        $(
            pub struct $name;

            impl_deserialize!($name);
        )*
    }
}
formats! { Foo, Bar, }

It seems incremental compilation has to be enabled to repro the ICE, which is why it doesn't repro on playground.

@Aaron1011
Copy link
Member

This results from the fact that the HashStable implementation for SyntaxContext ignores ids (SyntaxContext and ExpnId), instead (recursively) looking at SyntaxContextData and ExpnData. This allows the stable hash to remain stable across compilation sessions (where we may end up with different SyntaxContext/ExpnId values, due to importing hygiene information from other crates).

Unfortunately, it turns out that we can have two distinct SyntaxContexts that end up having equivalent SyntaxContextData/ExpnData (ignoring orig_id). This is demonstrated by @SNCPlay42's example - the repetition expansion in formats! generates two distinct ExpnIds that have the same ExpnData (since they are both invoked from the same location in the source).

I'll need to think about how to adjust stable hashing to take this into account

@petrochenkov
Copy link
Contributor

DefPaths have a disambiguator counter for this purpose, perhaps such counter can be added to ExpnData as well, using orig_id to detect differences, for example.

@spastorino
Copy link
Member

@weiznich @kvark @workingjubilee @matthiaskrgr can you try #80732? where I'm re-introducing the thing that undercover this problem that ended affecting your code bases.

@camelid
Copy link
Member

camelid commented Jan 6, 2021

(See #80732 (comment) for how to try out the PR.)

@weiznich
Copy link
Contributor Author

weiznich commented Jan 6, 2021

@spastorino Thanks for the ping 👍. Diesel builds fine for me using the toolchain build from c963187c6f959417cbb13a33e9eaea4607696fc4.

m-ou-se added a commit to m-ou-se/rust that referenced this issue Jan 16, 2021
… r=nikomatsakis

Allow Trait inheritance with cycles on associated types take 2

This reverts the revert of rust-lang#79209 and fixes the ICEs that's occasioned by that PR exposing some problems that are addressed in rust-lang#80648 and rust-lang#79811.
For easier review I'd say, check only the last commit, the first one is just a revert of the revert of rust-lang#79209 which was already approved.

This also could be considered part or the actual fix of rust-lang#79560 but I guess for that to be closed and fixed completely we would need to land rust-lang#80648 and rust-lang#79811 too.

r? `@nikomatsakis`
cc `@Aaron1011`
bors added a commit to rust-lang-ci/rust that referenced this issue Jan 24, 2021
…chenkov

Add disambiugator to ExpnData

I still need to write a bunch of comments. Opening to see how bad the perf impact is.

cc rust-lang#79560
@pnkfelix pnkfelix self-assigned this Jan 26, 2021
@pnkfelix
Copy link
Member

main remaining work item we've identified is to consider adding a build of diesel to CI. I'll follow up on that.

@weiznich
Copy link
Contributor Author

@pnkfelix I've already opened #79599 for that a while ago. Maybe it's as simple as just reopen and merge that one.

Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue Feb 9, 2021
… r=nikomatsakis

Allow Trait inheritance with cycles on associated types take 2

This reverts the revert of rust-lang#79209 and fixes the ICEs that's occasioned by that PR exposing some problems that are addressed in rust-lang#80648 and rust-lang#79811.
For easier review I'd say, check only the last commit, the first one is just a revert of the revert of rust-lang#79209 which was already approved.

This also could be considered part or the actual fix of rust-lang#79560 but I guess for that to be closed and fixed completely we would need to land rust-lang#80648 and rust-lang#79811 too.

r? `@nikomatsakis`
cc `@Aaron1011`
@spastorino spastorino removed their assignment Feb 9, 2021
@spastorino
Copy link
Member

The original issue is now fixed and if I'm not wrong @pnkfelix wants to keep this issue open to add a build of diesel to CI.

@jyn514 jyn514 added the E-needs-test Call for participation: An issue has been fixed and does not reproduce, but no test has been added. label Feb 11, 2021
@Aaron1011 Aaron1011 removed their assignment Feb 20, 2021
bors pushed a commit to rust-lang-ci/rust that referenced this issue Apr 3, 2021
This was proposed in rust-lang#79459 and rust-lang#79560.
Diesel stresses the trait system implementation quite a lot and there
have been various regressions regarding that in the past.
bors added a commit to rust-lang-ci/rust that referenced this issue Apr 3, 2021
…Mark-Simulacrum

Adding diesel to the cargotest suite

As discussed in rust-lang#79560 (comment) this adds diesel to the compilers test suite. This is basically a reopened version of rust-lang#79599, but now with the backing of the compiler team.

r? `@pnkfelix`
@weiznich
Copy link
Contributor Author

This likely can be closed as diesel is part of the rustc test suite since 99a33b3

@spastorino
Copy link
Member

Thanks @weiznich, closing this issue then.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-incr-comp Area: Incremental compilation A-query-system Area: The rustc query system (https://rustc-dev-guide.rust-lang.org/query.html) C-bug Category: This is a bug. E-needs-test Call for participation: An issue has been fixed and does not reproduce, but no test has been added. 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. WG-incr-comp Working group: Incremental compilation
Projects
None yet
Development

No branches or pull requests