aboutsummaryrefslogtreecommitdiff
Commit message (Collapse)AuthorAgeFilesLines
...
* | | | Merge #7535bors[bot]2021-02-054-1/+2189
|\ \ \ \ | |/ / / |/| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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 <[email protected]>
| * | | allow extracted body to be indented(dedent it)Vladyslav Katasonov2021-02-051-13/+101
| | | |
| * | | allow transitive `&mut` access for fields in extract_functionVladyslav Katasonov2021-02-051-27/+92
| | | |
| * | | add tests for extracting if/match/while/for exprsVladyslav Katasonov2021-02-041-0/+120
| | | |
| * | | document extract_function assist implementationVladyslav Katasonov2021-02-041-22/+126
| | | |
| * | | use `&T` for non copy params of extracted functionVladyslav Katasonov2021-02-041-2/+55
| | | | | | | | | | | | | | | | Use shared ref if param is not `T: Copy` and is used after body
| * | | split extract_function into pieces and order themVladyslav Katasonov2021-02-041-380/+510
| | | |
| * | | remove ignored test for downgrading mut to sharedVladyslav Katasonov2021-02-031-30/+0
| | | |
| * | | allow calling `&mut` methods on outer vars when extracing functionVladyslav Katasonov2021-02-031-0/+116
| | | |
| * | | allow `&mut param` when extracting functionVladyslav Katasonov2021-02-031-3/+107
| | | | | | | | | | | | | | | | | | | | | | | | Recognise &mut as variable modification. This allows extracting functions with `&mut var` with `var` being in outer scope
| * | | allow modifications of vars from outer scope inside extracted functionVladyslav Katasonov2021-02-032-46/+337
| | | | | | | | | | | | | | | | | | | | It currently allows only directly setting variable. No `&mut` references or methods.
| * | | allow local variables to be used after extracted bodyVladyslav Katasonov2021-02-031-41/+183
| | | | | | | | | | | | | | | | | | | | when variable is defined inside extracted body export this variable to original scope via return value(s)
| * | | change TODO to FIXMEVladyslav Katasonov2021-02-031-2/+2
| | | |
| * | | disable test for downgrading mutability on extractVladyslav Katasonov2021-02-031-0/+3
| | | |
| * | | convert IdentPat to Pat via IntoVladyslav Katasonov2021-02-031-5/+5
| | | | | | | | | | | | | | | | before child getter was used
| * | | support extracting methods; no mut loweringVladyslav Katasonov2021-02-031-37/+191
| | | | | | | | | | | | | | | | | | | | currently mut refernce will *not* be downgraded to shared if it is sufficient(see relevant test for example)
| * | | initial version of extract function assistVladyslav Katasonov2021-02-033-0/+848
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | there are a few currently limitations: * no modifications of function body * does not handle mutability and references * no method support * may produce incorrect results
* | | | Merge #7561bors[bot]2021-02-043-8/+9
|\ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 7561: Avoid using ModPath's fields directly r=jonas-schievink a=jonas-schievink bors r+ Co-authored-by: Jonas Schievink <[email protected]>
| * | | | Avoid using ModPath's fields directlyJonas Schievink2021-02-043-8/+9
|/ / / /
* | | | Merge #7559bors[bot]2021-02-0418-51/+70
|\ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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 <[email protected]>
| * | | | Make `ModPath`'s representation privateJonas Schievink2021-02-0418-51/+70
| | | | |
* | | | | Merge #7558bors[bot]2021-02-041-2/+2
|\ \ \ \ \ | |/ / / / |/| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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 <[email protected]>
| * | | | Update thread_localkjeremy2021-02-041-2/+2
|/ / / / | | | | | | | | | | | | | | | | Pulls in https://github.com/Amanieu/thread_local-rs/pull/30 which fixes a leak when dropping ThreadLocal.
* | | | Merge #7557bors[bot]2021-02-044-21/+67
|\ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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 <[email protected]>
| * | | | Intern `TypeRef`s in the containing `ItemTree`Jonas Schievink2021-02-044-21/+67
|/ / / /
* | | | Merge #7555bors[bot]2021-02-042-14/+11
|\ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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 <[email protected]>
| * | | | Expander: store a LocalModuleId, not ModuleIdJonas Schievink2021-02-042-14/+11
|/ / / / | | | | | | | | | | | | | | | | It already stores the DefMap containing the module, so having a full ModuleId is unnecessary and makes it easier to mix things up
* | | | Merge #7554bors[bot]2021-02-045-36/+67
|\ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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 <[email protected]>
| * | | | Don't keep the parent DefMap alive via ArcJonas Schievink2021-02-045-36/+67
|/ / / / | | | | | | | | | | | | | | | | This seems like it could easily leak a lot of memory since we don't currently run GC
* | | | Merge #7553bors[bot]2021-02-041-2/+42
|\ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 7553: More architecture.md r=matklad a=matklad bors r+ 🤖 Co-authored-by: Aleksey Kladov <[email protected]>
| * | | | More architecture.mdAleksey Kladov2021-02-041-2/+42
|/ / / /
* | | | Merge #7547bors[bot]2021-02-032-21/+26
|\ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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 <[email protected]>
| * | | | Split out ItemScope::dump from DefMap::dumpJonas Schievink2021-02-032-21/+26
|/ / / /
* | | | Merge #7546bors[bot]2021-02-032-0/+11
|\ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 7546: Add newline between block and crate maps r=jonas-schievink a=jonas-schievink bors r+ Co-authored-by: Jonas Schievink <[email protected]>
| * | | | Add newline between block and crate mapsJonas Schievink2021-02-032-0/+11
| | | | |
* | | | | Merge #7545bors[bot]2021-02-031-0/+1
|\ \ \ \ \ | |/ / / / |/| | | | | | | | | | | | | | | | | | | | | | | | | | | | | 7545: Add a FIXME to ItemTree r=jonas-schievink a=jonas-schievink bors r+ Co-authored-by: Jonas Schievink <[email protected]>
| * | | | Add a FIXME to ItemTreeJonas Schievink2021-02-031-0/+1
|/ / / /
* | | | Merge #7544bors[bot]2021-02-032-1/+25
|\ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 7544: Update `DefMap` and `block_def_map` docs r=jonas-schievink a=jonas-schievink bors r+ Co-authored-by: Jonas Schievink <[email protected]>
| * | | | Update `DefMap` and `block_def_map` docsJonas Schievink2021-02-032-1/+25
| | | | |
* | | | | Merge #7543bors[bot]2021-02-031-23/+23
|\ \ \ \ \ | |/ / / / |/| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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 <[email protected]>
| * | | | typo fixesKushagra Gupta2021-02-031-4/+4
| | | | |
| * | | | Grammar fixesKushagra Gupta2021-02-031-20/+20
|/ / / / | | | | | | | | | | | | | | | | I think line 235 is still wrong, but I am not sure. Is the `crated/tt` in line 252 supposed to be `crates/tt`?
* | | | Merge #7541bors[bot]2021-02-0312-100/+211
|\ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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 <[email protected]>
| * | | | Test for name resolution with DefMap shortcutJonas Schievink2021-02-031-0/+33
| | | | |
| * | | | Shortcut `block_def_map` if there's no inner itemsJonas Schievink2021-02-035-11/+26
| | | | | | | | | | | | | | | | | | | | | | | | | This previously didn't work, but apparently only because of the wonky test setup
| * | | | Use body lowering for block_def_map testsJonas Schievink2021-02-033-68/+117
| | | | | | | | | | | | | | | | | | | | Removes the hacky and buggy custom lowering code
| * | | | Use block_def_map in body loweringJonas Schievink2021-02-036-26/+40
| | | | |
* | | | | Merge #7539bors[bot]2021-02-033-14/+50
|\ \ \ \ \ | |/ / / / |/| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 7539: Add cargo file tidy test r=edwin0cheng a=edwin0cheng bors r+ cc @pksunkara Co-authored-by: Edwin Cheng <[email protected]>
| * | | | Add cargo file tidy testEdwin Cheng2021-02-033-14/+50
|/ / / /
* | | | Merge #7538bors[bot]2021-02-031-1/+1
|\ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 7538: Make sure normal dependencies always have version r=edwin0cheng a=pksunkara How do I prevent this happening in the future by doing something in the CI? IIRC this is the second time. Co-authored-by: Pavan Kumar Sunkara <[email protected]>
generated by cgit v1.2.3 (git 2.25.1) at 2024-12-27 07:51:24 +0000