Criterions for Refactoring

Non-local control flows

Toy example

This example shows the definition of y as a branching statement that exits from the function immediately on one branch:

pub fn original_foo() -> String {
    let x = '1';
    let y = if x > '2' {
        x
    } else {
        return String::from("y")
    };
    String::from(y)
}

To extract the definition of y, one needs to account for the return statement. However, IntelliJ's did not:

pub fn new_foo() -> String {
    let x = '1';
    let y = bar_extracted(x);
    String::from(y)
}

fn bar_extracted(x: char) -> char {
    if x > '2' {
        x
    } else {
        return String::from("y")
    }
}

This did not compile as returning String for a function that returns char is invalid. The compiler message is below:

error[E0308]: mismatched types
  --> src/non_control_flow.rs:26:16
   |
22 | fn bar_extracted(x: char) -> char {
   |                              ---- expected `char` because of return type
...
26 |         return String::from("y")
   |                ^^^^^^^^^^^^^^^^^ expected `char`, found struct `String`

A useful container that can be the return value of the extracted function is Result<A, B> where A is the type for successful result and B is for failures. In this case, char in the "success" case, and String is the "failure" case:

pub fn new_foo_fixed() -> String {
    let x = '1';
    let y = match bar_extracted_fixed(x) {
        Ok(x) => x,
        Err(s) => return s,
    };
    String::from(y)
}

fn bar_extracted_fixed(x: char) -> Result<char, String> {
    if x > '2' {
        Ok(x)
    } else {
        Err(String::from("y"))
    }
}

Formalized criterion

For a given program $P_0 = Stmt_{original}$, extracting a subset of its statements $Stmt_{extracted}$ containing $Stmt_{NLCF} = \{\text{return}, \text{break}, \text{continue} \}$ i.e. $Stmt_{NLCF} \in Stmt_{extracted}$, the resulting extracted function needs to have 4 distinct possible control flows to propagate the $Stmt_{NLCF}$ back to the function being extracted from i.e. $P_1 = Stmt_{new}$ such that $Stmt_{NLCF} \in Stmt_{new}$ and for any possible input $X, P_0(X) = P_1(X)$.

Implementation suggestion.

Borrowed reference

Toy example

There are 5 small toy examples here, which demonstrates borrowing levels needed simply:

if there is no use after the extracted block, then ownership can be transferred to the extracted function, otherwise, the extracted function must use a reference.
if there are mutation done then the extracted function must use a mutable borrow, otherwise, the extracted function must use an immutable borrow.
For the sake of the examples, I will only borrow reference and never transfer ownership. For the most parts, Intellij's implementation got the borrowing right, however, on the mutable borrow--IntelliJ did not change the value using the Kleene star operation. Below is the original function:

pub fn extract_read_and_write() {
    let mut x = 1;
    x = 5;
    println!("x={}", x);
    println!("x={}", x);
}

The extracted function is this:

fn extract_read_and_write_bar(x: &mut i32) {
    x = 5;
    println!("x={}", x);
}

This causes an error because it assigns the value to the reference of x rather than the value of x. To correct this, simply use the Kleene star operation:

fn extract_read_and_write_bar_fixed(x: &mut i32) {
    *x = 5;
    println!("x={}", x);
}

Then the new function is:

pub fn extract_read_and_write_new() {
    let mut x = 1;
    extract_read_and_write_bar_fixed(&mut x);
    println!("x={}", x);
}

Formalized criterion

For a given program $P_0 = Stmt_{original}$ with control flow graph $CFG_{original}$, extracting a subset of its statements $Stmt_{extracted}$, for all node $n$ of the $CFG_{original}$ if $use[n]$.

Different in/out lifetime

Toy example

There are 3 lifetimes involved in this example: 'static which lives throughout, 'a which lives throughout the function original_foo i.e. scope a, and 'b lives through the code block shown as scope b. The code to be extracted in this example is the conditional statement for z definition. For this conditional branch, the input lifetimes includes (z, 'a), (x_ref, 'a), (&y, 'b), (&W, 'static) and the output is either (&y, 'b) or (&W, 'static) with 'static living at least as long as 'b. This is valid as z is dropped at the end of scope b so both output references are valid.

const W: i32 = 5; // 'static

pub fn original_foo () { // scope a
    let x = 1;
    let x_ref = &x; // 'a
    let mut z : &i32; // 'a
    { // scope b
        let y = 2;
        z = &y; // 'b
        z = if *z < *x_ref {
            &y // 'b
        } else {
            &W // 'static
        };
        println!("{}", *z);
    }
}

To extract the conditional then one needs to account for those input lifetimes through annotations and then compute and annotate the output lifetime for the function. However, IntelliJ's did not account for this:

pub fn new_foo () {
    let x = 1;
    let x_ref = &x;
    let mut z : &i32;
    {
        let y = 2;
        z = &y;
        z = bar_extracted(x_ref, z, &y);
        println!("{}", *z);
    }
}

fn bar_extracted(x_ref: &i32, z: &mut &i32, y: &i32) -> &i32 {
    if *z < *x_ref {
        &y
    } else {
        &W
    }
}

This did not compile as the function more than 1 lifetime, needs to be annotated so for the borrow checker to accept it. The compile message is here:

error[E0106]: missing lifetime specifier
  --> src/in_out_lifetimes.rs:44:57
   |
44 | fn bar_extracted(x_ref: &i32, z: &mut &i32, y: &i32) -> &i32 {
   |                         ----     ---------     ----     ^ expected named lifetime parameter
   |

To fix the extracted function, we need to figure out the annotations that was mentioned in the beginning--the input remains the same. For the output, since 'b is the smallest lifetime that fits with the current usage i.e. 'static : 'b meaning 'static lives at least as long as 'b, the output should be 'b:

pub fn new_foo_fixed () {
    let x = 1;
    let x_ref = &x;
    let mut z : &i32;
    {
        let y = 2;
        z = &y;
        z = bar_fixed(x_ref, z, &y);
        println!("{}", *z);
    }
}

fn bar_fixed<'a, 'b>(x_ref: & 'a i32, z: & 'a i32, y: &'b i32) -> & 'b i32 {
    if *z < *x_ref {
        y
    } else {
        &W
    }
}

Formalized criterion

Lifetime bounds

Toy example

This example deals with lifetimes that requires strict bounds to be extracted correctly. We see that p is a mutable pointer so any assignment to (p, 'a) needs to live at least as long as it so the bounds here for (&x, 'b) requires 'b: 'a which in Rust lifetime bounds syntax means 'b needs to live at least as long as 'a.

pub fn original_foo(){
    let p : &mut &i32 = &mut &0; // 'a
    {
        let x = 1;
        *p = &x; // 'b
    }
}

To extract this again annotations are needed on the input to let it compile but for this extraction to be semantically correct, the bounds need to be added too. IntelliJ extraction did not do the annotation again as above:

pub fn new_foo(){
    let p : &mut &i32 = &mut &0;
    {
        let x = 1;
        bar_extracted(p, &x);
        println!("{}", **p);
    }
}

fn bar_extracted(p: &mut &i32, x: &i32) {
    *p = &x;
}

This causes the following compiler error:

error: lifetime may not live long enough
  --> src/lifetime_bounds.rs:20:5
   |
19 | fn bar_extracted(p: &mut &i32, x: &i32) {
   |                          -        - let's call the lifetime of this reference `'1`
   |                          |
   |                          let's call the lifetime of this reference `'2`
20 |     *p = &x;
   |     ^^^^^^^ assignment requires that `'1` must outlive `'2`
   |

Fixing this extraction requires annotating the lifetimes as well as adding its bounds so that the value the pointer refers to never dies before the pointer dies:

pub fn new_foo_fixed(){
    let p : &mut &i32 = &mut &0;
    {
        let x = 1;
        bar_fixed(p, &x);
        println!("{}", **p);
    }
}

fn bar_fixed<'a, 'b: 'a>(p: & 'a mut & 'b i32, x: & 'b i32) {
    *p = &x;
}

Formalized criterion

Extract to trait

Toy example

This example setups require:

a trait that requires multiple lifetimes in its function: MultiLifetimeTrait.
a struct that implements the trait: SimpleStruct.
a simple function that utilizes the struct and its trait function.

The trait function trait_function requires references of 2 lifetimes 'a and 'b such that 'a : 'b ('a lives at least as long as 'b) and outputs a reference of lifetime 'b; this includes references with lifetime 'a because the bounds 'a: 'b. The function in (3) is a simple call to the trait function and assigning the output to a mutable reference z:

trait MultiLifetimeTrait<'b, 'a: 'b> {
    fn trait_function(self: &Self, x: & 'a i32, y: &'b i32) -> & 'b i32;
}

struct SimpleStruct;

impl<'b, 'a: 'b> MultiLifetimeTrait<'b, 'a> for SimpleStruct {
    fn trait_function(&self, x: &'a i32, y: &'b i32) -> &'b i32 {
        if *x < *y {
            y
        } else {
            x
        }
    }
}

pub fn original_foo<'b, 'a: 'b>(x: &'a i32, y: &'b i32) {
    let foo = SimpleStruct;
    let z = &mut &0;
    *z = foo.trait_function(x, y);
}

To extract this assignment to *z, you need to make sure that the lifetime of the extracted method is compatible with the lifetime of the inputs to trait function---in this case, having the same lifetime signature works (as it is only one line--a toy example after all). However, same as with the above lifetime examples, IntelliJ did not do it:

pub fn new_foo<'b, 'a: 'b>(x: &'a i32, y: &'b i32) {
    let foo = SimpleStruct;
    let z = &mut &0;
    *z = bar_extracted(x, y, foo);
}

fn bar_extracted(x: &i32, y: &i32, foo: SimpleStruct) -> &i32 {
    foo.trait_function(x, y)
}

This causes the following compiler error:

error[E0106]: missing lifetime specifier
  --> src/extract_to_trait.rs:30:58
   |
30 | fn bar_extracted(x: &i32, y: &i32, foo: SimpleStruct) -> &i32 {
   |                     ----     ----                        ^ expected named lifetime parameter
   |

Fixing the extraction by adding the lifetime annotations and the bounds:

pub fn new_foo_fixed<'b, 'a: 'b>(x: &'a i32, y: &'b i32) {
    let foo = SimpleStruct;
    let z = &mut &0;
    *z = bar_fixed(x, y, foo);
}

fn bar_fixed<'b, 'a: 'b>(x: &'a i32, y: &'b i32, foo: SimpleStruct) -> &'b i32 {
    foo.trait_function(x, y)
}

Formalized criterion

Multiple expressions with different lifetimes

Toy example

Usually when you extract to function, you expect to reuse that function on possible duplicate extracted code elsewhere in your source code. In this example, there are two duplicate segments that requires different level of lifetime usage. In original_foo1, we requires strictly that lifetimes for references p and x are the same 'a. However, in original_foo2, we only require that lifetime 'b : 'a, i.e. 'b need only live as long as 'a so 'a can be alive longer 'b.

pub fn original_foo1<'a> (p: &'a mut &'a i32, x: &'a i32){
    *p = x;
}

pub fn original_foo2<'a, 'b : 'a>(p: &'a mut &'b i32, x: &'b i32){
    *p = x;
}

To extract from original_foo1, we need to keep track of the lifetimes and annotate it same as original_foo1. However, again IntelliJ do not account for this:

pub fn new_foo1<'a> (p: &'a mut &'a i32, x: &'a i32){
    bar_extracted(p, x);
}

fn bar_extracted(p: &mut &i32, x: &i32) {
    *p = x;
}

This causes the following error:

error: lifetime may not live long enough
  --> src/multiple_expressions_with_different_lifetimes.rs:15:5
   |
14 | fn bar_extracted(p: &mut &i32, x: &i32) {
   |                          -        - let's call the lifetime of this reference `'1`
   |                          |
   |                          let's call the lifetime of this reference `'2`
15 |     *p = x;
   |     ^^^^^^ assignment requires that `'1` must outlive `'2`
   |

Fixing the extraction by including the bounds:

pub fn new_foo1_fixed<'a> (p: &'a mut &'a i32, x: &'a i32) {
    bar_fixed(p, x);
}

fn bar_fixed<'a>(p: & 'a mut & 'a i32, x: & 'a i32) {
    *p = &x;
}

However, we cannot simply replace original_foo2 body with a function call to the extracted function even though it is exactly the same as the lifetime is different:

pub fn new_foo2<'a, 'b : 'a>(p: &'a mut &'b i32, x: &'b i32){
    bar_fixed(p, x);
}

This causes the following error by the compiler:

error: lifetime may not live long enough
  --> src/multiple_expressions_with_different_lifetimes.rs:25:5
   |
24 | pub fn new_foo2<'a, 'b : 'a>(p: &'a mut &'b i32, x: &'b i32){
   |                 --  -- lifetime `'b` defined here
   |                 |
   |                 lifetime `'a` defined here
25 |     bar_fixed(p, x);
   |     ^^^^^^^^^^^^^^^ argument requires that `'a` must outlive `'b`
   |

So the multiple expression extractions also need to calculate the lifetime at every "duplicate" expressions it encounters.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Criterions for Refactoring

Non-local control flows

Toy example

Formalized criterion

Borrowed reference

Toy example

Formalized criterion

Different in/out lifetime

Toy example

Formalized criterion

Lifetime bounds

Toy example

Formalized criterion

Extract to trait

Toy example

Formalized criterion

Multiple expressions with different lifetimes

Toy example

Formalized criterion

Content Pages

Clone this wiki locally