Bump chalk #4982

lnicola · 2020-06-22T12:17:09Z

This passes the tests, but fails in a weird way on analysis-stats:

thread 'main' panicked at 'called `Option::unwrap()` on a `None` value', /home/me/.cargo/registry/src/github.com-1ecc6299db9ec823/chalk-solve-0.14.0/src/clauses/builtin_traits/fn_family.rs:96:20
stack backtrace:
   0: backtrace::backtrace::libunwind::trace
             at /cargo/registry/src/github.com-1ecc6299db9ec823/backtrace-0.3.46/src/backtrace/libunwind.rs:86
   1: backtrace::backtrace::trace_unsynchronized
             at /cargo/registry/src/github.com-1ecc6299db9ec823/backtrace-0.3.46/src/backtrace/mod.rs:66
   2: std::sys_common::backtrace::_print_fmt
             at src/libstd/sys_common/backtrace.rs:78
   3: <std::sys_common::backtrace::_print::DisplayBacktrace as core::fmt::Display>::fmt
             at src/libstd/sys_common/backtrace.rs:59
   4: core::fmt::write
             at src/libcore/fmt/mod.rs:1076
   5: std::io::Write::write_fmt
             at src/libstd/io/mod.rs:1537
   6: std::sys_common::backtrace::_print
             at src/libstd/sys_common/backtrace.rs:62
   7: std::sys_common::backtrace::print
             at src/libstd/sys_common/backtrace.rs:49
   8: std::panicking::default_hook::{{closure}}
             at src/libstd/panicking.rs:198
   9: std::panicking::default_hook
             at src/libstd/panicking.rs:218
  10: std::panicking::rust_panic_with_hook
             at src/libstd/panicking.rs:477
  11: rust_begin_unwind
             at src/libstd/panicking.rs:385
  12: core::panicking::panic_fmt
             at src/libcore/panicking.rs:86
  13: core::panicking::panic
             at src/libcore/panicking.rs:51
  14: chalk_solve::clauses::builtin_traits::fn_family::add_fn_trait_program_clauses
  15: chalk_solve::clauses::builder::ClauseBuilder<I>::push_binders
  16: chalk_solve::clauses::program_clauses_that_could_match
  17: chalk_solve::clauses::program_clauses_for_goal
  18: chalk_solve::recursive::Solver<I>::solve_new_subgoal
  19: <chalk_solve::recursive::Solver<I> as chalk_solve::recursive::solve::SolveDatabase<I>>::solve_goal
  20: chalk_solve::recursive::Solver<I>::solve_root_goal
  21: chalk_solve::solve::Solver<I>::solve_limited
  22: ra_hir_ty::traits::trait_solve_query
  23: salsa::runtime::Runtime<DB>::execute_query_implementation
  24: salsa::derived::slot::Slot<DB,Q,MP>::read_upgrade
  25: salsa::derived::slot::Slot<DB,Q,MP>::read
  26: <salsa::derived::DerivedStorage<DB,Q,MP> as salsa::plumbing::QueryStorageOps<DB,Q>>::try_fetch
  27: salsa::QueryTable<DB,Q>::get
  28: <T as ra_hir_ty::db::HirDatabase>::trait_solve
  29: ra_hir_ty::infer::InferenceContext::resolve_ty_as_possible
  30: ra_hir_ty::infer::expr::<impl ra_hir_ty::infer::InferenceContext>::infer_expr_inner
  31: ra_hir_ty::infer::expr::<impl ra_hir_ty::infer::InferenceContext>::infer_expr_coerce
  32: ra_hir_ty::infer::expr::<impl ra_hir_ty::infer::InferenceContext>::check_call_arguments
  33: ra_hir_ty::infer::expr::<impl ra_hir_ty::infer::InferenceContext>::infer_method_call
  34: ra_hir_ty::infer::expr::<impl ra_hir_ty::infer::InferenceContext>::infer_expr_inner
  35: ra_hir_ty::infer::expr::<impl ra_hir_ty::infer::InferenceContext>::infer_expr
  36: ra_hir_ty::infer::expr::<impl ra_hir_ty::infer::InferenceContext>::infer_method_call
  37: ra_hir_ty::infer::expr::<impl ra_hir_ty::infer::InferenceContext>::infer_expr_inner
  38: ra_hir_ty::infer::expr::<impl ra_hir_ty::infer::InferenceContext>::infer_expr_coerce
  39: ra_hir_ty::infer::expr::<impl ra_hir_ty::infer::InferenceContext>::infer_expr_inner
  40: ra_hir_ty::infer::expr::<impl ra_hir_ty::infer::InferenceContext>::infer_expr_inner
  41: ra_hir_ty::infer::expr::<impl ra_hir_ty::infer::InferenceContext>::infer_expr_coerce
  42: ra_hir_ty::infer::expr::<impl ra_hir_ty::infer::InferenceContext>::infer_expr_inner
  43: ra_hir_ty::infer::expr::<impl ra_hir_ty::infer::InferenceContext>::infer_expr_coerce
  44: ra_hir_ty::infer::infer_query
  45: salsa::runtime::Runtime<DB>::execute_query_implementation
  46: salsa::derived::slot::Slot<DB,Q,MP>::read_upgrade
  47: salsa::derived::slot::Slot<DB,Q,MP>::read
  48: <salsa::derived::DerivedStorage<DB,Q,MP> as salsa::plumbing::QueryStorageOps<DB,Q>>::try_fetch
  49: ra_hir_ty::db::infer_wait
  50: rust_analyzer::cli::analysis_stats::analysis_stats
  51: rust_analyzer::main
  52: std::rt::lang_start::{{closure}}
  53: std::rt::lang_start_internal::{{closure}}
             at src/libstd/rt.rs:52
  54: std::panicking::try::do_call
             at src/libstd/panicking.rs:297
  55: std::panicking::try
             at src/libstd/panicking.rs:274
  56: std::panic::catch_unwind
             at src/libstd/panic.rs:394
  57: std::rt::lang_start_internal
             at src/libstd/rt.rs:51
  58: main
  59: __libc_start_main
  60: _start
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

It's as if we're returning None for a well-known trait that chalk asks about. That seems to happen for Deref, CoerceUnsized and Try.

r? @flodiebold

crates/ra_hir_ty/src/traits/chalk.rs

flodiebold · 2020-06-22T13:31:00Z

It's as if we're returning None for a well-known trait that chalk asks about. That seems to happen for Deref, CoerceUnsized and Try.

What do you mean? Do these even exist as well-known traits in Chalk?

lnicola · 2020-06-22T13:46:06Z

What do you mean? Do these even exist as well-known traits in Chalk?

Ah, never mind those. I think the problem is that here lang_items.target("fn_mut") returns None.

lnicola · 2020-06-22T13:53:34Z

It looks like lang_item is empty. Maybe we pass the wrong crate to self.db.crate_lang_items(self.krate)?

flodiebold · 2020-06-22T13:58:50Z

Ah, that should be self.db.lang_item(self.krate, lang_attr) 😩 crate_lang_items is not transitive. I think I made that exact mistake once already 🤦

flodiebold · 2020-06-22T13:59:31Z

Maybe we should rename crate_lang_items to something like lang_items_defined_in_crate.

lnicola · 2020-06-22T14:27:36Z

Done, but after a rebase it crashes with

thread 'main' panicked at 'assertion failed: `(left == right)`
  left: `^1`,
 right: `^0`', /home/me/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/src/libstd/macros.rs:16:9
stack backtrace:
   0: backtrace::backtrace::libunwind::trace
             at /cargo/registry/src/github.com-1ecc6299db9ec823/backtrace-0.3.46/src/backtrace/libunwind.rs:86
   1: backtrace::backtrace::trace_unsynchronized
             at /cargo/registry/src/github.com-1ecc6299db9ec823/backtrace-0.3.46/src/backtrace/mod.rs:66
   2: std::sys_common::backtrace::_print_fmt
             at src/libstd/sys_common/backtrace.rs:78
   3: <std::sys_common::backtrace::_print::DisplayBacktrace as core::fmt::Display>::fmt
             at src/libstd/sys_common/backtrace.rs:59
   4: core::fmt::write
             at src/libcore/fmt/mod.rs:1076
   5: std::io::Write::write_fmt
             at src/libstd/io/mod.rs:1537
   6: std::sys_common::backtrace::_print
             at src/libstd/sys_common/backtrace.rs:62
   7: std::sys_common::backtrace::print
             at src/libstd/sys_common/backtrace.rs:49
   8: std::panicking::default_hook::{{closure}}
             at src/libstd/panicking.rs:198
   9: std::panicking::default_hook
             at src/libstd/panicking.rs:218
  10: std::panicking::rust_panic_with_hook
             at src/libstd/panicking.rs:477
  11: rust_begin_unwind
             at src/libstd/panicking.rs:385
  12: std::panicking::begin_panic_fmt
             at src/libstd/panicking.rs:339
  13: <&chalk_ir::SubstFolder<I> as chalk_ir::fold::Folder<I>>::fold_free_var_ty
  14: <chalk_ir::Ty<I> as chalk_ir::fold::SuperFold<I,TI>>::super_fold_with
  15: chalk_ir::_DERIVE_chalk_ir_fold_Fold_I_TI_FOR_DomainGoal::<impl chalk_ir::fold::Fold<I,_TI> for chalk_ir::DomainGoal<I>>::fold_with
  16: chalk_ir::_DERIVE_chalk_ir_fold_Fold_I_TI_FOR_ProgramClauseImplication::<impl chalk_ir::fold::Fold<I,_TI> for chalk_ir::ProgramClauseImplication<I>>::fold_with
  17: chalk_solve::infer::instantiate::<impl chalk_solve::infer::InferenceTable<I>>::instantiate_binders_existentially
  18: chalk_solve::recursive::fulfill::Fulfill<I,Solver,Infer>::new_with_clause
  19: chalk_solve::recursive::Solver<I>::solve_new_subgoal
  20: <chalk_solve::recursive::Solver<I> as chalk_solve::recursive::solve::SolveDatabase<I>>::solve_goal
  21: chalk_solve::recursive::fulfill::Fulfill<I,Solver,Infer>::prove
  22: chalk_solve::recursive::fulfill::Fulfill<I,Solver,Infer>::solve
  23: chalk_solve::recursive::Solver<I>::solve_new_subgoal
  24: <chalk_solve::recursive::Solver<I> as chalk_solve::recursive::solve::SolveDatabase<I>>::solve_goal
  25: chalk_solve::recursive::Solver<I>::solve_root_goal
  26: chalk_solve::solve::Solver<I>::solve_limited
  27: ra_hir_ty::traits::trait_solve_query
  28: salsa::runtime::Runtime<DB>::execute_query_implementation
  29: salsa::derived::slot::Slot<DB,Q,MP>::read_upgrade
  30: salsa::derived::slot::Slot<DB,Q,MP>::read
  31: <salsa::derived::DerivedStorage<DB,Q,MP> as salsa::plumbing::QueryStorageOps<DB,Q>>::try_fetch
  32: salsa::QueryTable<DB,Q>::get
  33: <T as ra_hir_ty::db::HirDatabase>::trait_solve
  34: ra_hir_ty::infer::InferenceContext::resolve_ty_as_possible

flodiebold · 2020-06-22T18:29:59Z

Hmm I think that's a bug in Chalk. Looking into it.

There were two problems: - the FnDef impl was using a wrong substitution for the FnDef (calling `builder.substitution_in_scope()`, which wasn't even necessarily the same number of parameters -- not sure what the intention was there) - when looking for `Normalize` clauses, the self type wasn't generalized (so bound variables were handled wrongly). (This lead to crashes when trying to integrate it in rust-analyzer: rust-lang/rust-analyzer#4982)

flodiebold · 2020-06-23T21:19:15Z

I think that Chalk PR should fix it.

There were two problems: - the FnDef impl was using a wrong substitution for the FnDef (calling `builder.substitution_in_scope()`, which wasn't even necessarily the same number of parameters -- not sure what the intention was there) - when looking for `Normalize` clauses, the self type wasn't generalized (so bound variables were handled wrongly). (This lead to crashes when trying to integrate it in rust-analyzer: rust-lang/rust-analyzer#4982)

lnicola · 2020-06-24T08:07:06Z

It does!

I updated the PR with a git dependency, but we can wait until the Sunday release instead.

matklad · 2020-06-24T08:18:18Z

but we can wait until the Sunday release instead.

Hm, long-term, this is not the pattern I'd love to see.... I'd say its pretty crucial to be able to apply improvements immediately, without waiting (as blocking easily accumulates). But that interacts with vendoring....

lnicola · 2020-06-24T08:21:34Z

Expressions of unknown type: 2070 (1%)
Expressions of partially unknown type: 2243 (1%)
Type mismatches: 780

Expressions of unknown type: 1581 (0%)
Expressions of partially unknown type: 1098 (0%)
Type mismatches: 593

I've been watching those percentages drop towards 0 since the beginning of 2019 😀.

But that interacts with vendoring....

rustc already uses chalk, so I guess it would be nice to use the same version.. OTOH we (might want to) upgrade more often.

cynecx · 2020-06-25T14:52:14Z

Unfortunately, this doesn't fix #2880.

lnicola · 2020-06-25T14:56:32Z

This doesn't implement closures.

flodiebold · 2020-06-25T14:58:25Z

It shouldn't need to, since we still have our own implementation of the built-in impl for that -- but isn't #2880 about function pointers? 🤔

cynecx · 2020-06-25T21:38:19Z

Sorry, I just had the impression that we’ve had support for that (per #4493).

flodiebold · 2020-06-26T12:18:08Z

Ah wait, no, we do indeed not have built-in impls of Fn for function pointers (#4493 didn't add them because I didn't want to add them on our side), and the Chalk update should indeed provide them. 🤔

flodiebold · 2020-06-26T13:12:00Z

I've found the problem -- we're lowering the type parameter default wrongly, with a placeholder where a variable should be, so we end up with Lazy<Foo, fn() -> T> instead of Lazy<Foo, fn() -> Foo>. So it's an unrelated bug.

matklad · 2020-06-27T09:45:35Z

bors r+

bors · 2020-06-27T10:00:15Z

Build succeeded:

pksunkara · 2020-07-01T08:36:54Z

Hm, long-term, this is not the pattern I'd love to see.... I'd say its pretty crucial to be able to apply improvements immediately, without waiting (as blocking easily accumulates). But that interacts with vendoring....

I wanted to set up a repository action dispatch which when used can create new releases in chalk instead of doing it every Sunday. @jackh726 I think we should do that for chalk.

jackh726 · 2020-07-01T19:44:37Z

@pksunkara that's fine with me if there's a nice way to do that with github

pksunkara · 2020-07-12T09:46:48Z

We can't do that with github itself. We will need a small app like https://www.actionspanel.app or something else. Maybe @pietroalbini can setup a dashboard on the rust-lang.org to send these repository dispatches as part of rust infra.

lnicola · 2020-07-12T09:53:43Z

rust-analyzer releases are done by pushing to a release branch, maybe that could work for chalk too?

pietroalbini · 2020-07-13T10:04:01Z

@pksunkara GitHub Actions recently added a UI for starting workflow_dispatch jobs. Could that work for you?

matklad · 2020-07-13T10:17:34Z

Haven't looked into how chalk is published, but I would probably do the following:

setup auto-publishing from release branch on every push, using GITHUB_RUN_NUMBER as a version number
setup a cron action to push current master to release every something

I think "push the branch" somewhat better trigger than "push the button".

jackh726 · 2020-07-13T22:57:55Z

Actually, the UI is quite nice! You can even set the version to be published. This would be nice to transition to when we don't want weekly auto-releases.

lnicola changed the title ~~Bump chalk~~ WIP: Bump chalk Jun 22, 2020

flodiebold reviewed Jun 22, 2020

View reviewed changes

crates/ra_hir_ty/src/traits/chalk.rs Outdated Show resolved Hide resolved

lnicola force-pushed the bump-chalk branch 3 times, most recently from bac3910 to f05a5ca Compare June 22, 2020 14:19

flodiebold mentioned this pull request Jun 23, 2020

Fix built-in Fn impls when generics are involved rust-lang/chalk#541

Merged

lnicola force-pushed the bump-chalk branch 2 times, most recently from 4284d9f to ac8f86b Compare June 24, 2020 08:05

lnicola force-pushed the bump-chalk branch from ac8f86b to f605f34 Compare June 24, 2020 08:07

Bump chalk

7e104ea

lnicola force-pushed the bump-chalk branch from f605f34 to 7e104ea Compare June 26, 2020 10:03

lnicola changed the title ~~WIP: Bump chalk~~ Bump chalk Jun 27, 2020

bors bot merged commit 18a6dd4 into rust-lang:master Jun 27, 2020

lnicola deleted the bump-chalk branch June 27, 2020 10:08

pksunkara mentioned this pull request Jul 21, 2020

Add workflow_dispatch for manual publish rust-lang/chalk#578

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bump chalk #4982

Bump chalk #4982

lnicola commented Jun 22, 2020 •

edited

Loading

flodiebold commented Jun 22, 2020

lnicola commented Jun 22, 2020

lnicola commented Jun 22, 2020

flodiebold commented Jun 22, 2020

flodiebold commented Jun 22, 2020

lnicola commented Jun 22, 2020

flodiebold commented Jun 22, 2020

flodiebold commented Jun 23, 2020

lnicola commented Jun 24, 2020

matklad commented Jun 24, 2020

lnicola commented Jun 24, 2020 •

edited

Loading

cynecx commented Jun 25, 2020

lnicola commented Jun 25, 2020

flodiebold commented Jun 25, 2020

cynecx commented Jun 25, 2020 •

edited

Loading

flodiebold commented Jun 26, 2020

flodiebold commented Jun 26, 2020 •

edited

Loading

matklad commented Jun 27, 2020

bors bot commented Jun 27, 2020

pksunkara commented Jul 1, 2020

jackh726 commented Jul 1, 2020

pksunkara commented Jul 12, 2020

lnicola commented Jul 12, 2020

pietroalbini commented Jul 13, 2020

matklad commented Jul 13, 2020

jackh726 commented Jul 13, 2020

Bump chalk #4982

Bump chalk #4982

Conversation

lnicola commented Jun 22, 2020 • edited Loading

flodiebold commented Jun 22, 2020

lnicola commented Jun 22, 2020

lnicola commented Jun 22, 2020

flodiebold commented Jun 22, 2020

flodiebold commented Jun 22, 2020

lnicola commented Jun 22, 2020

flodiebold commented Jun 22, 2020

flodiebold commented Jun 23, 2020

lnicola commented Jun 24, 2020

matklad commented Jun 24, 2020

lnicola commented Jun 24, 2020 • edited Loading

cynecx commented Jun 25, 2020

lnicola commented Jun 25, 2020

flodiebold commented Jun 25, 2020

cynecx commented Jun 25, 2020 • edited Loading

flodiebold commented Jun 26, 2020

flodiebold commented Jun 26, 2020 • edited Loading

matklad commented Jun 27, 2020

bors bot commented Jun 27, 2020

pksunkara commented Jul 1, 2020

jackh726 commented Jul 1, 2020

pksunkara commented Jul 12, 2020

lnicola commented Jul 12, 2020

pietroalbini commented Jul 13, 2020

matklad commented Jul 13, 2020

jackh726 commented Jul 13, 2020

lnicola commented Jun 22, 2020 •

edited

Loading

lnicola commented Jun 24, 2020 •

edited

Loading

cynecx commented Jun 25, 2020 •

edited

Loading

flodiebold commented Jun 26, 2020 •

edited

Loading