summary refs log tree commit diff
path: root/compiler/rustc_borrowck/src/diagnostics
AgeCommit message (Collapse)AuthorLines
2022-03-30Auto merge of #95466 - Dylan-DPC:rollup-g7ddr8y, r=Dylan-DPCbors-2/+2
Rollup of 5 pull requests Successful merges: - #95294 (Document Linux kernel handoff in std::io::copy and std::fs::copy) - #95443 (Clarify how `src/tools/x` searches for python) - #95452 (fix since field version for termination stabilization) - #95460 (Spellchecking compiler code) - #95461 (Spellchecking some comments) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
2022-03-30Rollup merge of #95461 - nyurik:spelling, r=lcnrDylan DPC-2/+2
Spellchecking some comments This PR attempts to clean up some minor spelling mistakes in comments
2022-03-30Spellchecking some commentsYuri Astrakhan-2/+2
This PR attempts to clean up some minor spelling mistakes in comments
2022-03-30Auto merge of #94081 - oli-obk:lazy_tait_take_two, r=nikomatsakisbors-21/+91
Lazy type-alias-impl-trait take two ### user visible change 1: RPIT inference from recursive call sites Lazy TAIT has an insta-stable change. The following snippet now compiles, because opaque types can now have their hidden type set from wherever the opaque type is mentioned. ```rust fn bar(b: bool) -> impl std::fmt::Debug { if b { return 42 } let x: u32 = bar(false); // this errors on stable 99 } ``` The return type of `bar` stays opaque, you can't do `bar(false) + 42`, you need to actually mention the hidden type. ### user visible change 2: divergence between RPIT and TAIT in return statements Note that `return` statements and the trailing return expression are special with RPIT (but not TAIT). So ```rust #![feature(type_alias_impl_trait)] type Foo = impl std::fmt::Debug; fn foo(b: bool) -> Foo { if b { return vec![42]; } std::iter::empty().collect() //~ ERROR `Foo` cannot be built from an iterator } fn bar(b: bool) -> impl std::fmt::Debug { if b { return vec![42] } std::iter::empty().collect() // Works, magic (accidentally stabilized, not intended) } ``` But when we are working with the return value of a recursive call, the behavior of RPIT and TAIT is the same: ```rust type Foo = impl std::fmt::Debug; fn foo(b: bool) -> Foo { if b { return vec![]; } let mut x = foo(false); x = std::iter::empty().collect(); //~ ERROR `Foo` cannot be built from an iterator vec![] } fn bar(b: bool) -> impl std::fmt::Debug { if b { return vec![]; } let mut x = bar(false); x = std::iter::empty().collect(); //~ ERROR `impl Debug` cannot be built from an iterator vec![] } ``` ### user visible change 3: TAIT does not merge types across branches In contrast to RPIT, TAIT does not merge types across branches, so the following does not compile. ```rust type Foo = impl std::fmt::Debug; fn foo(b: bool) -> Foo { if b { vec![42_i32] } else { std::iter::empty().collect() //~^ ERROR `Foo` cannot be built from an iterator over elements of type `_` } } ``` It is easy to support, but we should make an explicit decision to include the additional complexity in the implementation (it's not much, see a721052457cf513487fb4266e3ade65c29b272d2 which needs to be reverted to enable this). ### PR formalities previous attempt: #92007 This PR also includes #92306 and #93783, as they were reverted along with #92007 in #93893 fixes #93411 fixes #88236 fixes #89312 fixes #87340 fixes #86800 fixes #86719 fixes #84073 fixes #83919 fixes #82139 fixes #77987 fixes #74282 fixes #67830 fixes #62742 fixes #54895
2022-03-29Rollup merge of #95415 - notriddle:notriddle/issue-82081, r=Dylan-DPCDylan DPC-2/+7
diagnostics: regression test for HashMap iter_mut suggestion Closes #82081
2022-03-28Suggest function borrow ignoring needs_noteMichael Goulet-2/+6
`needs_note` is false if we've already suggested why the type is Copy... but that has nothing to do with the diagnostic.
2022-03-28drive-by: move Copy bound suggestion to its own functionMichael Goulet-60/+65
2022-03-28Add suggestion to borrow opaque Fn and FnMut instead of moveMichael Goulet-13/+90
2022-03-28diagnostics: do not suggest `map.iter_mut()()`Michael Howell-2/+7
2022-03-28Revert "Auto merge of #93893 - oli-obk:sad_revert, r=oli-obk"Oli Scherer-21/+91
This reverts commit 6499c5e7fc173a3f55b7a3bd1e6a50e9edef782d, reversing changes made to 78450d2d602b06d9b94349aaf8cece1a4acaf3a8.
2022-03-19diagnostics: do not give Option::as_ref suggestion for complex matchMichael Howell-5/+19
Fixes #82528
2022-03-17Rollup merge of #94698 - WaffleLapkin:simplify-copy-suggestions, r=estebankDylan DPC-80/+55
Remove redundant code from copy-suggestions Follow up to #94375, just remove some code that is not necessary anymore. This may make the perf of such suggestions a little bit worse, but I don't think this is significant. r? `@estebank`
2022-03-11Improve `AdtDef` interning.Nicholas Nethercote-9/+9
This commit makes `AdtDef` use `Interned`. Much the commit is tedious changes to introduce getter functions. The interesting changes are in `compiler/rustc_middle/src/ty/adt.rs`.
2022-03-10Auto merge of #94737 - lcnr:pass-stuff-by-value, r=davidtwcobors-7/+7
add `#[rustc_pass_by_value]` to more types the only interesting changes here should be to `TransitiveRelation`, but I believe to be highly unlikely that we will ever use a non `Copy` type with this type.
2022-03-09Auto merge of #94515 - estebank:tweak-move-error, r=davidtwcobors-183/+195
Tweak move error Point at method definition that causes type to be consumed. Fix #94056.
2022-03-08add `#[rustc_pass_by_value]` to more typeslcnr-7/+7
2022-03-07Remove redundant code from copy-suggestionsMaybe Waffle-80/+55
2022-03-06Erase regions when checking for missing Copy predicatesMichael Goulet-2/+10
2022-03-03Tweak move errorEsteban Kuber-183/+195
Point at method definition that causes type to be consumed. Fix #94056.
2022-03-03Rollup merge of #94375 - WaffleLapkin:copy-suggestion, r=estebankDylan DPC-1/+62
Adt copy suggestions Previously we've only suggested adding `Copy` bounds when the type being moved/copied is a type parameter (generic). With this PR we also suggest adding bounds when a type - Can be copy - All predicates that need to be satisfied for that are based on type params i.e. we will suggest `T: Copy` for `Option<T>`, but won't suggest anything for `Option<String>`. An example: ```rust fn duplicate<T>(t: Option<T>) -> (Option<T>, Option<T>) { (t, t) } ``` New error (current compiler doesn't provide `help`:): ```text error[E0382]: use of moved value: `t` --> t.rs:2:9 | 1 | fn duplicate<T>(t: Option<T>) -> (Option<T>, Option<T>) { | - move occurs because `t` has type `Option<T>`, which does not implement the `Copy` trait 2 | (t, t) | - ^ value used here after move | | | value moved here | help: consider restricting type parameter `T` | 1 | fn duplicate<T: Copy>(t: Option<T>) -> (Option<T>, Option<T>) { | ++++++ ``` Fixes #93623 r? ``````````@estebank`````````` ``````````@rustbot`````````` label +A-diagnostics +A-suggestion-diagnostics +C-enhancement ---- I'm not at all sure if this is the right implementation for this kind of suggestion, but it seems to work :')
2022-03-02rename ErrorReported -> ErrorGuaranteedmark-26/+26
2022-03-01Suggest adding `Copy` bound when Adt is moved outMaybe Waffle-1/+62
Previously we've only suggested adding `Copy` bounds when the type being moved/copied is a type parameter (generic). With this commit we also suggest adding bounds when a type - Can be copy - All predicates that need to be satisfied for that are based on type params i.e. we will suggest `T: Copy` for `Option<T>`, but won't suggest anything for `Option<String>`. Future work: it would be nice to also suggest adding `.clone()` calls
2022-02-273 - Make more use of let_chainsCaio-43/+32
Continuation of #94376. cc #53667
2022-02-25Auto merge of #93368 - eddyb:diagbld-guarantee, r=estebankbors-69/+74
rustc_errors: let `DiagnosticBuilder::emit` return a "guarantee of emission". That is, `DiagnosticBuilder` is now generic over the return type of `.emit()`, so we'll now have: * `DiagnosticBuilder<ErrorReported>` for error (incl. fatal/bug) diagnostics * can only be created via a `const L: Level`-generic constructor, that limits allowed variants via a `where` clause, so not even `rustc_errors` can accidentally bypass this limitation * asserts `diagnostic.is_error()` on emission, just in case the construction restriction was bypassed (e.g. by replacing the whole `Diagnostic` inside `DiagnosticBuilder`) * `.emit()` returns `ErrorReported`, as a "proof" token that `.emit()` was called (though note that this isn't a real guarantee until after completing the work on #69426) * `DiagnosticBuilder<()>` for everything else (warnings, notes, etc.) * can also be obtained from other `DiagnosticBuilder`s by calling `.forget_guarantee()` This PR is a companion to other ongoing work, namely: * #69426 and it's ongoing implementation: #93222 the API changes in this PR are needed to get statically-checked "only errors produce `ErrorReported` from `.emit()`", but doesn't itself provide any really strong guarantees without those other `ErrorReported` changes * #93244 would make the choices of API changes (esp. naming) in this PR fit better overall In order to be able to let `.emit()` return anything trustable, several changes had to be made: * `Diagnostic`'s `level` field is now private to `rustc_errors`, to disallow arbitrary "downgrade"s from "some kind of error" to "warning" (or anything else that doesn't cause compilation to fail) * it's still possible to replace the whole `Diagnostic` inside the `DiagnosticBuilder`, sadly, that's harder to fix, but it's unlikely enough that we can paper over it with asserts on `.emit()` * `.cancel()` now consumes `DiagnosticBuilder`, preventing `.emit()` calls on a cancelled diagnostic * it's also now done internally, through `DiagnosticBuilder`-private state, instead of having a `Level::Cancelled` variant that can be read (or worse, written) by the user * this removes a hazard of calling `.cancel()` on an error then continuing to attach details to it, and even expect to be able to `.emit()` it * warnings were switched to *only* `can_emit_warnings` on emission (instead of pre-cancelling early) * `struct_dummy` was removed (as it relied on a pre-`Cancelled` `Diagnostic`) * since `.emit()` doesn't consume the `DiagnosticBuilder` <sub>(I tried and gave up, it's much more work than this PR)</sub>, we have to make `.emit()` idempotent wrt the guarantees it returns * thankfully, `err.emit(); err.emit();` can return `ErrorReported` both times, as the second `.emit()` call has no side-effects *only* because the first one did do the appropriate emission * `&mut Diagnostic` is now used in a lot of function signatures, which used to take `&mut DiagnosticBuilder` (in the interest of not having to make those functions generic) * the APIs were already mostly identical, allowing for low-effort porting to this new setup * only some of the suggestion methods needed some rework, to have the extra `DiagnosticBuilder` functionality on the `Diagnostic` methods themselves (that change is also present in #93259) * `.emit()`/`.cancel()` aren't available, but IMO calling them from an "error decorator/annotator" function isn't a good practice, and can lead to strange behavior (from the caller's perspective) * `.downgrade_to_delayed_bug()` was added, letting you convert any `.is_error()` diagnostic into a `delay_span_bug` one (which works because in both cases the guarantees available are the same) This PR should ideally be reviewed commit-by-commit, since there is a lot of fallout in each. r? `@estebank` cc `@Manishearth` `@nikomatsakis` `@mark-i-m`
2022-02-24Auto merge of #94131 - Mark-Simulacrum:fmt-string, r=oli-obkbors-8/+4
Always format to internal String in FmtPrinter This avoids monomorphizing for different parameters, decreasing generic code instantiated downstream from rustc_middle -- locally seeing 7% unoptimized LLVM IR line wins on rustc_borrowck, for example. We likely can't/shouldn't get rid of the Result-ness on most functions, though some further cleanup avoiding fmt::Error where we now know it won't occur may be possible, though somewhat painful -- fmt::Write is a pretty annoying API to work with in practice when you're trying to use it infallibly.
2022-02-23rustc_errors: let `DiagnosticBuilder::emit` return a "guarantee of emission".Eduard-Mihai Burtescu-27/+49
2022-02-23Replace `&mut DiagnosticBuilder`, in signatures, with `&mut Diagnostic`.Eduard-Mihai Burtescu-45/+28
2022-02-21use `List<Ty<'tcx>>` for tupleslcnr-5/+4
2022-02-20Always format to internal String in FmtPrinterMark Rousskov-8/+4
This avoids monomorphizing for different parameters, decreasing generic code instantiated downstream from rustc_middle.
2022-02-20Rollup merge of #94146 - est31:let_else, r=cjgillotMatthias Krüger-35/+16
Adopt let else in more places Continuation of #89933, #91018, #91481, #93046, #93590, #94011. I have extended my clippy lint to also recognize tuple passing and match statements. The diff caused by fixing it is way above 1 thousand lines. Thus, I split it up into multiple pull requests to make reviewing easier. This is the biggest of these PRs and handles the changes outside of rustdoc, rustc_typeck, rustc_const_eval, rustc_trait_selection, which were handled in PRs #94139, #94142, #94143, #94144.
2022-02-19Adopt let else in more placesest31-35/+16
2022-02-19Rollup merge of #94006 - pierwill:upvar-field, r=nikomatsakisMatthias Krüger-6/+16
Use a `Field` in `ConstraintCategory::ClosureUpvar` As part of #90317, we do not want `HirId` to implement `Ord`, `PartialOrd`. This line of code has made that difficult https://github.com/rust-lang/rust/blob/1b27144afc77031ba9c05d86c06c64485589775a/compiler/rustc_borrowck/src/region_infer/mod.rs#L2184 since it sorts a [`ConstraintCategory::ClosureUpvar(HirId)`](https://doc.rust-lang.org/nightly/nightly-rustc/rustc_middle/mir/enum.ConstraintCategory.html#variant.ClosureUpvar). This PR makes that variant take a [`Field`](https://doc.rust-lang.org/nightly/nightly-rustc/rustc_middle/mir/struct.Field.html) instead. r? `@nikomatsakis`
2022-02-17Rollup merge of #94011 - est31:let_else, r=lcnrMatthias Krüger-12/+7
Even more let_else adoptions Continuation of #89933, #91018, #91481, #93046, #93590.
2022-02-16Use a `Field` in `ConstraintCategory::ClosureUpvar`pierwill-6/+16
2022-02-16Adopt let_else in even more placesest31-12/+7
2022-02-15Overhaul `RegionKind` and `Region`.Nicholas Nethercote-29/+27
Specifically, change `Region` from this: ``` pub type Region<'tcx> = &'tcx RegionKind; ``` to this: ``` pub struct Region<'tcx>(&'tcx Interned<RegionKind>); ``` This now matches `Ty` and `Predicate` more closely. Things to note - Regions have always been interned, but we haven't been using pointer-based `Eq` and `Hash`. This is now happening. - I chose to impl `Deref` for `Region` because it makes pattern matching a lot nicer, and `Region` can be viewed as just a smart wrapper for `RegionKind`. - Various methods are moved from `RegionKind` to `Region`. - There is a lot of tedious sigil changes. - A couple of types like `HighlightBuilder`, `RegionHighlightMode` now have a `'tcx` lifetime because they hold a `Ty<'tcx>`, so they can call `mk_region`. - A couple of test outputs change slightly, I'm not sure why, but the new outputs are a little better.
2022-02-15Overhaul `TyS` and `Ty`.Nicholas Nethercote-20/+20
Specifically, change `Ty` from this: ``` pub type Ty<'tcx> = &'tcx TyS<'tcx>; ``` to this ``` pub struct Ty<'tcx>(Interned<'tcx, TyS<'tcx>>); ``` There are two benefits to this. - It's now a first class type, so we can define methods on it. This means we can move a lot of methods away from `TyS`, leaving `TyS` as a barely-used type, which is appropriate given that it's not meant to be used directly. - The uniqueness requirement is now explicit, via the `Interned` type. E.g. the pointer-based `Eq` and `Hash` comes from `Interned`, rather than via `TyS`, which wasn't obvious at all. Much of this commit is boring churn. The interesting changes are in these files: - compiler/rustc_middle/src/arena.rs - compiler/rustc_middle/src/mir/visit.rs - compiler/rustc_middle/src/ty/context.rs - compiler/rustc_middle/src/ty/mod.rs Specifically: - Most mentions of `TyS` are removed. It's very much a dumb struct now; `Ty` has all the smarts. - `TyS` now has `crate` visibility instead of `pub`. - `TyS::make_for_test` is removed in favour of the static `BOOL_TY`, which just works better with the new structure. - The `Eq`/`Ord`/`Hash` impls are removed from `TyS`. `Interned`s impls of `Eq`/`Hash` now suffice. `Ord` is now partly on `Interned` (pointer-based, for the `Equal` case) and partly on `TyS` (contents-based, for the other cases). - There are many tedious sigil adjustments, i.e. adding or removing `*` or `&`. They seem to be unavoidable.
2022-02-12Handle Fn family trait call errrorDeadbeef-2/+2
2022-02-12Rebased and improved errorsDeadbeef-1/+3
2022-02-12Improve error messages even moreDeadbeef-107/+37
2022-02-11rework borrowck errors so that it's harder to not set taintedMichael Goulet-12/+2
2022-02-11implement tainted_by_errors in mir borrowckMichael Goulet-41/+37
2022-02-11Auto merge of #93893 - oli-obk:sad_revert, r=oli-obkbors-87/+21
Revert lazy TAIT PR Revert https://github.com/rust-lang/rust/pull/92306 (sorry `@Aaron1011,` will include your changes in the fix PR) Revert https://github.com/rust-lang/rust/pull/93783 Revert https://github.com/rust-lang/rust/pull/92007 fixes https://github.com/rust-lang/rust/issues/93788 fixes https://github.com/rust-lang/rust/issues/93794 fixes https://github.com/rust-lang/rust/issues/93821 fixes https://github.com/rust-lang/rust/issues/93831 fixes https://github.com/rust-lang/rust/issues/93841
2022-02-11Revert "Auto merge of #92306 - Aaron1011:opaque-type-op, r=oli-obk"Oli Scherer-87/+21
This reverts commit 1f0a96862ac9d4c6ca3e4bb500c8b9eac4d83049, reversing changes made to bf242bb1199e25ca2274df5c4114e0c9436b74e9.
2022-02-10Remove further usage of `&hir::Map`Frank Steffahn-2/+2
2022-02-08Improve opaque type higher-ranked region error message under NLLAaron Hill-21/+87
Currently, any higher-ranked region errors involving opaque types fall back to a generic "higher-ranked subtype error" message when run under NLL. This PR adds better error message handling for this case, giving us the same kinds of error messages that we currently get without NLL: ``` error: implementation of `MyTrait` is not general enough --> $DIR/opaque-hrtb.rs:12:13 | LL | fn foo() -> impl for<'a> MyTrait<&'a str> { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ implementation of `MyTrait` is not general enough | = note: `impl MyTrait<&'2 str>` must implement `MyTrait<&'1 str>`, for any lifetime `'1`... = note: ...but it actually implements `MyTrait<&'2 str>`, for some specific lifetime `'2` error: aborting due to previous error ``` To accomplish this, several different refactoring needed to be made: * We now have a dedicated `InstantiateOpaqueType` struct which implements `TypeOp`. This is used to invoke `instantiate_opaque_types` during MIR type checking. * `TypeOp` is refactored to pass around a `MirBorrowckCtxt`, which is needed to report opaque type region errors. * We no longer assume that all `TypeOp`s correspond to canonicalized queries. This allows us to properly handle opaque type instantiation (which does not occur in a query) as a `TypeOp`. A new `ErrorInfo` associated type is used to determine what additional information is used during higher-ranked region error handling. * The body of `try_extract_error_from_fulfill_cx` has been moved out to a new function `try_extract_error_from_region_constraints`. This allows us to re-use the same error reporting code between canonicalized queries (which can extract region constraints directly from a fresh `InferCtxt`) and opaque type handling (which needs to take region constraints from the pre-existing `InferCtxt` that we use throughout MIR borrow checking).
2022-02-02Rollup merge of #93590 - est31:let_else, r=lcnrMatthias Krüger-72/+65
More let_else adoptions Continuation of #89933, #91018, #91481, #93046.
2022-02-02Rollup merge of #93221 - alyssaverkade:fix-93093, r=wesleywiserMatthias Krüger-6/+26
[borrowck] Fix help on mutating &self in async fns Previously, when rustc was provided an async function that tried to mutate through a shared reference to an implicit self (as shown in the ui test), rustc would suggest modifying the parameter signature to `&mut` + the fully qualified name of the ty (in the case of the repro `S`). If a user modified their code to match the suggestion, the compiler would not accept it. This commit modifies the suggestion so that when rustc is provided the ui test that is also attached in this commit, it suggests (correctly) `&mut self`. We try to be careful about distinguishing between implicit and explicit self annotations, since the latter seem to be handled correctly already. This is my first PR here so I'm pretty sure I probably missed something/could use better terminology. I also didn't try to make the match exhaustive since implicit self is the only real special case that I need to handle (that I'm aware of), and I'm pretty sure there's a cleaner way to do this so any advice would be greatly appreciated! (I'm also not terribly confident about how I wrote the ui tests) here is your cc as requested `@compiler-errors` This is an attempt to fix #93093
2022-02-02More let_else adoptionsest31-72/+65
2022-01-22[borrowck] Fix help on mutating &self in async fnsAlyssa Verkade-6/+26
Previously, when rustc was provided an async function that tried to mutate through a shared reference to an implicit self (as shown in the ui test), rustc would suggest modifying the parameter signature to `&mut` + the fully qualified name of the ty (in the case of the repro `S`). If a user modified their code to match the suggestion, the compiler would not accept it. This commit modifies the suggestion so that when rustc is provided the ui test that is also attached in this commit, it suggests (correctly) `&mut self`. We try to be careful about distinguishing between implicit and explicit self annotations, since the latter seem to be handled correctly already. Fixes rust-lang/rust#93093