Make Handler more thread-safe

[Zoxc]

The use of code_emitted to suppress extended explanations is not thread safe. I'm not sure why we keep the documentation for errors outside diagnostics.rs anyway. It would be better to add a teach method to DiagnosticsBuilder, so instead of:

if self.tcx.sess.teach(&err.get_code().unwrap()) {
    err.note("...");
}

we'd use err.teach("...")

cc @estebank

r? @michaelwoerister

[shepmaster]

Ping from review, @michaelwoerister !

[Zoxc]

The Session.one_time_diagnostics field should probably also be integrated in the Handler.

[bors]

☔ The latest upstream changes (presumably #49045) made this pull request unmergeable. Please resolve the merge conflicts.

[pietroalbini]

Ping from triage @michaelwoerister! This PR still needs your review.

[estebank]

The use of code_emitted to suppress extended explanations is not thread safe. I'm not sure why we keep the documentation for errors outside diagnostics.rs anyway. It would be better to add a teach method to DiagnosticsBuilder(...)

IIRC, I tried that first, but hit a wall with some of the module dependencies. I agree that it'd be a better API, but it'll probably require a larger refactoring (and I was trying to get the proof of concept for --teach out there to iterate quickly over it).

[Zoxc]

Actually the scheme used by one_time_diagnostics is thread-safe, so I applied it to taught_diagnostics (renamed from tracked_diagnostic_codes) too.

Neither is compatible with incremental compilation though. @michaelwoerister Do we have a tracking issue for incremental compilation?

[rust-highfive]

Your PR failed on Travis (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.

[00:00:47] configure: rust.quiet-tests     := True
---
[00:49:42] ..............................................................................i.....................
[00:49:48] .....................i.......................................................................F....F.
[00:49:52] F.F.F.F.............................................................................................
---
[00:50:31] i..........................................................................i........................
---
[00:50:54] 4 LL | const CON : Box<i32> = box 0; //~ ERROR E0010
[00:50:54] 5    |                        ^^^^^ allocation not allowed in constants
[00:50:54] -    |
[00:50:54] -    = note: The value of statics and constants must be known at compile time, and they live for the entire lifetime of a program. Creating a boxed value allocates memory on the heap at runtime, and therefore cannot be done at compile time.
---
[00:50:54] /checkout/src/test/ui/update-references.sh '/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui' 'error-codes/E0010-teach.rs'
[00:50:54]
[00:50:54] error: 1 errors occurred comparing output.
[00:50:54] status: exit code: 101
[00:50:54] command: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "/checkout/src/test/ui/error-co,"rendered":"error[E0010]: allocations are not allowed in constants\n  --> /checkout/src/test/ui/error-codes/E0010-teach.rs:16:24\n   |\nLL | const CON : Box<i32> = box 0; //~ ERROR E0010\n   |                        ^^^^^ allocation not allowed in constants\n\n"}
[00:50:54] {"message":"aborting due to previous error","code":null,"level":"error","spans":[],"children":[],"rendered":"error: aborting due to previous error\n\n"}
[00:50:54] {"message":"For more information about this error, try `rustc --explain E0010`.","code":null,"level":"","spans":[],"children":[],"rendered":"For more information about this error, try `rustc --explain E0010`.\n"}
---
[00:50:54] 4 LL |         Thing { x, y, z } => {}
[00:50:54] 5    |                       ^ struct `Thing` does not have this field
[00:50:54] -    |
[00:50:54] -    = note: This error indicates that a struct pattern attempted to extract a non-existent field from a struct. Struct fields are identified by the name used before the colon : so struct patterns should resemble the declaration of the struct type being matched.
[00:50:54] -
[00:50:54] -            If you are using shorthand field patterns but want to refer to the struct field by a different name, you shouldcause the compiler checks that the range is non-empty at compile-time, and is unable to evaluate arbitrary comparison functions. If you want to capture values of an orderable type between two end-points, you can use a guard.
---
[00:50:54] /checkout/src/test/ui/update-references.sh '/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui' 'error-codes/E0029-teach.rs'
[00:50:54]
[00:50:54] error: 1 errors occurred comparing output.
[00:50:54] status: exit code: 101
[00:50:54] command: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "/checkout/src/test/ui/error-codes/E0029-teach.rs" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui" "--target=x86_64-unknown-linux-gnu" "--error-format" "json" "-Zui-testing" "-C" "prefer-dynamic" "-o" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/error-codes/E0029-teach.stage2-x86_64-unknown-linux-gnu" "-Crpath" "-O" "-Zmiri" "-Zunstable-options" "-Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "-Z" "teach" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/error-codes/E0029-teach.stage2-x86_64-unknown-linux-gnu.aux" "-A" "unused"
---
[00:50:54] {"message":"only char and numeric types are allowed in range patterns","code":{"code":"E0029","explanation":"\nIn a match expression, only numbers and characters can be matched against a\nrange. This is because the compiler checks that the range is non-empty at\ncompile-time, and is unable to evaluate arbitrary comparison functions. If you\nwant to capture values of an orderable type between two end-points, you can use\na guard.\n\n```compile_fail,E0029\nlet string = \"salutations !\";\n\n// The ordering relation for strings can't be evaluated at compile time,\n// so this doesn't work:\nmatch string {\n    \"hello\" ... \"world\" => {}\n    _ => {}\n}\n\n// This is a more general version, using a guard:\nmatch string {\n    s if s >= \"hello\" && s <= \"world\" => {}\n    _ => {}\n}\n```\n"},"level":"error","spans":[{"file_name":"/checkout/src/test/ui/error-codes/E0029-teach.rs","byte_start":550,"byte_end":569,"line_start":17,"line_end":17,"column_start":9,"column_end":28,"is_primary":true,"text":[{"text":"        \"hello\" ... \"world\" => {}","highlight_start":9,"highlight_end":28}],"label":"ranges require char or numeric types","suggested_replacement":null,"expansion":null}],"children":[{"message":"start type: &'static str","code":null,"level":"note","spans":[],"children":[],"rendered":null},{"message":"end type: &'static str","code":null,"level":"note","spans":[],"children":[],"rendered":null}],"rendered":"error[E0029]: only char and numeric types are allowed in range patterns\n  --> /checkout/src/test/ui/error-codes/E0029-teach.rs:17:9\n   |\nLL |         \"hello\" ... \"world\" => {}\n   |         ^^^^^^^^^^^^^^^^^^^ ranges require char or numeric types\n   |\n   = note: start type: &'static str\n   = note: end type: &'static str\n\n"}
[00:50:54] {"message":"aborting due to previous error","code":null,"level":"error","spans":[],"children":[],"rendered":"error: aborting due to previous error\n\n"}
[00:50:54] {"message":"For more information about this error, try `rustc --explain E0029`.","code":null,"level":"","spans":[],"children":[],"rendered":"For more information about this error, try `rustc --explain E0029`.\n"}
---
[00:50:54] 4 LL |         1000 ... 5 => {}
[00:50:54] 5    |         ^^^^ lower bound larger than upper bound
[00:50:54] -    |
[00:50:54] -    = note: When matching against a range, the compiler verifies that the range is non-empty. Range patterns include both end-points, so this is equivalent to requiring the start of the range to be less than or equal to the end of the range.
---
[00:50:54] /checkout/src/test/ui/update-references.sh '/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui' 'error-codes/E0030-teach.rs'
[00:50:54]
[00:50:54] error: 1 errors occurred comparing output.
[00:50:54] status: exit code: 101
[00:50:54] command: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "/checkout/src/test/ui/error-codes/E0030-teach.rs" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui" "--target=x86_64-unknown-linux-gnu" "--error-format" "json" "-Zui-testing" "-C" "prefer-dynamic" "-o" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/error-codes/E0030-teach.stage2-x86_64-unknown-linux-gnu" "-Crpath" "-O" "-Zmiri" "-Zunstable-options" "-Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "-Z" "teach" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/error-codes/E0030-teach.stage2-x86_64-unknown-linux-gnu.aux" "-A" "unused"
---
[00:50:54] {"message":"lower range bound must be less than or equal to upper","code":{"code":"E0030","explanation":"\nWhen matching against a range, the compiler verifies that the range is\nnon-empty.  Range patterns include both end-points, so this is equivalent to\nrequiring the start of the range to be less than or equal to the end of the\nrange.\n\nFor example:\n\n```compile_fail\nmatch 5u32 {\n    // This range is ok, albeit pointless.\n    1 ... 1 => {}\n    // This range is empty, and the compiler can tell.\n    1000 ... 5 => {}\n}\n```\n"},"level":"error","spans":[{"file_name":"/checkout/src/test/ui/error-codes/E0030-teach.rs","byte_start":532,"byte_end":536,"line_start":15,"line_end":15,"column_start":9,"column_end":13,"is_primary":true,"text":[{"text":"        1000 ... 5 => {}","highlight_start":9,"highlight_end":13}],"label":"lower bound larger than upper bound","suggested_replacement":null,"expansion":null}],"children":[],"rendered":"error[E0030]: lower range bound must be less than or equal to upper\n  --> /checkout/src/test/ui/error-codes/E0030-teach.rs:15:9\n   |\nLL |         1000 ... 5 => {}\n   |         ^^^^ lower bound larger than upper bound\n\n"}
[00:50:54] {"message":"aborting due to previous error","code":null,"level":"error","spans":[],"children":[],"rendered":"error: aborting due to previous error\n\n"}
[00:50:54] {"message":"For more information about this error, try `rustc --explain E0030`.","code":null,"level":"","spans":[],"children":[],"rendered":"For more information about this error, try `rustc --explain E0030`.\n"}
---
[00:50:54] 18 LL |     let &invalid = trait_obj;
[00:50:54] 19    |         ^^^^^^^^ type `&SomeTrait` cannot be dereferenced
[00:50:54] -    |
[00:50:54] -    = note: This error indicates that a pointer to a trait type cannot be implicitly dereferenced by a pattern. Every trait defines a type, but because the size of trait implementors isn't fixed, this type has no compile-time size. Therefore, all accesses to trait types must be through pointers. If you encounter this error you should try to avoid dereferencing the pointer.
[00:50:54] -
[00:50:54] -            You can read more about trait objects in the Trait Objects section of the Reference: https://doc.rust-lang.org/reference/types.html#trait-objects
---
[00:50:54] /checkout/src/test/ui/update-references.sh '/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui' 'error-codes/E0033-teach.rs'
[00:50:54]
[00:50:54] error: 1 errors occurred comparing output.
[00:50:54] status: exit code: 101
[00:50:54] command: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "/checkout/src/test/ui/error-codes/E0033-teach.rs" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui" "--target=x86_64-unknown-linux-gnu" "--error-format" "json" "-Zui-testing" "-C" "prefer-dynamic" "-o" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/error-codes/E0033-teach.stage2-x86_64-unknown-linux-gnu" "-Crpath" "-O" "-Zmiri" "-Zunstable-options" "-Lnative=/checkout/obj/build/x86_64-unknown-l":"E0038","explanation":"\nTrait objects like `Box<Trait>` can only be constructed when certain\nrequirements are satisfied by the trait in question.\n\nTrait objects are a form of dynamic dispatch and use a dynamically sized type\nfor the inner type. So, for a given trait `Trait`, when `Trait` is treated as a\ntype, as in `Box<Trait>`, the inner type is 'unsized'. In such cases the boxed\npointer is a 'fat pointer' that contains an extra pointer to a table of methods\n(among other things) for dynamic dispatch. This design mandates some\nrestrictions on the types of traits that are allowed to be used in trait\nobjects, which are collectively termed as 'object safety' rules.\n\nAttempting to create a trait object for a non object-safe trait will trigger\nthis error.\n\nThere are various rules:\n\n### The trait cannot require `Self: Sized`\n\nWhen `Trait` is treated as a type, the type does not implement the special\n`Sized` trait, because the type does not have a known size at compile time and\ncan only be accessed behind a pointer. Thus, if we have a trait like the\nfollowing:\n\n```\ntrait Foo where Self: Sized {\n\n}\n```\n\nWe cannot create an object of type `Box<Foo>` or `&Foo` since in this case\n`Self` would not be `Sized`.\n\nGenerally, `Self : Sized` is used to indicate that the trait should not be used\nas a trait object. If the trait comes from your own crate, consider removing\nthis restriction.\n\n### Method references the `Self` type in its arguments or return type\n\nThis happens when a trait has a method like the following:\n\n```\ntrait Trait {\n    fn foo(&self) -> Self;\n}\n\nimpl Trait for String {\n    fn foo(&self) -> Self {\n        \"hi\".to_owned()\n    }\n}\n\nimpl Trait for u8 {\n    fn foo(&self) -> Self {\n        1\n    }\n}\n```\n\n(Note that `&self` and `&mut self` are okay, it's additional `Self` types which\ncause this problem.)\n\nIn such a case, the compiler cannot predict the return type of `foo()` in a\nsituation like the following:\n\n```compile_fail\ntrait Trait {\n    fn foo(&self) -> Self;\n}\n\nfn call_foo(x: Box<Trait>) {\n    let y = x.foo(); // What type is y?\n    // ...\n}\n```\n\nIf only some methods aren't object-safe, you can add a `where Self: Sized` bound\non them to mark them as explicitly unavailable to trait objects. The\nfunctionality will still be available to all other implementers, including\n`Box<Trait>` which is itself sized (assuming you `impl Trait for Box<Trait>`).\n\n```\ntrait Trait {\n    fn foo(&self) -> Self where Self: Sized;\n    // more functions\n}\n```\n\nNow, `foo()` can no longer be called on a trait object, but you will now be\nallowed to make a trait object, and that will be able to call any object-safe\nmethods. With such a bound, one can still call `foo()` on types implementing\nthat trait that aren't behind trait objects.\n\n### Method has generic type parameters\n\nAs mentioned before, trait objects contain pointers to method tables. So, if we\nhave:\n\n```\ntrait Trait {\n    fn foo(&self);\n}\n\nimpl Trait for String {\n    fn foo(&self) {\n        // implementation 1\n    }\n}\n\nimpl Trait for u8 {\n    fn foo(&self) {\n        // implementation 2\n    }\n}\n// ...\n```\n\nAt compile time each implementation of `Trait` will produce a table containing\nthe various methods (and other items) related to the implementation.\n\nThis works fine, but when the method gains generic parameters, we can have a\nproblem.\n\nUsually, generic parameters get _monomorphized_. For example, if I have\n\n```\nfn foo<T>(x: T) {\n    // ...\n}\n```\n\nThe machine code for `foo::<u8>()`, `foo::<bool>()`, `foo::<String>()`, or any\nother type substitution is different. Hence the compiler generates the\nimplementation on-demand. If you call `foo()` with a `bool` parameter, the\ncompiler will only generate code for `foo::<bool>()`. When we have additional\ntype parameters, the number of monomorphized implementations the compiler\ngenerates does not grow drastically, since the compiler will only generate an\nimplementation if the function is called with unparametrized substitutions\n(i.e., substitutions where none of the substituted types are themselves\nparametrized).\n\nHowever, with trait objects we have to make a table containing _every_ object\nthat implements the trait. Now, if it has type parameters, we need to add\nimplementations for every type that implements the trait, and there could\ntheoretically be an infinite number of types.\n\nFor example, with:\n\n```\ntrait Trait {\n    fn foo<T>(&self, on: T);\n    // more methods\n}\n\nimpl Trait for String {\n    fn foo<T>(&self, on: T) {\n        // implementation 1\n    }\n}\n\nimpl Trait for u8 {\n    fn foo<T>(&self, on: T) {\n        // implementation 2\n    }\n}\n\n// 8 more implementations\n```\n\nNow, if we have the following code:\n\n```compile_fail,E0038\n# trait Trait { fn foo<T>(&self, on: T); }\n# impl Trait for String { fn foo<T>(&self, on: T) {} }\n# impl Trait for u8 { fn foo<T>(&self, on: T) {} }\n# impl Trait for bool { fn foo<T>(&self, on: T) {} }\n# // etc.\nfn call_foo(thing: Box<Trait>) {\n    thing.foo(true); // this could be any one of the 8 types above\n    thing.foo(1);\n    thing.foo(\"hello\");\n}\n```\n\nWe don't just need to create a table of all implementations of all methods of\n`Trait`, we need to create such a table, for each different type fed to\n`foo()`. In this case this turns out to be (10 types implementing `Trait`)*(3\ntypes being fed to `foo()`) = 30 implementations!\n\nWith real world traits these numbers can grow drastically.\n\nTo fix this, it is suggested to use a `where Self: Sized` bound similar to the\nfix for the sub-error above if you do not intend to call the method with type\nparameters:\n\n```\ntrait Trait {\n    fn foo<T>(&self, on: T) where Self: Sized;\n    // more methods\n}\n```\n\nIf this is not an option, consider replacing the type parameter with another\ntrait object (e.g. if `T: OtherTrait`, use `on: Box<OtherTrait>`). If the number\nof types you intend to feed to this method is limited, consider manually listing\nout the methods of different types.\n\n### Method has no receiver\n\nMethods that do not take a `self` parameter can't be called since there won't be\na way to get a pointer to the method table for them.\n\n```\ntrait Foo {\n    fn foo() -> u8;\n}\n```\n\nThis could be called as `<Foo as Foo>::foo()`, which would not be able to pick\nan implementation.\n\nAdding a `Self: Sized` bound to these methods will generally make this compile.\n\n```\ntrait Foo {\n    fn foo() -> u8 where Self: Sized;\n}\n```\n\ "--color" "always"
[00:50:54] expected success, got: exit code: 101
[00:50:54]
[00:50:54]
[00:50:54] failed to run: /checkout/obj/build/bootstrap/debug/bootstrap test
[00:50:54] Build completed unsuccessfully in 0:02:30
[00:50:54] Makefile:58: recipe for target 'check' failed
[00:50:54] make: *** [check] Error 1

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

[michaelwoerister]

@Zoxc We can just add incr. comp. related tasks to #48685, I'd say.

[bors]

☔ The latest upstream changes (presumably #49558) made this pull request unmergeable. Please resolve the merge conflicts.

[pietroalbini]

Ping from triage @michaelwoerister! What's the status of this PR?

[bors]

☔ The latest upstream changes (presumably #49882) made this pull request unmergeable. Please resolve the merge conflicts.

[rust-highfive]

Your PR failed on Travis (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.

[00:00:46] configure: rust.quiet-tests     := True
---
[00:33:14] 189 |         emitter,
[00:33:14]     |         ^^^^^^^ expected trait `errors::emitter::Emitter + rustc_data_structures::sync::Send`, found trait `errors::emitter::Emitter`
[00:33:14]     |
[00:33:14]     = note: expected type `std::boxed::Box<errors::emitter::Emitter + rustc_data_structures::sync::Send + 'static>`
[00:33:14]                found type `std::boxed::Box<errors::emitter::Emitter>`
---
[00:33:20] Makefile:28: recipe for target 'all' failed
[00:33:20] make: *** [all] Error 1
---
$ ls -lat $HOME/Library/Logs/DiagnosticReports/
ls: cannot access /home/travis/Library/Logs/DiagnosticReports/: No such file or directory
travis_time:end:090e2dc4:start=1523974141721054188,finish=1523974141735218647,duration=14164459
travis_fold:end:after_failure.2
travis_fold:start:after_failure.3
travis_time:start:0840ff99
$ find $HOME/Library/Logs/DiagnosticReports -type f -name '*.crash' -not -name '*.stage2-*.crash' -not -name 'com.apple.CoreSimulator.CoreSimulatorService-*.crash' -exec printf travis_fold":start:crashlog\n\033[31;1m%s\033[0m\n" {} \; -exec head -750 {} \; -exec echo travis_fold":"end:crashlog \; || true
find: `/home/travis/Library/Logs/DiagnosticReports': No such file or directory
travis_time:end:0840ff99:start=1523974141745405845,finish=1523974141755642511,duration=10236666
travis_fold:end:after_failure.3
travis_fold:start:after_failure.4
travis_time:start:2ae47e9f
$ dmesg | grep -i kill
[   10.658621] init: failsafe main process (1096) killed by TERM signal

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

[michaelwoerister]

@bors r+

[bors]

📌 Commit e5fc06d has been approved by michaelwoerister

[bors]

⌛ Testing commit e5fc06d with merge f4bb956...

[bors]

☀️ Test successful - status-appveyor, status-travis
Approved by: michaelwoerister
Pushing f4bb956 to master...