|
| 1 | + |
| 2 | +Gopls v0.14 supports a new refactoring operation: |
| 3 | +inlining of function calls. |
| 4 | + |
| 5 | +You can find it in VS Code by selecting a static call to a function or |
| 6 | +method f and choosing the `Refactor...` command followed by `Inline |
| 7 | +call to f`. |
| 8 | +Other editors and LSP clients have their own idiomatic command for it; |
| 9 | +for example, in Emacs with Eglot it is |
| 10 | +[`M-x eglot-code-action-inline`](https://joaotavora.github.io/eglot/#index-M_002dx-eglot_002dcode_002daction_002dinline) |
| 11 | +and in Vim with coc.nvim it is `coc-rename`. |
| 12 | + |
| 13 | +<!-- source code used for images: |
| 14 | +
|
| 15 | +func six() int { |
| 16 | + return sum(1, 2, 3) |
| 17 | +} |
| 18 | +
|
| 19 | +func sum(values ...int) int { |
| 20 | + total := 0 |
| 21 | + for _, v := range values { |
| 22 | + total += v |
| 23 | + } |
| 24 | + return total |
| 25 | +} |
| 26 | +--> |
| 27 | + |
| 28 | + |
| 29 | + |
| 30 | +Inlining replaces the call expression by a copy of the function body, |
| 31 | +with parameters replaced by arguments. |
| 32 | +Inlining is useful for a number of reasons. |
| 33 | +Perhaps you want to eliminate a call to a deprecated |
| 34 | +function such as `ioutil.ReadFile` by replacing it with a call to the |
| 35 | +newer `os.ReadFile`; inlining will do that for you. |
| 36 | +Or perhaps you want to copy and modify an existing function in some |
| 37 | +way; inlining can provide a starting point. |
| 38 | +The inlining logic also provides a building block for |
| 39 | +other refactorings to come, such as "change signature". |
| 40 | + |
| 41 | +Not every call can be inlined. |
| 42 | +Of course, the tool needs to know which function is being called, so |
| 43 | +you can't inline a dynamic call through a function value or interface |
| 44 | +method; but static calls to methods are fine. |
| 45 | +Nor can you inline a call if the callee is declared in another package |
| 46 | +and refers to non-exported parts of that package, or to [internal |
| 47 | +packages](https://go.dev/doc/go1.4#internalpackages) that are |
| 48 | +inaccessible to the caller. |
| 49 | + |
| 50 | +When inlining is possible, it's critical that the tool preserve |
| 51 | +the original behavior of the program. |
| 52 | +We don't want refactoring to break the build, or, worse, to introduce |
| 53 | +subtle latent bugs. |
| 54 | +This is especially important when inlining tools are used to perform |
| 55 | +automated clean-ups in large code bases. |
| 56 | +We must be able to trust the tool. |
| 57 | +Our inliner is very careful not to make guesses or unsound |
| 58 | +assumptions about the behavior of the code. |
| 59 | +However, that does mean it sometimes produces a change that differs |
| 60 | +from what someone with expert knowledge of the same code might have |
| 61 | +written by hand. |
| 62 | + |
| 63 | +In the most difficult cases, especially with complex control flow, it |
| 64 | +may not be safe to eliminate the function call at all. |
| 65 | +For example, the behavior of a `defer` statement is intimately tied to |
| 66 | +its enclosing function call, and `defer` is the only control |
| 67 | +construct that can be used to handle panics, so it cannot be reduced |
| 68 | +into simpler constructs. |
| 69 | +So, for example, given a function f defined as: |
| 70 | + |
| 71 | +```go |
| 72 | +func f(s string) { |
| 73 | + defer fmt.Println("goodbye") |
| 74 | + fmt.Println(s) |
| 75 | +} |
| 76 | +``` |
| 77 | +a call `f("hello")` will be inlined to: |
| 78 | +```go |
| 79 | + func() { |
| 80 | + defer fmt.Println("goodbye") |
| 81 | + fmt.Println("hello") |
| 82 | + }() |
| 83 | +``` |
| 84 | +Although the parameter was eliminated, the function call remains. |
| 85 | + |
| 86 | +An inliner is a bit like an optimizing compiler. |
| 87 | +A compiler is considered "correct" if it doesn't change the meaning of |
| 88 | +the program in translation from source language to target language. |
| 89 | +An _optimizing_ compiler exploits the particulars of the input to |
| 90 | +generate better code, where "better" usually means more efficient. |
| 91 | +As users report inputs that cause the compiler to emit suboptimal |
| 92 | +code, the compiler is improved to recognize more cases, or more rules, |
| 93 | +and more exceptions to rules---but this process has no end. |
| 94 | +Inlining is similar, except that "better" code means tidier code. |
| 95 | +The most conservative translation provides a simple but (hopefully!) |
| 96 | +correct foundation, on top of which endless rules, and exceptions to |
| 97 | +rules, can embellish and improve the quality of the output. |
| 98 | + |
| 99 | +The following section lists some of the technical |
| 100 | +challenges involved in sound inlining: |
| 101 | + |
| 102 | +- **Effects:** When replacing a parameter by its argument expression, |
| 103 | + we must be careful not to change the effects of the call. For |
| 104 | + example, if we call a function `func twice(x int) int { return x + x }` |
| 105 | + with `twice(g())`, we do not want to see `g() + g()`, which would |
| 106 | + cause g's effects to occur twice, and potentially each call might |
| 107 | + return a different value. All effects must occur the same number of |
| 108 | + times, and in the same order. This requires analyzing both the |
| 109 | + arguments and the callee function to determine whether they are |
| 110 | + "pure", whether they read variables, or whether (and when) they |
| 111 | + update them too. The inliner will introduce a declaration such as |
| 112 | + `var x int = g()` when it cannot prove that it is safe to substitute |
| 113 | + the argument throughout. |
| 114 | + |
| 115 | +- **Constants:** If inlining always replaced a parameter by its argument |
| 116 | + when the value is constant, some programs would no longer build |
| 117 | + because checks previously done at run time would happen at compile time. |
| 118 | + For example `func index(s string, i int) byte { return s[i] }` |
| 119 | + is a valid function, but if inlining were to replace the call `index("abc", 3)` |
| 120 | + by the expression `"abc"[3]`, the compiler will report that the |
| 121 | + index `3` is out of bounds for the string `"abc"`. |
| 122 | + The inliner will prevent substitution of parameters by problematic |
| 123 | + constant arguments, again introducing a `var` declaration instead. |
| 124 | + |
| 125 | +- **Referential integrity:** When a parameter variable is replaced by |
| 126 | + its argument expression, we must ensure that any names in the |
| 127 | + argument expression continue to refer to the same thing---not to a |
| 128 | + different declaration in the callee function body that happens to |
| 129 | + use the same name! The inliner must replace local references such as |
| 130 | + `Printf` by qualified references such as `fmt.Printf`, and add an |
| 131 | + import of package `fmt` as needed. |
| 132 | + |
| 133 | +- **Implicit conversions:** When passing an argument to a function, it |
| 134 | + is implicitly converted to the parameter type. |
| 135 | + If we eliminate the parameter variable, we don't want to |
| 136 | + lose the conversion as it may be important. |
| 137 | + For example, in `func f(x any) { y := x; fmt.Printf("%T", &y) }` the |
| 138 | + type of variable y is `any`, so the program prints `"*interface{}"`. |
| 139 | + But if inlining the call `f(1)` were to produce the statement `y := |
| 140 | + 1`, then the type of y would have changed to `int`, which could |
| 141 | + cause a compile error or, as in this case, a bug, as the program |
| 142 | + now prints `"*int"`. When the inliner substitutes a parameter variable |
| 143 | + by its argument value, it may need to introduce explicit conversions |
| 144 | + of each value to the original parameter type, such as `y := any(1)`. |
| 145 | + |
| 146 | +- **Last reference:** When an argument expression has no effects |
| 147 | + and its corresponding parameter is never used, the expression |
| 148 | + may be eliminated. However, if the expression contains the last |
| 149 | + reference to a local variable at the caller, this may cause a compile |
| 150 | + error because the variable is now unused! So the inliner must be |
| 151 | + cautious about eliminating references to local variables. |
| 152 | + |
| 153 | +This is just a taste of the problem domain. If you're curious, the |
| 154 | +documentation for [golang.org/x/tools/internal/refactor/inline](https://pkg.go.dev/golang.org/x/tools/internal/refactor/inline) has |
| 155 | +more detail. All of this is to say, it's a complex problem, and we aim |
| 156 | +for correctness first of all. We've already implemented a number of |
| 157 | +important "tidiness optimizations" and we expect more to follow. |
| 158 | + |
| 159 | +Please give the inliner a try, and if you find any bugs (where the |
| 160 | +transformation is incorrect), please do report them. We'd also like to |
| 161 | +hear what "optimizations" you'd like to see next. |
0 commit comments