Commit Graph

15517 Commits

Author SHA1 Message Date
bors[bot]
5009958847
Merge #7568
7568: Fix merging of `segment_index` in path resolution r=jonas-schievink a=jonas-schievink

This caused associated item lookup to fail when modifying `resolver.rs` to handle block expressions with inner items.

bors r+

Co-authored-by: Jonas Schievink <jonasschievink@gmail.com>
2021-02-05 14:16:12 +00:00
Jonas Schievink
6239fe4730 Fix merging of segment_index in path resolution 2021-02-05 15:14:33 +01:00
bors[bot]
c72b0c3719
Merge #7567
7567: Remove unnecessary allocs in case_conv r=Veykril a=Veykril

and some replace unwraps

bors r+

Co-authored-by: Lukas Wirth <lukastw97@gmail.com>
2021-02-05 14:03:52 +00:00
Lukas Wirth
486c7175fe Remove unnecessary allocs in case_conv 2021-02-05 15:02:39 +01:00
Yoshua Wuyts
dfd751303e Move find_struct_impl to assist utils 2021-02-05 14:36:07 +01:00
Luciano Bestia
084b21bc36 simple comparison instead of regex 2021-02-05 14:32:03 +01:00
Yoshua Wuyts
13d663dd16 add generate-enum-match assist 2021-02-05 11:28:11 +01:00
bors[bot]
ac5958485e
Merge #7535
7535: Extract function assist r=cpud36 a=cpud36

This PR adds `extract function/method` assist. closes #5409.

# Supported features
Assist should support extracting from expressions(`1`, `2 + 2`, `loop { }`) and from a series of statements, e.g.:
```rust
foo();
$0bar();
baz();$0
quix();
```
Assist also supports extracting parameters, like:
```rust
fn foo() -> i32 {
  let n = 1;
  $0n + 1$0
}

// -
fn foo() -> i32 {
  let n = 1;
  fun_name(n)
}

fn fun_name(n: i32) -> i32 {
  n + 1
}
```

Extracting methods also generally works.

Assist allows referencing outer variables, both mutably and immutably, and handles handles access to variables local to extracted function:
```rust
fn foo() {
  let mut n = 1;
  let mut m = 2;
  let mut moved_v = Vec::new();
  let mut ref_mut_v = Vec::new();
  $0
  n += 1;
  let k = 1;
  moved_v.push(n);
  let r = &mut m;
  ref_mut_v.push(*r);
  let h = 3;
  $0
  n = ref_mut_v.len() + k;
  n -= h + m;
}

// -
fn foo() {
  let mut n = 1;
  let mut m = 2;
  let mut moved_v = Vec::new();
  let mut ref_mut_v = Vec::new();
 
  let (k, h) =  fun_name(&mut n, moved_v, &mut m, &mut ref_mut_v);

  n = ref_mut_v.len() + k;
  n -= h + m;
}

fn fun_name(n: &mut i32, mut moved_v: Vec<i32>, m: &mut i32, ref_mut_v: &mut Vec<i32>) -> (i32, i32) {
  *n += 1;
  let k = 1;
  moved_v.push(*n);
  let r = m;
  ref_mut_v.push(*r);
  let h = 3;
  (k, h)
}
```

So we handle both input and output paramters

# Showcase

![extract_cursor_in_range_3](https://user-images.githubusercontent.com/4218373/106980190-c9870800-6770-11eb-83d9-3d36b2550ff6.gif)
![fill_match_arms_discard_wildcard](https://user-images.githubusercontent.com/4218373/106980197-cbe96200-6770-11eb-96b0-14c27894fac0.gif)
![ide_db_helpers_handle_kind](https://user-images.githubusercontent.com/4218373/106980201-cdb32580-6770-11eb-9e6e-6ac8155d65ac.gif)
![ide_db_imports_location_local_query](https://user-images.githubusercontent.com/4218373/106980205-cf7ce900-6770-11eb-8516-653c8fcca807.gif)

# Working with non-`Copy` types

Consider the following example:
```rust
fn foo() {
  let v = Vec::new();
  $0
  let n = v.len();
  $0
  let is_empty = v.is_empty();
}
```
`v` must be a parameter to extracted function. 
The question is, what type should it have.
It could be `v: Vec<i32>`, or `v: &Vec<i32>`. 
The former is incorrect for `Vec<i32>`, but the later is silly for `i32`.

To resolve this we need to know if the type implements `Copy` trait.

I didn't find any api available from assists to query this. 
`hir_ty::method_resolution::implements` seems relevant, but is isn't publicly re-exported from `hir`.

# Star(`*`) token and pointer dereference

If I understand correctly, in order to create expression like `*p`, one should use `ast::make::expr_prefix(T![*], ...)`, which
in turn calls `token(T![*])`.

`token` does not have star in `tokens::SOURCE_FILE`, so this panics.
I had to add `*` to `SOURCE_FILE` to make it work.

Correct me if this is not intended way to do this.

# Lowering access `value -> mut ref -> shared ref`

Consider the following example:
```rust
fn foo() {
  let v = Vec::new();
  $0 let n = v.len(); $0
}
```
`v` is not used after extracted function body, so both `v: &Vec<i32>` and `v: Vec<i32>` would work.
Currently the later would be chosen.

We can however check the body of extracted function and conclude that `v: &Vec<i32>` is sufficient.
Using `v: &Vec<i32>`(that is a minimal required access level) might be a better default.
I am unsure.

# Cleanup
The assist seems to be reasonably handling most of common cases.
If there are no concerns with code it produces(i.e. with test cases), I will start cleaning up

[edit]
added showcase


Co-authored-by: Vladyslav Katasonov <cpud47@gmail.com>
2021-02-05 02:55:56 +00:00
Vladyslav Katasonov
7eaa3e56a0 allow extracted body to be indented(dedent it) 2021-02-05 05:44:08 +03:00
Vladyslav Katasonov
876ca60316 allow transitive &mut access for fields in extract_function 2021-02-05 04:35:41 +03:00
Vladyslav Katasonov
271c1cb013 add tests for extracting if/match/while/for exprs 2021-02-05 02:30:34 +03:00
Vladyslav Katasonov
4dc2a42500 document extract_function assist implementation 2021-02-05 02:14:32 +03:00
Vladyslav Katasonov
0ff74467c0 use &T for non copy params of extracted function
Use shared ref if param is not `T: Copy` and is used after body
2021-02-05 01:41:40 +03:00
bors[bot]
842033b150
Merge #7561
7561: Avoid using ModPath's fields directly r=jonas-schievink a=jonas-schievink

bors r+

Co-authored-by: Jonas Schievink <jonasschievink@gmail.com>
2021-02-04 21:42:57 +00:00
Jonas Schievink
474df093a9 Avoid using ModPath's fields directly 2021-02-04 22:42:21 +01:00
Vladyslav Katasonov
d9b122858b split extract_function into pieces and order them 2021-02-05 00:35:28 +03:00
bors[bot]
de046bf457
Merge #7559
7559: Make `ModPath`'s representation private r=jonas-schievink a=jonas-schievink

This lets us switch out the `Vec` for something more efficient

bors r+

Co-authored-by: Jonas Schievink <jonasschievink@gmail.com>
2021-02-04 19:59:21 +00:00
Jonas Schievink
5d99ba1d9a Make ModPath's representation private 2021-02-04 20:49:24 +01:00
bors[bot]
4c1fcda0f8
Merge #7558
7558: Update thread_local r=kjeremy a=kjeremy

Pulls in https://github.com/Amanieu/thread_local-rs/pull/30 which fixes
a leak when dropping ThreadLocal.

Co-authored-by: kjeremy <kjeremy@gmail.com>
2021-02-04 19:46:01 +00:00
kjeremy
4a722a34f9 Update thread_local
Pulls in https://github.com/Amanieu/thread_local-rs/pull/30 which fixes
a leak when dropping ThreadLocal.
2021-02-04 14:42:45 -05:00
bors[bot]
36191543a6
Merge #7557
7557: Intern `TypeRef`s in the containing `ItemTree` r=jonas-schievink a=jonas-schievink

This reduces the memory used by `ItemTreeQuery` by ~20%. As it turns out, `TypeRef` is very heavy, and is used a lot in `ItemTree`:

* Many types are frequently repeated throughout the same file. This is what this PR addresses by storing identical `TypeRef`s only once per `ItemTree`.
* The vast majority of `TypeRef` consist of a plain path with only a single element. "Type anchors" like in `<Ty>::func` are used incredibly rarely, and even multi-segment paths are much rarer than single-segment paths.

Co-authored-by: Jonas Schievink <jonasschievink@gmail.com>
2021-02-04 18:33:57 +00:00
Jonas Schievink
003ee0086a Intern TypeRefs in the containing ItemTree 2021-02-04 19:23:56 +01:00
bors[bot]
663d404a4e
Merge #7555
7555: Expander: store a LocalModuleId, not ModuleId r=jonas-schievink a=jonas-schievink

It already stores the DefMap containing the module, so having
a full ModuleId is unnecessary and makes it easier to mix things up

bors r+

Co-authored-by: Jonas Schievink <jonasschievink@gmail.com>
2021-02-04 14:05:25 +00:00
Jonas Schievink
cacaebcb33 Expander: store a LocalModuleId, not ModuleId
It already stores the DefMap containing the module, so having
a full ModuleId is unnecessary and makes it easier to mix things up
2021-02-04 15:04:21 +01:00
bors[bot]
1bae5509ad
Merge #7554
7554: Don't keep the parent DefMap alive via Arc r=jonas-schievink a=jonas-schievink

This seems like it could easily leak a lot of memory since we don't
currently run GC

bors r+

Co-authored-by: Jonas Schievink <jonasschievink@gmail.com>
2021-02-04 12:45:40 +00:00
Jonas Schievink
26a2a2433c Don't keep the parent DefMap alive via Arc
This seems like it could easily leak a lot of memory since we don't
currently run GC
2021-02-04 13:44:54 +01:00
Lukas Wirth
34d5d77cfa Increase Highlights highlight range to covering element 2021-02-04 11:37:14 +01:00
bors[bot]
01bc1fdff8
Merge #7553
7553: More architecture.md r=matklad a=matklad

bors r+
🤖

Co-authored-by: Aleksey Kladov <aleksey.kladov@gmail.com>
2021-02-04 10:28:13 +00:00
Aleksey Kladov
06aa34cd10 More architecture.md 2021-02-04 13:27:34 +03:00
Vladyslav Katasonov
ff77c5e68f remove ignored test for downgrading mut to shared 2021-02-04 00:52:53 +03:00
Vladyslav Katasonov
c4f3669e70 allow calling &mut methods on outer vars when extracing function 2021-02-04 00:44:36 +03:00
Vladyslav Katasonov
86ff1d4809 allow &mut param when extracting function
Recognise &mut as variable modification.
This allows extracting functions with
`&mut var` with `var` being in outer scope
2021-02-04 00:27:31 +03:00
Vladyslav Katasonov
f102616aae allow modifications of vars from outer scope inside extracted function
It currently allows only directly setting variable.
No `&mut` references or methods.
2021-02-03 23:45:03 +03:00
Vladyslav Katasonov
82787febde allow local variables to be used after extracted body
when variable is defined inside extracted body
export this variable to original scope via return value(s)
2021-02-03 21:11:12 +03:00
bors[bot]
74a223faa3
Merge #7547
7547: Split out ItemScope::dump from DefMap::dump r=jonas-schievink a=jonas-schievink

This is helpful for more targeted debugging

bors r+

Co-authored-by: Jonas Schievink <jonasschievink@gmail.com>
2021-02-03 18:06:55 +00:00
Jonas Schievink
1a8ea81a79 Split out ItemScope::dump from DefMap::dump 2021-02-03 19:05:11 +01:00
bors[bot]
3ff2aa6d09
Merge #7546
7546: Add newline between block and crate maps r=jonas-schievink a=jonas-schievink

bors r+

Co-authored-by: Jonas Schievink <jonasschievink@gmail.com>
2021-02-03 17:25:06 +00:00
Jonas Schievink
6458f9107c Add newline between block and crate maps 2021-02-03 18:23:59 +01:00
bors[bot]
a6ccd50ece
Merge #7545
7545: Add a FIXME to ItemTree r=jonas-schievink a=jonas-schievink

bors r+

Co-authored-by: Jonas Schievink <jonasschievink@gmail.com>
2021-02-03 17:15:21 +00:00
Jonas Schievink
eda1cb7ceb Add a FIXME to ItemTree 2021-02-03 18:14:39 +01:00
bors[bot]
6817f1ff99
Merge #7544
7544: Update `DefMap` and `block_def_map` docs r=jonas-schievink a=jonas-schievink

bors r+

Co-authored-by: Jonas Schievink <jonasschievink@gmail.com>
2021-02-03 16:54:29 +00:00
Jonas Schievink
d4a22fc801 Update DefMap and block_def_map docs 2021-02-03 17:54:03 +01:00
bors[bot]
c92be90fd2
Merge #7543
7543: Grammar fixes r=Kushagra-0801 a=Kushagra-0801

I think line 235 is still wrong, but I am not sure.

Is the `crated/tt` in line 252 supposed to be `crates/tt`?

Co-authored-by: Kushagra Gupta <39802979+Kushagra-0801@users.noreply.github.com>
2021-02-03 16:20:25 +00:00
Kushagra Gupta
b75e85998d
typo fixes 2021-02-03 21:30:42 +05:30
Kushagra Gupta
ed35e516b1
Grammar fixes
I think line 235 is still wrong, but I am not sure.

Is the `crated/tt` in line 252 supposed to be `crates/tt`?
2021-02-03 21:05:21 +05:30
Vladyslav Katasonov
313aa5f3a2 change TODO to FIXME 2021-02-03 17:47:21 +03:00
Vladyslav Katasonov
f0d2bb9131 disable test for downgrading mutability on extract 2021-02-03 17:46:57 +03:00
Vladyslav Katasonov
88b3034636 convert IdentPat to Pat via Into
before child getter was used
2021-02-03 17:45:36 +03:00
bors[bot]
fd84df9e1b
Merge #7541
7541: Use block_def_map in body lowering (third time's the charm) r=jonas-schievink a=jonas-schievink

After https://github.com/rust-analyzer/rust-analyzer/pull/7380 and https://github.com/rust-analyzer/rust-analyzer/pull/7506 both had to be reverted, this should have finally resolved all remaining bugs.

Most importantly, the optimization to skip `block_def_map` computation when the block contains no inner items was fixed (which fortunately was simpler than expected).

I've ran `analysis-stats` on libstd locally, which works fine, and also ran this PR locally for a short while without issues.

Note that this *still* has no (or almost no) user-facing impact, because the rest of r-a still relies on some local item support hacks.

bors r+

Co-authored-by: Jonas Schievink <jonasschievink@gmail.com>
2021-02-03 14:41:13 +00:00
Jonas Schievink
63744fe128 Test for name resolution with DefMap shortcut 2021-02-03 15:33:25 +01:00