|
| 1 | +- Feature Name: default_and_expanded_errors_for_rustc |
| 2 | +- Start Date: 2016-06-07 |
| 3 | +- RFC PR: (leave this empty) |
| 4 | +- Rust Issue: (leave this empty) |
| 5 | + |
| 6 | +# Summary |
| 7 | +This RFC proposes an update to error reporting in rustc. Its focus is to change the format of Rust |
| 8 | +error messages and improve --explain capabilities to focus on the user's code. The end goal is for |
| 9 | +errors and explain text to be more readable, more friendly to new users, while still helping Rust |
| 10 | +coders fix bugs as quickly as possible. We expect to follow this RFC with a supplemental RFC that |
| 11 | +provides a writing style guide for error messages and explain text with a focus on readability and |
| 12 | +education. |
| 13 | + |
| 14 | +# Motivation |
| 15 | + |
| 16 | +## Default error format |
| 17 | + |
| 18 | +Rust offers a unique value proposition in the landscape of languages in part by codifying concepts |
| 19 | +like ownership and borrowing. Because these concepts are unique to Rust, it's critical that the |
| 20 | +learning curve be as smooth as possible. And one of the most important tools for lowering the |
| 21 | +learning curve is providing excellent errors that serve to make the concepts less intimidating, |
| 22 | +and to help 'tell the story' about what those concepts mean in the context of the programmer's code. |
| 23 | + |
| 24 | +[as text] |
| 25 | +``` |
| 26 | +src/test/compile-fail/borrowck/borrowck-borrow-from-owned-ptr.rs:29:22: 29:30 error: cannot borrow `foo.bar1` as mutable more than once at a time [E0499] |
| 27 | +src/test/compile-fail/borrowck/borrowck-borrow-from-owned-ptr.rs:29 let _bar2 = &mut foo.bar1; |
| 28 | + ^~~~~~~~ |
| 29 | +src/test/compile-fail/borrowck/borrowck-borrow-from-owned-ptr.rs:29:22: 29:30 help: run `rustc --explain E0499` to see a detailed explanation |
| 30 | +src/test/compile-fail/borrowck/borrowck-borrow-from-owned-ptr.rs:28:21: 28:29 note: previous borrow of `foo.bar1` occurs here; the mutable borrow prevents subsequent moves, borrows, or modification of `foo.bar1` until the borrow ends |
| 31 | +src/test/compile-fail/borrowck/borrowck-borrow-from-owned-ptr.rs:28 let bar1 = &mut foo.bar1; |
| 32 | + ^~~~~~~~ |
| 33 | +src/test/compile-fail/borrowck/borrowck-borrow-from-owned-ptr.rs:31:2: 31:2 note: previous borrow ends here |
| 34 | +src/test/compile-fail/borrowck/borrowck-borrow-from-owned-ptr.rs:26 fn borrow_same_field_twice_mut_mut() { |
| 35 | +src/test/compile-fail/borrowck/borrowck-borrow-from-owned-ptr.rs:27 let mut foo = make_foo(); |
| 36 | +src/test/compile-fail/borrowck/borrowck-borrow-from-owned-ptr.rs:28 let bar1 = &mut foo.bar1; |
| 37 | +src/test/compile-fail/borrowck/borrowck-borrow-from-owned-ptr.rs:29 let _bar2 = &mut foo.bar1; |
| 38 | +src/test/compile-fail/borrowck/borrowck-borrow-from-owned-ptr.rs:30 *bar1; |
| 39 | +src/test/compile-fail/borrowck/borrowck-borrow-from-owned-ptr.rs:31 } |
| 40 | + ^ |
| 41 | +``` |
| 42 | + |
| 43 | +[as image] |
| 44 | + |
| 45 | + |
| 46 | +*Example of a borrow check error in the current compiler* |
| 47 | + |
| 48 | +Though a lot of time has been spent on the current error messages, they have a couple flaws which |
| 49 | +make them difficult to use. Specifically, the current error format: |
| 50 | + |
| 51 | +* Repeats the file position on the left-hand side. This offers no additional information, but |
| 52 | +instead makes the error harder to read. |
| 53 | +* Prints messages about lines often out of order. This makes it difficult for the developer to |
| 54 | +glance at the error and recognize why the error is occuring |
| 55 | +* Lacks a clear visual break between errors. As more errors occur it becomes more difficult to tell |
| 56 | +them apart. |
| 57 | +* Uses technical terminology that is difficult for new users who may be unfamiliar with compiler |
| 58 | +terminology or terminology specific to Rust. |
| 59 | + |
| 60 | +This RFC details a redesign of errors to focus more on the source the programmer wrote. This format |
| 61 | +addresses the above concerns by eliminating clutter, following a more natural order for help |
| 62 | +messages, and pointing the user to both "what" the error is and "why" the error is occurring by |
| 63 | +using color-coded labels. Below you can see the same error again, this time using the proposed |
| 64 | +format: |
| 65 | + |
| 66 | +[as text] |
| 67 | +``` |
| 68 | +error[E0499]: cannot borrow `foo.bar1` as mutable more than once at a time |
| 69 | + --> src/test/compile-fail/borrowck/borrowck-borrow-from-owned-ptr.rs:29:22 |
| 70 | + | |
| 71 | +28 | let bar1 = &mut foo.bar1; |
| 72 | + | -------- first mutable borrow occurs here |
| 73 | +29 | let _bar2 = &mut foo.bar1; |
| 74 | + | ^^^^^^^^ second mutable borrow occurs here |
| 75 | +30 | *bar1; |
| 76 | +31 | } |
| 77 | + | - first borrow ends here |
| 78 | +``` |
| 79 | + |
| 80 | +[as image] |
| 81 | + |
| 82 | +<a href="http://www.jonathanturner.org/images/new_errors_3.png"><img src="http://www.jonathanturner.org/images/new_errors_3.png" height="200" width="750" ></a> |
| 83 | + |
| 84 | +*Example of the same borrow check error in the proposed format* |
| 85 | + |
| 86 | +## Expanded error format (revised --explain) |
| 87 | + |
| 88 | +Languages like Elm have shown how effective an educational tool error messages can be if the |
| 89 | +explanations like our --explain text are mixed with the user's code. As mentioned earlier, it's |
| 90 | +crucial for Rust to be easy-to-use, especially since it introduces a fair number of concepts that |
| 91 | +may be unfamiliar to the user. Even experienced users may need to use --explain text from time to |
| 92 | +time when they encounter unfamiliar messages. |
| 93 | + |
| 94 | +While we have --explain text today, it uses generic examples that require the user to mentally |
| 95 | +translate the given example into what works for their specific situation. |
| 96 | + |
| 97 | +``` |
| 98 | +You tried to move out of a value which was borrowed. Erroneous code example: |
| 99 | +
|
| 100 | +use std::cell::RefCell; |
| 101 | +
|
| 102 | +struct TheDarkKnight; |
| 103 | +
|
| 104 | +impl TheDarkKnight { |
| 105 | + fn nothing_is_true(self) {} |
| 106 | +} |
| 107 | +... |
| 108 | +``` |
| 109 | + |
| 110 | +*Example of the current --explain (showing E0507)* |
| 111 | + |
| 112 | +To help users, this RFC proposes a new `--explain errors`. This new mode is more textual error |
| 113 | +reporting mode that gives additional explanation to help better understand compiler messages. The |
| 114 | +end result is a richer, on-demand error reporting style. |
| 115 | + |
| 116 | +``` |
| 117 | +error: cannot move out of borrowed content |
| 118 | + --> /Users/jturner/Source/errors/borrowck-move-out-of-vec-tail.rs:30:17 |
| 119 | +
|
| 120 | +I’m trying to track the ownership of the contents of `tail`, which is borrowed, through this match |
| 121 | +statement: |
| 122 | +
|
| 123 | +29 | match tail { |
| 124 | +
|
| 125 | +In this match, you use an expression of the form [...]. When you do this, it’s like you are opening |
| 126 | +up the `tail` value and taking out its contents. Because `tail` is borrowed, you can’t safely move |
| 127 | +the contents. |
| 128 | +
|
| 129 | +30 | [Foo { string: aa }, |
| 130 | + | ^^ cannot move out of borrowed content |
| 131 | +
|
| 132 | +You can avoid moving the contents out by working with each part using a reference rather than a |
| 133 | +move. A naive fix might look this: |
| 134 | +
|
| 135 | +30 | [Foo { string: ref aa }, |
| 136 | +
|
| 137 | +``` |
| 138 | + |
| 139 | +# Detailed design |
| 140 | + |
| 141 | +The RFC is separated into two parts: the format of error messages and the format of expanded error |
| 142 | +messages (using `--explain errors`). |
| 143 | + |
| 144 | +## Format of error messages |
| 145 | + |
| 146 | +The proposal is a lighter error format focused on the code the user wrote. Messages that help |
| 147 | +understand why an error occurred appear as labels on the source. The goals of this new format are |
| 148 | +to: |
| 149 | + |
| 150 | +* Create something that's visually easy to parse |
| 151 | +* Remove noise/unnecessary information |
| 152 | +* Present information in a way that works well for new developers, post-onboarding, and experienced |
| 153 | +developers without special configuration |
| 154 | +* Draw inspiration from Elm as well as Dybuk and other systems that have already improved on the |
| 155 | +kind of errors that Rust has. |
| 156 | + |
| 157 | +In order to accomplish this, the proposed design needs to satisfy a number of constraints to make |
| 158 | +the result maximally flexible across various terminals: |
| 159 | + |
| 160 | +* Multiple errors beside each other should be clearly separate and not muddled together. |
| 161 | +* Each error message should draw the eye to where the error occurs with sufficient context to |
| 162 | +understand why the error occurs. |
| 163 | +* Each error should have a "header" section that is visually distinct from the code section. |
| 164 | +* Code should visually stand out from text and other error messages. This allows the developer to |
| 165 | +immediately recognize their code. |
| 166 | +* Error messages should be just as readable when not using colors (eg for users of black-and-white |
| 167 | +terminals, color-impaired readers, weird color schemes that we can't predict, or just people that |
| 168 | +turn colors off) |
| 169 | +* Be careful using “ascii art” and avoid unicode. Instead look for ways to show the information |
| 170 | +concisely that will work across the broadest number of terminals. We expect IDEs to possibly allow |
| 171 | +for a more graphical error in the future. |
| 172 | +* Where possible, use labels on the source itself rather than sentence "notes" at the end. |
| 173 | +* Keep filename:line easy to spot for people who use editors that let them click on errors |
| 174 | + |
| 175 | +### Header |
| 176 | + |
| 177 | +``` |
| 178 | +error[E0499]: cannot borrow `foo.bar1` as mutable more than once at a time |
| 179 | + --> src/test/compile-fail/borrowck/borrowck-borrow-from-owned-ptr.rs:29:22 |
| 180 | +``` |
| 181 | + |
| 182 | +The header still serves the original purpose of knowing: a) if it's a warning or error, b) the text |
| 183 | +of the warning/error, and c) the location of this warning/error. We keep the error code, now a part |
| 184 | +of the error indicator, as a way to help improve search results. |
| 185 | + |
| 186 | +### Line number column |
| 187 | + |
| 188 | +``` |
| 189 | + | |
| 190 | +28 | |
| 191 | + | |
| 192 | +29 | |
| 193 | + | |
| 194 | +30 | |
| 195 | +31 | |
| 196 | + | |
| 197 | +``` |
| 198 | + |
| 199 | +The line number column lets you know where the error is occurring in the file. Because we only show |
| 200 | +lines that are of interest for the given error/warning, we elide lines if they are not annotated as |
| 201 | +part of the message (we currently use the heuristic to elide after one un-annotated line). |
| 202 | + |
| 203 | +Inspired by Dybuk and Elm, the line numbers are separated with a 'wall', a separator formed from |
| 204 | +pipe('|') characters, to clearly distinguish what is a line number from what is source at a glance. |
| 205 | + |
| 206 | +As the wall also forms a way to visually separate distinct errors, we propose extending this concept |
| 207 | +to also support span-less notes and hints. For example: |
| 208 | + |
| 209 | +``` |
| 210 | +92 | config.target_dir(&pkg) |
| 211 | + | ^^^^ expected `core::workspace::Workspace`, found `core::package::Package` |
| 212 | + = note: expected type `&core::workspace::Workspace<'_>` |
| 213 | + = note: found type `&core::package::Package` |
| 214 | +``` |
| 215 | +### Source area |
| 216 | + |
| 217 | +``` |
| 218 | + let bar1 = &mut foo.bar1; |
| 219 | + -------- first mutable borrow occurs here |
| 220 | + let _bar2 = &mut foo.bar1; |
| 221 | + ^^^^^^^^ second mutable borrow occurs here |
| 222 | + *bar1; |
| 223 | + } |
| 224 | + - first borrow ends here |
| 225 | +``` |
| 226 | + |
| 227 | +The source area shows the related source code for the error/warning. The source is laid out in the |
| 228 | +order it appears in the source file, giving the user a way to map the message against the source |
| 229 | +they wrote. |
| 230 | + |
| 231 | +Key parts of the code are labeled with messages to help the user understand the message. |
| 232 | + |
| 233 | +The primary label is the label associated with the main warning/error. It explains the **what** of |
| 234 | +the compiler message. By reading it, the user can begin to understand what the root cause of the |
| 235 | +error or warning is. This label is colored to match the level of the message (yellow for warning, |
| 236 | +red for error) and uses the ^^^ underline. |
| 237 | + |
| 238 | +Secondary labels help to understand the error and use blue text and --- underline. These labels |
| 239 | +explain the **why** of the compiler message. You can see one such example in the above message |
| 240 | +where the secondary labels explain that there is already another borrow going on. In another |
| 241 | +example, we see another way that primary and secondary work together to tell the whole story for |
| 242 | +why the error occurred. |
| 243 | + |
| 244 | +Taken together, primary and secondary labels create a 'flow' to the message. Flow in the message |
| 245 | +lets the user glance at the colored labels and quickly form an educated guess as to how to correctly |
| 246 | +update their code. |
| 247 | + |
| 248 | +Note: We'll talk more about additional style guidance for wording to help create flow in the |
| 249 | +subsequent style RFC. |
| 250 | + |
| 251 | +## Expanded error messages |
| 252 | + |
| 253 | +Currently, --explain text focuses on the error code. You invoke the compiler with --explain |
| 254 | +<error code> and receive a verbose description of what causes errors of that number. The resulting |
| 255 | +message can be helpful, but it uses generic sample code which makes it feel less connected to the |
| 256 | +user's code. |
| 257 | + |
| 258 | +We propose adding a new `--explain errors`. By passing this to the compiler (or to cargo), the |
| 259 | +compiler will switch to an expanded error form which incorporates the same source and label |
| 260 | +information the user saw in the default message with more explanation text. |
| 261 | + |
| 262 | +``` |
| 263 | +error: cannot move out of borrowed content |
| 264 | + --> /Users/jturner/Source/errors/borrowck-move-out-of-vec-tail.rs:30:17 |
| 265 | +
|
| 266 | +I’m trying to track the ownership of the contents of `tail`, which is borrowed, through this match |
| 267 | +statement: |
| 268 | +
|
| 269 | +29 | match tail { |
| 270 | +
|
| 271 | +In this match, you use an expression of the form [...]. When you do this, it’s like you are opening |
| 272 | +up the `tail` value and taking out its contents. Because `tail` is borrowed, you can’t safely move |
| 273 | +the contents. |
| 274 | +
|
| 275 | +30 | [Foo { string: aa }, |
| 276 | + | ^^ cannot move out of borrowed content |
| 277 | +
|
| 278 | +You can avoid moving the contents out by working with each part using a reference rather than a |
| 279 | +move. A naive fix might look this: |
| 280 | +
|
| 281 | +30 | [Foo { string: ref aa }, |
| 282 | +``` |
| 283 | + |
| 284 | +*Example of an expanded error message* |
| 285 | + |
| 286 | +The expanded error message effectively becomes a template. The text of the template is the |
| 287 | +educational text that is explaining the message more more detail. The template is then populated |
| 288 | +using the source lines, labels, and spans from the same compiler message that's printed in the |
| 289 | +default mode. This lets the message writer call out each label or span as appropriate in the |
| 290 | +expanded text. |
| 291 | + |
| 292 | +It's possible to also add additional labels that aren't necessarily shown in the default error mode |
| 293 | +but would be available in the expanded error format. This gives the explain text writer maximal |
| 294 | +flexibility without impacting the readability of the default message. I'm currently prototyping an |
| 295 | +implementation of how this templating could work in practice. |
| 296 | + |
| 297 | +## Tying it together |
| 298 | + |
| 299 | +Lastly, we propose that the final error message: |
| 300 | + |
| 301 | +``` |
| 302 | +error: aborting due to 2 previous errors |
| 303 | +``` |
| 304 | + |
| 305 | +Be changed to notify users of this ability: |
| 306 | + |
| 307 | +``` |
| 308 | +note: compile failed due to 2 errors. You can compile again with `--explain errors` for more information |
| 309 | +``` |
| 310 | + |
| 311 | +# Drawbacks |
| 312 | + |
| 313 | +Changes in the error format can impact integration with other tools. For example, IDEs that use a |
| 314 | +simple regex to detect the error would need to be updated to support the new format. This takes |
| 315 | +time and community coordination. |
| 316 | + |
| 317 | +While the new error format has a lot of benefits, it's possible that some errors will feel |
| 318 | +"shoehorned" into it and, even after careful selection of secondary labels, may still not read as |
| 319 | +well as the original format. |
| 320 | + |
| 321 | +There is a fair amount of work involved to update the errors and explain text to the proposed |
| 322 | +format. |
| 323 | + |
| 324 | +# Alternatives |
| 325 | + |
| 326 | +Rather than using the proposed error format format, we could only provide the verbose --explain |
| 327 | +style that is proposed in this RFC. Respected programmers like |
| 328 | +[John Carmack](https://twitter.com/ID_AA_Carmack/status/735197548034412546) have praised the Elm |
| 329 | +error format. |
| 330 | + |
| 331 | +``` |
| 332 | +Detected errors in 1 module. |
| 333 | +
|
| 334 | +-- TYPE MISMATCH --------------------------------------------------------------- |
| 335 | +The right argument of (+) is causing a type mismatch. |
| 336 | +
|
| 337 | +25| model + "1" |
| 338 | + ^^^ |
| 339 | +(+) is expecting the right argument to be a: |
| 340 | +
|
| 341 | + number |
| 342 | +
|
| 343 | +But the right argument is: |
| 344 | +
|
| 345 | + String |
| 346 | +
|
| 347 | +Hint: To append strings in Elm, you need to use the (++) operator, not (+). |
| 348 | +<http://package.elm-lang.org/packages/elm-lang/core/latest/Basics#++> |
| 349 | +
|
| 350 | +Hint: I always figure out the type of the left argument first and if it is acceptable on its own, I |
| 351 | +assume it is "correct" in subsequent checks. So the problem may actually be in how the left and |
| 352 | +right arguments interact. |
| 353 | +``` |
| 354 | + |
| 355 | +*Example of an Elm error* |
| 356 | + |
| 357 | +In developing this RFC, we experimented with both styles. The Elm error format is great as an |
| 358 | +educational tool, and we wanted to leverage its style in Rust. For day-to-day work, though, we |
| 359 | +favor an error format that puts heavy emphasis on quickly guiding the user to what the error is and |
| 360 | +why it occurred, with an easy way to get the richer explanations (using --explain) when the user |
| 361 | +wants them. |
| 362 | + |
| 363 | +# Stabilization |
| 364 | + |
| 365 | +Currently, this new rust error format is available on nightly using the |
| 366 | +```export RUST_NEW_ERROR_FORMAT=true``` environment variable. Ultimately, this should become the |
| 367 | +default. In order to get there, we need to ensure that the new error format is indeed an |
| 368 | +improvement over the existing format in practice. |
| 369 | + |
| 370 | +We also have not yet implemented the extended error format. This format will also be gated by its |
| 371 | +own flag while we explore and stabilize it. Because of the relative difference in maturity here, |
| 372 | +the default error message will be behind a flag for a cycle before it becomes default. The extended |
| 373 | +error format will be implemented and a follow-up RFC will be posted describing its design. This will |
| 374 | +start its stabilization period, after which time it too will be enabled. |
| 375 | + |
| 376 | +How do we measure the readability of error messages? This RFC details an educated guess as to what |
| 377 | +would improve the current state but shows no ways to measure success. |
| 378 | + |
| 379 | +Likewise, while some of us have been dogfooding these errors, we don't know what long-term use feels |
| 380 | +like. For example, after a time does the use of color feel excessive? We can always update the |
| 381 | +errors as we go, but it'd be helpful to catch it early if possible. |
| 382 | + |
| 383 | +# Unresolved questions |
| 384 | + |
| 385 | +There are a few unresolved questions: |
| 386 | +* Editors that rely on pattern-matching the compiler output will need to be updated. It's an open |
| 387 | +question how best to transition to using the new errors. There is on-going discussion of |
| 388 | +standardizing the JSON output, which could also be used. |
| 389 | +* Can additional error notes be shown without the "rainbow problem" where too many colors and too |
| 390 | +much boldness cause errors to become less readable? |
0 commit comments