Simplify ExprUseVisitor #64874

matthewjasper · 2019-09-28T15:22:51Z

Remove HIR const qualification
Remove parts of ExprUseVisitor that aren't being used

eddyb · 2019-09-28T15:28:31Z

I just noticed the old regionck is still around - do we have a timeline for removing it?

src/librustc/middle/mem_categorization.rs

oli-obk · 2019-09-28T15:35:53Z

src/librustc/middle/expr_use_visitor.rs

@@ -2,25 +2,20 @@
 //! normal visitor, which just walks the entire body in one shot, the


can you check whether clippy still works after your PR? We use this visitor a lot.

Oh... anything using EUV should've transitioned a long time ago to being based on MIR, IMO. I wish I brought this up to clippy people earlier :/

src/test/ui/issues/issue-23302-3.stderr

eddyb · 2019-09-28T15:38:20Z

src/librustc_passes/lib.rs

@@ -19,12 +19,10 @@ use rustc::ty::query::Providers;
 pub mod error_codes;

 pub mod ast_validation;
-pub mod rvalue_promotion;
 pub mod hir_stats;
 pub mod layout_test;
 pub mod loops;


Feels like this crate is becoming quite pointless.

We could split them out to a crate for each pass. :)

They are tiny though... a bunch of them belong in rustc::hir which could become a crate (with only the data types remaining in rustc).

Whatever we do, let's please not inflate the size of the rustc crate. :)

eddyb · 2019-09-28T15:39:47Z

src/librustc_typeck/check/upvar.rs

@@ -582,48 +582,13 @@ impl<'a, 'tcx> InferBorrowKind<'a, 'tcx> {
 }

 impl<'a, 'tcx> euv::Delegate<'tcx> for InferBorrowKind<'a, 'tcx> {


Is this the only user left? Can its reliance on MC be simplified too?

matthewjasper · 2019-09-28T16:18:34Z

The old regionck is still used for items. A lot of it can be removed now, but that's another PR.

Notes for anyone trying to update Clippy after this lands:

utils/usage.rs - doesn't use anything removed here
loops.rs - needs to change from the span passed in to the span of the cmt.
escape.rs - consume and consume_pat will need to be merged
needless_pass_by_value - should probably use borrow instead of matched_pat

bors · 2019-09-30T17:28:56Z

☔ The latest upstream changes (presumably #64778) made this pull request unmergeable. Please resolve the merge conflicts.

eddyb

LGTM, r=me if you don't want to change more due to #64874 (comment)

matthewjasper · 2019-10-03T11:58:44Z

@bors r=eddyb

bors · 2019-10-03T11:58:46Z

📌 Commit b4ad612 has been approved by eddyb

nikomatsakis

Nice cleanup, @matthewjasper!

@eddyb

Simplify ExprUseVisitor * Remove HIR const qualification * Remove parts of ExprUseVisitor that aren't being used r? @eddyb

@ghost

Rollup of 5 pull requests Successful merges: - #64749 (Fix most remaining Polonius test differences) - #64817 (Replace ClosureSubsts with SubstsRef) - #64874 (Simplify ExprUseVisitor) - #65026 (metadata: Some crate loading cleanup) - #65073 (Remove `borrowck_graphviz_postflow` from test) Failed merges: r? @ghost

Episode 1 - The simple cases

@matthewjasper

Rustup to rust-lang/rust#64874 TODO: - [x] replace `rvalue_promotable_map` in [1] - [ ] ~~fix [2] according to this comment rust-lang/rust#64874 (comment) this should be merged with `consume`, but I didn't figure out how to merge them, yet.~~ - [ ] ~~fix [3]; What to do with `LoanCause`?~~ [2]+[3] probably have to be resolved by a rewrite of the lint. #4628 (comment) [1] https://github.com/rust-lang/rust-clippy/blob/54bf4ffd626970e831bb80c037f804a3b3450835/clippy_lints/src/methods/mod.rs#L1292-L1299 [2] https://github.com/rust-lang/rust-clippy/blob/54bf4ffd626970e831bb80c037f804a3b3450835/clippy_lints/src/escape.rs#L126 [3] https://github.com/rust-lang/rust-clippy/blob/54bf4ffd626970e831bb80c037f804a3b3450835/clippy_lints/src/escape.rs#L166-L176 I could need some help with [1]. The purpose of this is to "don't lint for constant values". cc @matthewjasper For now I see what I can do with [2]. changelog: Temporary break `boxed_local` lint.

rust-highfive assigned eddyb Sep 28, 2019

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 28, 2019

eddyb reviewed Sep 28, 2019

View reviewed changes

src/librustc/middle/mem_categorization.rs Outdated Show resolved Hide resolved

oli-obk reviewed Sep 28, 2019

View reviewed changes

eddyb reviewed Sep 28, 2019

View reviewed changes

src/test/ui/issues/issue-23302-3.stderr Outdated Show resolved Hide resolved

eddyb reviewed Sep 28, 2019

View reviewed changes

matthewjasper force-pushed the simplify-euv branch from e18bf4b to c83e8a6 Compare September 28, 2019 21:27

matthewjasper added 2 commits September 30, 2019 19:56

Remove HIR based const qualification

455945f

Remove unused parts of ExprUseVisitor

b4ad612

matthewjasper force-pushed the simplify-euv branch from c83e8a6 to b4ad612 Compare September 30, 2019 18:59

eddyb approved these changes Oct 3, 2019

View reviewed changes

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 3, 2019

nikomatsakis approved these changes Oct 3, 2019

View reviewed changes

Centril added a commit to Centril/rust that referenced this pull request Oct 4, 2019

Rollup merge of rust-lang#64874 - matthewjasper:simplify-euv, r=eddyb

cb4145e

Simplify ExprUseVisitor * Remove HIR const qualification * Remove parts of ExprUseVisitor that aren't being used r? @eddyb

Centril mentioned this pull request Oct 4, 2019

Rollup of 5 pull requests #65087

Merged

bors merged commit b4ad612 into rust-lang:master Oct 4, 2019

JohnTitor mentioned this pull request Oct 4, 2019

clippy-driver no longer builds after rust-lang/rust#65087 #65096

Closed

matthewjasper deleted the simplify-euv branch October 4, 2019 12:03

flip1995 added a commit to flip1995/rust-clippy that referenced this pull request Oct 4, 2019

Rustup to rust-lang/rust#64874

f3cdee4

Episode 1 - The simple cases

flip1995 added a commit to flip1995/rust-clippy that referenced this pull request Oct 4, 2019

Rustup to rust-lang/rust#64874

b46f5b4

Episode 1 - The simple cases

flip1995 mentioned this pull request Oct 4, 2019

Rustup to rust-lang/rust#64874 rust-lang/rust-clippy#4628

Merged

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Simplify ExprUseVisitor #64874

Simplify ExprUseVisitor #64874

matthewjasper commented Sep 28, 2019

eddyb commented Sep 28, 2019

oli-obk Sep 28, 2019

eddyb Sep 28, 2019

eddyb Sep 28, 2019

Centril Sep 28, 2019

eddyb Sep 29, 2019

Centril Sep 29, 2019

eddyb Sep 28, 2019 •

edited

Loading

matthewjasper commented Sep 28, 2019

bors commented Sep 30, 2019

eddyb left a comment

matthewjasper commented Oct 3, 2019

bors commented Oct 3, 2019

nikomatsakis left a comment

		@@ -2,25 +2,20 @@
		//! normal visitor, which just walks the entire body in one shot, the

		@@ -582,48 +582,13 @@ impl<'a, 'tcx> InferBorrowKind<'a, 'tcx> {
		}

		impl<'a, 'tcx> euv::Delegate<'tcx> for InferBorrowKind<'a, 'tcx> {

Simplify ExprUseVisitor #64874

Simplify ExprUseVisitor #64874

Conversation

matthewjasper commented Sep 28, 2019

eddyb commented Sep 28, 2019

oli-obk Sep 28, 2019

Choose a reason for hiding this comment

eddyb Sep 28, 2019

Choose a reason for hiding this comment

eddyb Sep 28, 2019

Choose a reason for hiding this comment

Centril Sep 28, 2019

Choose a reason for hiding this comment

eddyb Sep 29, 2019

Choose a reason for hiding this comment

Centril Sep 29, 2019

Choose a reason for hiding this comment

eddyb Sep 28, 2019 • edited Loading

Choose a reason for hiding this comment

matthewjasper commented Sep 28, 2019

bors commented Sep 30, 2019

eddyb left a comment

Choose a reason for hiding this comment

matthewjasper commented Oct 3, 2019

bors commented Oct 3, 2019

nikomatsakis left a comment

Choose a reason for hiding this comment

eddyb Sep 28, 2019 •

edited

Loading