Use hir::Place instead of Symbol in closure_kind_origin #36

roxelo · 2020-12-03T03:44:17Z

Modifies closure_kind_origins such that Place is used instead of Symbol to describe which variable influenced the decision of selecting a specific closure kind (FnMut, FnOnce, Fn)

Remainging work: Figure out why for 4 of the tests, some stderr info are missing.

Closes rust-lang/project-rfc-2229/issues/21

This change is

roxelo · 2020-12-03T03:45:51Z

src/test/ui/associated-type-bounds/bounds-on-assoc-in-trait.stderr

-LL | pub trait Debug {
-   | --------------- required by this bound in `Debug`
+LL |       type A: Iterator<Item: Debug>;
+   |                              ^^^^^ `<<Self as Case1>::A as Iterator>::Item` cannot be formatted using `{:?}` because it doesn't implement `Debug`


Still investigating why some of the information is missing from stderr

compiler/rustc_mir/src/borrow_check/diagnostics/mod.rs

compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs

compiler/rustc_typeck/src/check/upvar.rs

compiler/rustc_middle/src/hir/place.rs

compiler/rustc_middle/src/ty/context.rs

compiler/rustc_middle/src/ty/mod.rs

arora-aman · 2020-12-14T02:15:30Z

src/test/ui/pattern/move-ref-patterns/move-ref-patterns-closure-captures.stderr

@@ -29,7 +29,7 @@ LL |     let c2 = || {
   |              ^^ this closure implements `FnMut`, not `Fn`
 ...
 LL |         drop::<&mut U>(_x2);
-   |                        --- closure is `FnMut` because it mutates the variable `_x2` here
+   |                        --- closure is `FnMut` because it mutates the variable `*_x2` here


Interesting, I would have not expected this to capture *_x2 :o

compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs

compiler/rustc_typeck/src/check/writeback.rs

src/test/ui/closure-struct-diagnostics.rs

arora-aman

One nit

compiler/rustc_typeck/src/check/upvar.rs

compiler/rustc_typeck/src/check/writeback.rs

compiler/rustc_middle/src/ty/mod.rs

arora-aman

Sorry I missed this

compiler/rustc_typeck/src/check/writeback.rs

roxelo · 2020-12-28T04:26:02Z

I added more tests and found another bug (it's actually a known bug, so I'll just modify it so it doesn't error out)

---- [ui] ui/closures/2229_closure_analysis/diagnostics/closure-single-variant-diagnostics.rs stdout ----

error: Error: expected failure status (Some(1)) but received status Some(101).
status: exit code: 101
command: "/users/rlmmfruy/rust/build/x86_64-unknown-linux-gnu/stage1/bin/rustc" "/users/rlmmfruy/rust/src/test/ui/closures/2229_closure_analysis/diagnostics/closure-single-variant-diagnostics.rs" "-Zthreads=1" "--target=x86_64-unknown-linux-gnu" "--error-format" "json" "-Zui-testing" "-Zdeduplicate-diagnostics=no" "-Zemit-future-incompat-report" "--emit" "metadata" "-C" "prefer-dynamic" "--out-dir" "/users/rlmmfruy/rust/build/x86_64-unknown-linux-gnu/test/ui/closures/2229_closure_analysis/diagnostics/closure-single-variant-diagnostics" "-A" "unused" "-Crpath" "-O" "-Cdebuginfo=0" "-Zunstable-options" "-Lnative=/users/rlmmfruy/rust/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "-L" "/users/rlmmfruy/rust/build/x86_64-unknown-linux-gnu/test/ui/closures/2229_closure_analysis/diagnostics/closure-single-variant-diagnostics/auxiliary"
stdout:
------------------------------------------

------------------------------------------
stderr:
------------------------------------------
warning: the feature `capture_disjoint_fields` is incomplete and may not be safe to use and/or cause compiler crashes
  --> /users/rlmmfruy/rust/src/test/ui/closures/2229_closure_analysis/diagnostics/closure-single-variant-diagnostics.rs:1:12
   |
LL | #![feature(capture_disjoint_fields)]
   |            ^^^^^^^^^^^^^^^^^^^^^^^
   |
   = note: `#[warn(incomplete_features)]` on by default
   = note: see issue #53488 <https://github.com/rust-lang/rust/issues/53488> for more information

thread 'rustc' panicked at 'called `Result::unwrap()` on an `Err` value: HirId { owner: DefId(0:8 ~ closure_single_variant_diagnostics[317d]::main), local_id: 9 }', compiler/rustc_mir_build/src/build/expr/as_place.rs:299:69
stack backtrace:
   0: rust_begin_unwind
   1: core::panicking::panic_fmt
   2: core::result::unwrap_failed
   3: rustc_mir_build::build::expr::as_place::PlaceBuilder::expect_upvars_resolved
   4: rustc_mir_build::build::expr::as_place::PlaceBuilder::into_place
   5: rustc_mir_build::build::expr::as_place::<impl rustc_mir_build::build::Builder>::as_place
   6: rustc_mir_build::build::matches::<impl rustc_mir_build::build::Builder>::expr_into_pattern
   7: rustc_mir_build::build::scope::<impl rustc_mir_build::build::Builder>::in_opt_scope
   8: rustc_mir_build::build::block::<impl rustc_mir_build::build::Builder>::ast_block_stmts
   9: rustc_mir_build::build::scope::<impl rustc_mir_build::build::Builder>::in_opt_scope
  10: rustc_mir_build::build::expr::into::<impl rustc_mir_build::build::Builder>::into_expr
  11: <rustc_mir_build::thir::ExprRef as rustc_mir_build::build::into::EvalInto>::eval_into
  12: rustc_mir_build::build::scope::<impl rustc_mir_build::build::Builder>::in_scope
  13: rustc_data_structures::stack::ensure_sufficient_stack
  14: rustc_mir_build::build::expr::into::<impl rustc_mir_build::build::Builder>::into_expr
  15: <rustc_mir_build::thir::ExprRef as rustc_mir_build::build::into::EvalInto>::eval_into
  16: rustc_mir_build::build::scope::<impl rustc_mir_build::build::Builder>::in_scope
  17: rustc_data_structures::stack::ensure_sufficient_stack
  18: rustc_mir_build::build::expr::into::<impl rustc_mir_build::build::Builder>::into_expr
  19: <rustc_mir_build::thir::Expr as rustc_mir_build::build::into::EvalInto>::eval_into
  20: rustc_mir_build::build::Builder::args_and_body
  21: rustc_mir_build::build::construct_fn
  22: rustc_infer::infer::InferCtxtBuilder::enter
  23: rustc_mir_build::build::mir_built
  24: rustc_middle::ty::query::<impl rustc_query_system::query::config::QueryAccessors<rustc_middle::ty::context::TyCtxt> for rustc_middle::ty::query::queries::mir_built>::compute
  25: rustc_query_system::dep_graph::graph::DepGraph<K>::with_task_impl
  26: rustc_query_system::dep_graph::graph::DepGraph<K>::with_task
  27: rustc_data_structures::stack::ensure_sufficient_stack
  28: rustc_query_system::query::plumbing::get_query_impl
  29: rustc_mir::transform::check_unsafety::unsafety_check_result
  30: core::ops::function::FnOnce::call_once
  31: rustc_middle::ty::query::<impl rustc_query_system::query::config::QueryAccessors<rustc_middle::ty::context::TyCtxt> for rustc_middle::ty::query::queries::unsafety_check_result>::compute
  32: rustc_query_system::dep_graph::graph::DepGraph<K>::with_task_impl
  33: rustc_data_structures::stack::ensure_sufficient_stack
  34: rustc_query_system::query::plumbing::get_query_impl
  35: <rustc_mir::transform::check_unsafety::UnsafetyChecker as rustc_middle::mir::visit::Visitor>::visit_rvalue
  36: <rustc_mir::transform::check_unsafety::UnsafetyChecker as rustc_middle::mir::visit::Visitor>::visit_statement
  37: rustc_middle::mir::visit::Visitor::visit_body
  38: rustc_mir::transform::check_unsafety::unsafety_check_result
  39: core::ops::function::FnOnce::call_once
  40: rustc_middle::ty::query::<impl rustc_query_system::query::config::QueryAccessors<rustc_middle::ty::context::TyCtxt> for rustc_middle::ty::query::queries::unsafety_check_result>::compute
  41: rustc_query_system::dep_graph::graph::DepGraph<K>::with_task_impl
  42: rustc_data_structures::stack::ensure_sufficient_stack
  43: rustc_query_system::query::plumbing::get_query_impl
  44: rustc_query_system::query::plumbing::ensure_query_impl
  45: rustc_mir::transform::mir_const
  46: rustc_middle::ty::query::<impl rustc_query_system::query::config::QueryAccessors<rustc_middle::ty::context::TyCtxt> for rustc_middle::ty::query::queries::mir_const>::compute
  47: rustc_query_system::dep_graph::graph::DepGraph<K>::with_task_impl
  48: rustc_query_system::dep_graph::graph::DepGraph<K>::with_task
  49: rustc_data_structures::stack::ensure_sufficient_stack
  50: rustc_query_system::query::plumbing::get_query_impl
  51: rustc_mir::transform::mir_promoted
  52: rustc_middle::ty::query::<impl rustc_query_system::query::config::QueryAccessors<rustc_middle::ty::context::TyCtxt> for rustc_middle::ty::query::queries::mir_promoted>::compute
  53: rustc_query_system::dep_graph::graph::DepGraph<K>::with_task_impl
  54: rustc_query_system::dep_graph::graph::DepGraph<K>::with_task
  55: rustc_data_structures::stack::ensure_sufficient_stack
  56: rustc_query_system::query::plumbing::get_query_impl
  57: rustc_mir::borrow_check::mir_borrowck
  58: core::ops::function::FnOnce::call_once
  59: rustc_middle::ty::query::<impl rustc_query_system::query::config::QueryAccessors<rustc_middle::ty::context::TyCtxt> for rustc_middle::ty::query::queries::mir_borrowck>::compute
  60: rustc_query_system::dep_graph::graph::DepGraph<K>::with_task_impl
  61: rustc_data_structures::stack::ensure_sufficient_stack
  62: rustc_query_system::query::plumbing::get_query_impl
  63: rustc_query_system::query::plumbing::ensure_query_impl
  64: rustc_middle::ty::<impl rustc_middle::ty::context::TyCtxt>::par_body_owners
  65: rustc_session::utils::<impl rustc_session::session::Session>::time
  66: rustc_interface::passes::analysis
  67: rustc_middle::ty::query::<impl rustc_query_system::query::config::QueryAccessors<rustc_middle::ty::context::TyCtxt> for rustc_middle::ty::query::queries::analysis>::compute
  68: rustc_query_system::dep_graph::graph::DepGraph<K>::with_task_impl
  69: rustc_query_system::dep_graph::graph::DepGraph<K>::with_eval_always_task
  70: rustc_data_structures::stack::ensure_sufficient_stack
  71: rustc_query_system::query::plumbing::get_query_impl
  72: rustc_interface::passes::QueryContext::enter
  73: rustc_interface::queries::<impl rustc_interface::interface::Compiler>::enter
  74: rustc_span::with_source_map
  75: rustc_interface::interface::create_compiler_and_run
  76: scoped_tls::ScopedKey<T>::set
  77: rustc_span::with_session_globals
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.51.0-dev running on x86_64-unknown-linux-gnu

note: compiler flags: -Z threads=1 -Z ui-testing -Z deduplicate-diagnostics=no -Z emit-future-incompat-report -Z unstable-options -C prefer-dynamic -C rpath -C debuginfo=0

query stack during panic:
#0 [mir_built] building MIR for `main::{closure#0}`
#1 [unsafety_check_result] unsafety-checking `main::{closure#0}`
#2 [unsafety_check_result] unsafety-checking `main`
#3 [mir_const] processing MIR for `main`
#4 [mir_promoted] processing `main`
#5 [mir_borrowck] borrow-checking `main`
#6 [analysis] running analysis passes on this crate
end of query stack
warning: 1 warning emitted

Will investigate tomorrow

arora-aman

Can you move the tests to
test/ui/closures/2229_closure_analysis/diagnostics/

And add origin to the naming scheme like closure-origin-<>

src/test/ui/closures/2229_closure_analysis/diagnostics/closure-single-variant-diagnostics.rs

compiler/rustc_middle/src/ty/mod.rs

compiler/rustc_typeck/src/check/upvar.rs

arora-aman · 2020-12-29T02:09:56Z

src/test/ui/closure-tuple-diagnostics.rs

+//~^ WARNING: the feature `capture_disjoint_fields` is incomplete
+//~| `#[warn(incomplete_features)]` on by default
+//~| see issue #53488 <https://github.com/rust-lang/rust/issues/53488>
+struct S(String);


can you change this have multiple elements, non copy elements?

arora-aman · 2020-12-29T02:10:03Z

src/test/ui/closure-tuple-diagnostics.rs

+fn main() {
+    let s = S(format!("s"));
+    let c = || { //~ ERROR expected a closure
+        let s = s.0;


Then use s.1

I have tried it, but it doesn't seem like a valid thing to do

compiler/rustc_middle/src/ty/mod.rs

Didn't mean to approve yet

compiler/rustc_middle/src/ty/mod.rs

arora-aman · 2020-12-30T22:27:25Z

src/test/ui/closures/2229_closure_analysis/diagnostics/closure-origin-tuple-diagnostics.rs

+
+fn main() {
+    let s = S(format!("s"));
+    let c = || { //~ ERROR expected a closure


can u turn the error into expected closure that implements Fn

Also can you modify the test like this and use s.1: https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=5c03fa69ecf41fc95e30bf12c937a848

ohh, I see what you want now

arora-aman

one nit. LGTM otherwise

compiler/rustc_middle/src/ty/mod.rs

roxelo commented Dec 3, 2020

View reviewed changes

arora-aman requested changes Dec 3, 2020

View reviewed changes

roxelo force-pushed the use-place-instead-of-symbol branch from cc6116c to b6298f5 Compare December 13, 2020 23:44

arora-aman reviewed Dec 14, 2020

View reviewed changes

compiler/rustc_middle/src/hir/place.rs Outdated Show resolved Hide resolved

compiler/rustc_middle/src/hir/place.rs Outdated Show resolved Hide resolved

arora-aman reviewed Dec 14, 2020

View reviewed changes

src/test/ui/closure-struct-diagnostics.rs Outdated Show resolved Hide resolved

arora-aman self-assigned this Dec 14, 2020

arora-aman reviewed Dec 19, 2020

View reviewed changes

compiler/rustc_typeck/src/check/upvar.rs Show resolved Hide resolved

arora-aman reviewed Dec 27, 2020

View reviewed changes

compiler/rustc_typeck/src/check/writeback.rs Show resolved Hide resolved

roxelo force-pushed the use-place-instead-of-symbol branch from 471af5f to 3fa405c Compare December 27, 2020 01:39

arora-aman reviewed Dec 27, 2020

View reviewed changes

compiler/rustc_middle/src/ty/mod.rs Outdated Show resolved Hide resolved

arora-aman reviewed Dec 27, 2020

View reviewed changes

compiler/rustc_typeck/src/check/writeback.rs Outdated Show resolved Hide resolved

roxelo force-pushed the use-place-instead-of-symbol branch from 11c2bb7 to 1cb3878 Compare December 28, 2020 22:01

arora-aman requested changes Dec 29, 2020

View reviewed changes

arora-aman previously approved these changes Dec 29, 2020

View reviewed changes

compiler/rustc_middle/src/ty/mod.rs Outdated Show resolved Hide resolved

arora-aman requested changes Dec 30, 2020

View reviewed changes

roxelo force-pushed the use-place-instead-of-symbol branch from 784681e to d73cf5f Compare January 2, 2021 19:33

arora-aman approved these changes Jan 2, 2021

View reviewed changes

compiler/rustc_middle/src/ty/mod.rs Outdated Show resolved Hide resolved

use hir::Place instead of Symbol in closure_kind_origin

b498870

roxelo force-pushed the use-place-instead-of-symbol branch from d73cf5f to b498870 Compare January 2, 2021 22:50

sexxi-bot merged commit 19f9780 into master Jan 17, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use hir::Place instead of Symbol in closure_kind_origin #36

Use hir::Place instead of Symbol in closure_kind_origin #36

roxelo commented Dec 3, 2020 •

edited by Azhng

Loading

roxelo Dec 3, 2020 •

edited

Loading

arora-aman Dec 14, 2020

arora-aman left a comment

arora-aman left a comment

roxelo commented Dec 28, 2020 •

edited

Loading

arora-aman left a comment

arora-aman Dec 29, 2020

arora-aman Dec 29, 2020

roxelo Dec 30, 2020

arora-aman Dec 30, 2020

arora-aman Dec 30, 2020

roxelo Jan 2, 2021

arora-aman left a comment

Use hir::Place instead of Symbol in closure_kind_origin #36

Use hir::Place instead of Symbol in closure_kind_origin #36

Conversation

roxelo commented Dec 3, 2020 • edited by Azhng Loading

roxelo Dec 3, 2020 • edited Loading

Choose a reason for hiding this comment

arora-aman Dec 14, 2020

Choose a reason for hiding this comment

arora-aman left a comment

Choose a reason for hiding this comment

arora-aman left a comment

Choose a reason for hiding this comment

roxelo commented Dec 28, 2020 • edited Loading

arora-aman left a comment

Choose a reason for hiding this comment

arora-aman Dec 29, 2020

Choose a reason for hiding this comment

arora-aman Dec 29, 2020

Choose a reason for hiding this comment

roxelo Dec 30, 2020

Choose a reason for hiding this comment

arora-aman Dec 30, 2020

Choose a reason for hiding this comment

arora-aman Dec 30, 2020

Choose a reason for hiding this comment

roxelo Jan 2, 2021

Choose a reason for hiding this comment

arora-aman left a comment

Choose a reason for hiding this comment

roxelo commented Dec 3, 2020 •

edited by Azhng

Loading

roxelo Dec 3, 2020 •

edited

Loading

roxelo commented Dec 28, 2020 •

edited

Loading