From 65eb381dec051e31d1fb17a5a7629bdee1eb9922 Mon Sep 17 00:00:00 2001 From: Fabian Wolff Date: Sat, 11 Sep 2021 01:15:40 +0200 Subject: [PATCH 1/9] Do not suggest importing inaccessible items --- compiler/rustc_resolve/src/diagnostics.rs | 85 +++++++++++++------ src/test/ui/hygiene/globs.stderr | 6 +- src/test/ui/imports/glob-resolve1.stderr | 54 ++++-------- src/test/ui/imports/issue-4366-2.stderr | 6 +- src/test/ui/resolve/issue-42944.stderr | 6 +- src/test/ui/resolve/issue-88472.rs | 35 ++++++++ src/test/ui/resolve/issue-88472.stderr | 34 ++++++++ src/test/ui/resolve/privacy-enum-ctor.stderr | 48 +++-------- .../ui/resolve/privacy-struct-ctor.stderr | 6 +- src/test/ui/self/self_type_keyword.stderr | 6 +- 10 files changed, 165 insertions(+), 121 deletions(-) create mode 100644 src/test/ui/resolve/issue-88472.rs create mode 100644 src/test/ui/resolve/issue-88472.stderr diff --git a/compiler/rustc_resolve/src/diagnostics.rs b/compiler/rustc_resolve/src/diagnostics.rs index d6ff5a7e90b21..e75db6f3f0026 100644 --- a/compiler/rustc_resolve/src/diagnostics.rs +++ b/compiler/rustc_resolve/src/diagnostics.rs @@ -1700,41 +1700,72 @@ crate fn show_candidates( return; } + let mut accessible_path_strings: Vec<(String, &str)> = Vec::new(); + let mut inaccessible_path_strings: Vec<(String, &str)> = Vec::new(); + + candidates.iter().for_each(|c| { + (if c.accessible { &mut accessible_path_strings } else { &mut inaccessible_path_strings }) + .push((path_names_to_string(&c.path), c.descr)) + }); + // we want consistent results across executions, but candidates are produced // by iterating through a hash map, so make sure they are ordered: - let mut path_strings: Vec<_> = - candidates.iter().map(|c| path_names_to_string(&c.path)).collect(); + for path_strings in [&mut accessible_path_strings, &mut inaccessible_path_strings] { + path_strings.sort(); + let core_path_strings = + path_strings.drain_filter(|p| p.starts_with("core::")).collect::>(); + path_strings.extend(core_path_strings); + path_strings.dedup(); + } - path_strings.sort(); - let core_path_strings = - path_strings.drain_filter(|p| p.starts_with("core::")).collect::>(); - path_strings.extend(core_path_strings); - path_strings.dedup(); + if !accessible_path_strings.is_empty() { + let (determiner, kind) = if accessible_path_strings.len() == 1 { + ("this", accessible_path_strings[0].1) + } else { + ("one of these", "items") + }; - let (determiner, kind) = if candidates.len() == 1 { - ("this", candidates[0].descr) - } else { - ("one of these", "items") - }; - - let instead = if instead { " instead" } else { "" }; - let mut msg = format!("consider importing {} {}{}", determiner, kind, instead); - - if let Some(span) = use_placement_span { - for candidate in &mut path_strings { - // produce an additional newline to separate the new use statement - // from the directly following item. - let additional_newline = if found_use { "" } else { "\n" }; - *candidate = format!("use {};\n{}", candidate, additional_newline); - } + let instead = if instead { " instead" } else { "" }; + let mut msg = format!("consider importing {} {}{}", determiner, kind, instead); - err.span_suggestions(span, &msg, path_strings.into_iter(), Applicability::Unspecified); + if let Some(span) = use_placement_span { + for candidate in &mut accessible_path_strings { + // produce an additional newline to separate the new use statement + // from the directly following item. + let additional_newline = if found_use { "" } else { "\n" }; + candidate.0 = format!("use {};\n{}", &candidate.0, additional_newline); + } + + err.span_suggestions( + span, + &msg, + accessible_path_strings.into_iter().map(|a| a.0), + Applicability::Unspecified, + ); + } else { + msg.push(':'); + + for candidate in accessible_path_strings { + msg.push('\n'); + msg.push_str(&candidate.0); + } + + err.note(&msg); + } } else { - msg.push(':'); + assert!(!inaccessible_path_strings.is_empty()); + + let (determiner, kind, verb1, verb2) = if inaccessible_path_strings.len() == 1 { + ("this", inaccessible_path_strings[0].1, "exists", "is") + } else { + ("these", "items", "exist", "are") + }; + + let mut msg = format!("{} {} {} but {} inaccessible:", determiner, kind, verb1, verb2); - for candidate in path_strings { + for candidate in inaccessible_path_strings { msg.push('\n'); - msg.push_str(&candidate); + msg.push_str(&candidate.0); } err.note(&msg); diff --git a/src/test/ui/hygiene/globs.stderr b/src/test/ui/hygiene/globs.stderr index c2497f8ff74d2..6c8b707b8e2f1 100644 --- a/src/test/ui/hygiene/globs.stderr +++ b/src/test/ui/hygiene/globs.stderr @@ -4,7 +4,7 @@ error[E0425]: cannot find function `f` in this scope LL | f(); | ^ not found in this scope | -help: consider importing one of these items +help: consider importing this function | LL | use foo::f; | @@ -37,7 +37,7 @@ LL | n!(f); LL | n!(f); | ^ not found in this scope | - = note: consider importing one of these items: + = note: consider importing this function: foo::f = note: this error originates in the macro `n` (in Nightly builds, run with -Z macro-backtrace for more info) @@ -50,7 +50,7 @@ LL | n!(f); LL | f | ^ not found in this scope | - = note: consider importing one of these items: + = note: consider importing this function: foo::f = note: this error originates in the macro `n` (in Nightly builds, run with -Z macro-backtrace for more info) diff --git a/src/test/ui/imports/glob-resolve1.stderr b/src/test/ui/imports/glob-resolve1.stderr index 2c7a8ad5c1adb..3e23c278d669d 100644 --- a/src/test/ui/imports/glob-resolve1.stderr +++ b/src/test/ui/imports/glob-resolve1.stderr @@ -4,10 +4,8 @@ error[E0425]: cannot find function `fpriv` in this scope LL | fpriv(); | ^^^^^ not found in this scope | -help: consider importing this function - | -LL | use bar::fpriv; - | + = note: this function exists but is inaccessible: + bar::fpriv error[E0425]: cannot find function `epriv` in this scope --> $DIR/glob-resolve1.rs:27:5 @@ -15,10 +13,8 @@ error[E0425]: cannot find function `epriv` in this scope LL | epriv(); | ^^^^^ not found in this scope | -help: consider importing this function - | -LL | use bar::epriv; - | + = note: this function exists but is inaccessible: + bar::epriv error[E0423]: expected value, found enum `B` --> $DIR/glob-resolve1.rs:28:5 @@ -44,10 +40,8 @@ error[E0425]: cannot find value `C` in this scope LL | C; | ^ not found in this scope | -help: consider importing this unit struct - | -LL | use bar::C; - | + = note: this unit struct exists but is inaccessible: + bar::C error[E0425]: cannot find function `import` in this scope --> $DIR/glob-resolve1.rs:30:5 @@ -67,16 +61,10 @@ LL | pub enum B { | ---------- similarly named enum `B` defined here ... LL | foo::(); - | ^ - | -help: an enum with a similar name exists - | -LL | foo::(); - | ~ -help: consider importing this enum - | -LL | use bar::A; + | ^ help: an enum with a similar name exists: `B` | + = note: this enum exists but is inaccessible: + bar::A error[E0412]: cannot find type `C` in this scope --> $DIR/glob-resolve1.rs:33:11 @@ -85,16 +73,10 @@ LL | pub enum B { | ---------- similarly named enum `B` defined here ... LL | foo::(); - | ^ - | -help: an enum with a similar name exists - | -LL | foo::(); - | ~ -help: consider importing this struct - | -LL | use bar::C; + | ^ help: an enum with a similar name exists: `B` | + = note: this struct exists but is inaccessible: + bar::C error[E0412]: cannot find type `D` in this scope --> $DIR/glob-resolve1.rs:34:11 @@ -103,16 +85,10 @@ LL | pub enum B { | ---------- similarly named enum `B` defined here ... LL | foo::(); - | ^ - | -help: an enum with a similar name exists - | -LL | foo::(); - | ~ -help: consider importing this type alias - | -LL | use bar::D; + | ^ help: an enum with a similar name exists: `B` | + = note: this type alias exists but is inaccessible: + bar::D error: aborting due to 8 previous errors diff --git a/src/test/ui/imports/issue-4366-2.stderr b/src/test/ui/imports/issue-4366-2.stderr index a86ec7fabea4b..225104d0dde24 100644 --- a/src/test/ui/imports/issue-4366-2.stderr +++ b/src/test/ui/imports/issue-4366-2.stderr @@ -4,10 +4,8 @@ error[E0412]: cannot find type `Bar` in this scope LL | fn sub() -> Bar { 1 } | ^^^ not found in this scope | -help: consider importing this type alias - | -LL | use a::b::Bar; - | + = note: this type alias exists but is inaccessible: + a::b::Bar error[E0423]: expected function, found module `foo` --> $DIR/issue-4366-2.rs:25:5 diff --git a/src/test/ui/resolve/issue-42944.stderr b/src/test/ui/resolve/issue-42944.stderr index 008492529d18c..1ca7a6d34f782 100644 --- a/src/test/ui/resolve/issue-42944.stderr +++ b/src/test/ui/resolve/issue-42944.stderr @@ -16,10 +16,8 @@ error[E0425]: cannot find function, tuple struct or tuple variant `Bx` in this s LL | Bx(()); | ^^ not found in this scope | -help: consider importing this tuple struct - | -LL | use foo::Bx; - | + = note: this tuple struct exists but is inaccessible: + foo::Bx error: aborting due to 2 previous errors diff --git a/src/test/ui/resolve/issue-88472.rs b/src/test/ui/resolve/issue-88472.rs new file mode 100644 index 0000000000000..53cfec5746664 --- /dev/null +++ b/src/test/ui/resolve/issue-88472.rs @@ -0,0 +1,35 @@ +// Regression test for #88472, where a suggestion was issued to +// import an inaccessible struct. + +#![warn(unused_imports)] +//~^ NOTE: the lint level is defined here + +mod a { + struct Foo; +} + +mod b { + use crate::a::*; + //~^ WARNING: unused import + type Bar = Foo; + //~^ ERROR: cannot find type `Foo` in this scope [E0412] + //~| NOTE: not found in this scope + //~| NOTE: this struct exists but is inaccessible +} + +mod c { + enum Eee {} + + mod d { + enum Eee {} + } +} + +mod e { + type Baz = Eee; + //~^ ERROR: cannot find type `Eee` in this scope [E0412] + //~| NOTE: not found in this scope + //~| NOTE: these items exist but are inaccessible +} + +fn main() {} diff --git a/src/test/ui/resolve/issue-88472.stderr b/src/test/ui/resolve/issue-88472.stderr new file mode 100644 index 0000000000000..6846210b302be --- /dev/null +++ b/src/test/ui/resolve/issue-88472.stderr @@ -0,0 +1,34 @@ +error[E0412]: cannot find type `Foo` in this scope + --> $DIR/issue-88472.rs:14:16 + | +LL | type Bar = Foo; + | ^^^ not found in this scope + | + = note: this struct exists but is inaccessible: + a::Foo + +error[E0412]: cannot find type `Eee` in this scope + --> $DIR/issue-88472.rs:29:16 + | +LL | type Baz = Eee; + | ^^^ not found in this scope + | + = note: these items exist but are inaccessible: + c::Eee + c::d::Eee + +warning: unused import: `crate::a::*` + --> $DIR/issue-88472.rs:12:9 + | +LL | use crate::a::*; + | ^^^^^^^^^^^ + | +note: the lint level is defined here + --> $DIR/issue-88472.rs:4:9 + | +LL | #![warn(unused_imports)] + | ^^^^^^^^^^^^^^ + +error: aborting due to 2 previous errors; 1 warning emitted + +For more information about this error, try `rustc --explain E0412`. diff --git a/src/test/ui/resolve/privacy-enum-ctor.stderr b/src/test/ui/resolve/privacy-enum-ctor.stderr index 192349e0fafe3..e79f953970a43 100644 --- a/src/test/ui/resolve/privacy-enum-ctor.stderr +++ b/src/test/ui/resolve/privacy-enum-ctor.stderr @@ -169,16 +169,10 @@ LL | pub enum E { | ---------- similarly named enum `E` defined here ... LL | let _: Z = m::n::Z; - | ^ - | -help: an enum with a similar name exists - | -LL | let _: E = m::n::Z; - | ~ -help: consider importing this enum - | -LL | use m::Z; + | ^ help: an enum with a similar name exists: `E` | + = note: this enum exists but is inaccessible: + m::Z error[E0423]: expected value, found enum `m::n::Z` --> $DIR/privacy-enum-ctor.rs:57:16 @@ -215,16 +209,10 @@ LL | pub enum E { | ---------- similarly named enum `E` defined here ... LL | let _: Z = m::n::Z::Fn; - | ^ - | -help: an enum with a similar name exists - | -LL | let _: E = m::n::Z::Fn; - | ~ -help: consider importing this enum - | -LL | use m::Z; + | ^ help: an enum with a similar name exists: `E` | + = note: this enum exists but is inaccessible: + m::Z error[E0412]: cannot find type `Z` in this scope --> $DIR/privacy-enum-ctor.rs:64:12 @@ -233,16 +221,10 @@ LL | pub enum E { | ---------- similarly named enum `E` defined here ... LL | let _: Z = m::n::Z::Struct; - | ^ - | -help: an enum with a similar name exists - | -LL | let _: E = m::n::Z::Struct; - | ~ -help: consider importing this enum - | -LL | use m::Z; + | ^ help: an enum with a similar name exists: `E` | + = note: this enum exists but is inaccessible: + m::Z error[E0423]: expected value, found struct variant `m::n::Z::Struct` --> $DIR/privacy-enum-ctor.rs:64:16 @@ -262,16 +244,10 @@ LL | pub enum E { | ---------- similarly named enum `E` defined here ... LL | let _: Z = m::n::Z::Unit {}; - | ^ - | -help: an enum with a similar name exists - | -LL | let _: E = m::n::Z::Unit {}; - | ~ -help: consider importing this enum - | -LL | use m::Z; + | ^ help: an enum with a similar name exists: `E` | + = note: this enum exists but is inaccessible: + m::Z error[E0603]: enum `Z` is private --> $DIR/privacy-enum-ctor.rs:57:22 diff --git a/src/test/ui/resolve/privacy-struct-ctor.stderr b/src/test/ui/resolve/privacy-struct-ctor.stderr index e5d6f7e9e24f0..bf1d6753cc1d0 100644 --- a/src/test/ui/resolve/privacy-struct-ctor.stderr +++ b/src/test/ui/resolve/privacy-struct-ctor.stderr @@ -33,10 +33,8 @@ error[E0423]: expected value, found struct `xcrate::S` LL | xcrate::S; | ^^^^^^^^^ constructor is not visible here due to private fields | -help: consider importing this tuple struct instead - | -LL | use m::S; - | + = note: this tuple struct exists but is inaccessible: + m::S error[E0603]: tuple struct constructor `Z` is private --> $DIR/privacy-struct-ctor.rs:18:12 diff --git a/src/test/ui/self/self_type_keyword.stderr b/src/test/ui/self/self_type_keyword.stderr index 47c04f1eb72ec..57b46058777ae 100644 --- a/src/test/ui/self/self_type_keyword.stderr +++ b/src/test/ui/self/self_type_keyword.stderr @@ -66,10 +66,8 @@ error[E0531]: cannot find unit struct, unit variant or constant `Self` in this s LL | mut Self => (), | ^^^^ not found in this scope | -help: consider importing this unit struct - | -LL | use foo::Self; - | + = note: this unit struct exists but is inaccessible: + foo::Self error[E0392]: parameter `'Self` is never used --> $DIR/self_type_keyword.rs:6:12 From 5c70f25f8c68716ae448fd62c47483d0e0128e53 Mon Sep 17 00:00:00 2001 From: Esteban Kuber Date: Sat, 25 Sep 2021 18:23:07 +0000 Subject: [PATCH 2/9] Detect when negative literal indices are used and suggest appropriate code --- compiler/rustc_typeck/src/check/expr.rs | 2 +- compiler/rustc_typeck/src/check/place_op.rs | 53 ++++++++++++++++++- .../suggestions/negative-literal-index.fixed | 22 ++++++++ .../ui/suggestions/negative-literal-index.rs | 22 ++++++++ .../suggestions/negative-literal-index.stderr | 35 ++++++++++++ 5 files changed, 132 insertions(+), 2 deletions(-) create mode 100644 src/test/ui/suggestions/negative-literal-index.fixed create mode 100644 src/test/ui/suggestions/negative-literal-index.rs create mode 100644 src/test/ui/suggestions/negative-literal-index.stderr diff --git a/compiler/rustc_typeck/src/check/expr.rs b/compiler/rustc_typeck/src/check/expr.rs index 8a69e0a737d50..09aeb62fc02c7 100644 --- a/compiler/rustc_typeck/src/check/expr.rs +++ b/compiler/rustc_typeck/src/check/expr.rs @@ -2136,7 +2136,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { idx_t } else { let base_t = self.structurally_resolved_type(base.span, base_t); - match self.lookup_indexing(expr, base, base_t, idx_t) { + match self.lookup_indexing(expr, base, base_t, idx, idx_t) { Some((index_ty, element_ty)) => { // two-phase not needed because index_ty is never mutable self.demand_coerce(idx, idx_t, index_ty, None, AllowTwoPhase::No); diff --git a/compiler/rustc_typeck/src/check/place_op.rs b/compiler/rustc_typeck/src/check/place_op.rs index 055072d3a1d9d..e5a5066544a8f 100644 --- a/compiler/rustc_typeck/src/check/place_op.rs +++ b/compiler/rustc_typeck/src/check/place_op.rs @@ -1,5 +1,7 @@ use crate::check::method::MethodCallee; use crate::check::{has_expected_num_generic_args, FnCtxt, PlaceOp}; +use rustc_ast as ast; +use rustc_errors::Applicability; use rustc_hir as hir; use rustc_infer::infer::type_variable::{TypeVariableOrigin, TypeVariableOriginKind}; use rustc_infer::infer::InferOk; @@ -47,6 +49,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { expr: &hir::Expr<'_>, base_expr: &'tcx hir::Expr<'tcx>, base_ty: Ty<'tcx>, + idx_expr: &'tcx hir::Expr<'tcx>, idx_ty: Ty<'tcx>, ) -> Option<(/*index type*/ Ty<'tcx>, /*element type*/ Ty<'tcx>)> { // FIXME(#18741) -- this is almost but not quite the same as the @@ -56,7 +59,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { let mut autoderef = self.autoderef(base_expr.span, base_ty); let mut result = None; while result.is_none() && autoderef.next().is_some() { - result = self.try_index_step(expr, base_expr, &autoderef, idx_ty); + result = self.try_index_step(expr, base_expr, &autoderef, idx_ty, idx_expr); } self.register_predicates(autoderef.into_obligations()); result @@ -73,6 +76,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { base_expr: &hir::Expr<'_>, autoderef: &Autoderef<'a, 'tcx>, index_ty: Ty<'tcx>, + idx_expr: &hir::Expr<'_>, ) -> Option<(/*index type*/ Ty<'tcx>, /*element type*/ Ty<'tcx>)> { let adjusted_ty = self.structurally_resolved_type(autoderef.span(), autoderef.final_ty(false)); @@ -82,6 +86,53 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { expr, base_expr, adjusted_ty, index_ty ); + let negative_index = || { + let ty = self.resolve_vars_if_possible(adjusted_ty); + let mut err = self.tcx.sess.struct_span_err( + idx_expr.span, + &format!("negative integers cannot be used to index on a `{}`", ty), + ); + err.span_label( + idx_expr.span, + &format!("cannot use a negative integer for indexing on `{}`", ty), + ); + if let (hir::ExprKind::Path(..), Ok(snippet)) = + (&base_expr.kind, self.tcx.sess.source_map().span_to_snippet(base_expr.span)) + { + // `foo[-1]` to `foo[foo.len() - 1]` + err.span_suggestion_verbose( + idx_expr.span.shrink_to_lo(), + &format!( + "if you wanted to access an element starting from the end of the `{}`, you \ + must compute it", + ty, + ), + format!("{}.len() ", snippet), + Applicability::MachineApplicable, + ); + } + err.emit(); + Some((self.tcx.ty_error(), self.tcx.ty_error())) + }; + if let hir::ExprKind::Unary( + hir::UnOp::Neg, + hir::Expr { + kind: hir::ExprKind::Lit(hir::Lit { node: ast::LitKind::Int(..), .. }), + .. + }, + ) = idx_expr.kind + { + match adjusted_ty.kind() { + ty::Adt(ty::AdtDef { did, .. }, _) + if self.tcx.is_diagnostic_item(sym::vec_type, *did) => + { + return negative_index(); + } + ty::Slice(_) | ty::Array(_, _) => return negative_index(), + _ => {} + } + } + for unsize in [false, true] { let mut self_ty = adjusted_ty; if unsize { diff --git a/src/test/ui/suggestions/negative-literal-index.fixed b/src/test/ui/suggestions/negative-literal-index.fixed new file mode 100644 index 0000000000000..e52714cf97fe6 --- /dev/null +++ b/src/test/ui/suggestions/negative-literal-index.fixed @@ -0,0 +1,22 @@ +// run-rustfix + +use std::ops::Index; +struct X; +impl Index for X { + type Output = (); + + fn index(&self, _: i32) -> &() { + &() + } +} + +fn main() { + let x = vec![1, 2, 3]; + x[x.len() -1]; //~ ERROR negative integers cannot be used to index on a + let x = [1, 2, 3]; + x[x.len() -1]; //~ ERROR negative integers cannot be used to index on a + let x = &[1, 2, 3]; + x[x.len() -1]; //~ ERROR negative integers cannot be used to index on a + let _ = x; + X[-1]; +} diff --git a/src/test/ui/suggestions/negative-literal-index.rs b/src/test/ui/suggestions/negative-literal-index.rs new file mode 100644 index 0000000000000..d88b66e679e56 --- /dev/null +++ b/src/test/ui/suggestions/negative-literal-index.rs @@ -0,0 +1,22 @@ +// run-rustfix + +use std::ops::Index; +struct X; +impl Index for X { + type Output = (); + + fn index(&self, _: i32) -> &() { + &() + } +} + +fn main() { + let x = vec![1, 2, 3]; + x[-1]; //~ ERROR negative integers cannot be used to index on a + let x = [1, 2, 3]; + x[-1]; //~ ERROR negative integers cannot be used to index on a + let x = &[1, 2, 3]; + x[-1]; //~ ERROR negative integers cannot be used to index on a + let _ = x; + X[-1]; +} diff --git a/src/test/ui/suggestions/negative-literal-index.stderr b/src/test/ui/suggestions/negative-literal-index.stderr new file mode 100644 index 0000000000000..f5ea980048b5b --- /dev/null +++ b/src/test/ui/suggestions/negative-literal-index.stderr @@ -0,0 +1,35 @@ +error: negative integers cannot be used to index on a `Vec<{integer}>` + --> $DIR/negative-literal-index.rs:15:7 + | +LL | x[-1]; + | ^^ cannot use a negative integer for indexing on `Vec<{integer}>` + | +help: if you wanted to access an element starting from the end of the `Vec<{integer}>`, you must compute it + | +LL | x[x.len() -1]; + | +++++++ + +error: negative integers cannot be used to index on a `[{integer}; 3]` + --> $DIR/negative-literal-index.rs:17:7 + | +LL | x[-1]; + | ^^ cannot use a negative integer for indexing on `[{integer}; 3]` + | +help: if you wanted to access an element starting from the end of the `[{integer}; 3]`, you must compute it + | +LL | x[x.len() -1]; + | +++++++ + +error: negative integers cannot be used to index on a `[{integer}; 3]` + --> $DIR/negative-literal-index.rs:19:7 + | +LL | x[-1]; + | ^^ cannot use a negative integer for indexing on `[{integer}; 3]` + | +help: if you wanted to access an element starting from the end of the `[{integer}; 3]`, you must compute it + | +LL | x[x.len() -1]; + | +++++++ + +error: aborting due to 3 previous errors + From 750018e16eb34f8b37477689fbd231bb720c8aaa Mon Sep 17 00:00:00 2001 From: Fabian Wolff Date: Sat, 18 Sep 2021 00:07:41 +0200 Subject: [PATCH 3/9] Improve diagnostics for inaccessible items --- compiler/rustc_resolve/src/diagnostics.rs | 81 +++++++++++++++---- compiler/rustc_resolve/src/lib.rs | 10 ++- src/test/ui/imports/glob-resolve1.stderr | 42 +++++++--- src/test/ui/imports/issue-4366-2.stderr | 7 +- src/test/ui/resolve/issue-42944.stderr | 7 +- src/test/ui/resolve/issue-88472.rs | 7 +- src/test/ui/resolve/issue-88472.stderr | 24 ++++-- src/test/ui/resolve/privacy-enum-ctor.stderr | 28 +++++-- .../ui/resolve/privacy-struct-ctor.stderr | 7 +- src/test/ui/self/self_type_keyword.stderr | 7 +- 10 files changed, 164 insertions(+), 56 deletions(-) diff --git a/compiler/rustc_resolve/src/diagnostics.rs b/compiler/rustc_resolve/src/diagnostics.rs index e75db6f3f0026..4b36196d4669c 100644 --- a/compiler/rustc_resolve/src/diagnostics.rs +++ b/compiler/rustc_resolve/src/diagnostics.rs @@ -949,7 +949,15 @@ impl<'a> Resolver<'a> { let import_suggestions = self.lookup_import_candidates(ident, Namespace::MacroNS, parent_scope, is_expected); - show_candidates(err, None, &import_suggestions, false, true); + show_candidates( + &self.definitions, + self.session, + err, + None, + &import_suggestions, + false, + true, + ); if macro_kind == MacroKind::Derive && (ident.name == sym::Send || ident.name == sym::Sync) { let msg = format!("unsafe traits like `{}` should be implemented explicitly", ident); @@ -1689,6 +1697,8 @@ fn find_span_immediately_after_crate_name( /// entities with that name in all crates. This method allows outputting the /// results of this search in a programmer-friendly way crate fn show_candidates( + definitions: &rustc_hir::definitions::Definitions, + session: &Session, err: &mut DiagnosticBuilder<'_>, // This is `None` if all placement locations are inside expansions use_placement_span: Option, @@ -1700,22 +1710,22 @@ crate fn show_candidates( return; } - let mut accessible_path_strings: Vec<(String, &str)> = Vec::new(); - let mut inaccessible_path_strings: Vec<(String, &str)> = Vec::new(); + let mut accessible_path_strings: Vec<(String, &str, Option)> = Vec::new(); + let mut inaccessible_path_strings: Vec<(String, &str, Option)> = Vec::new(); candidates.iter().for_each(|c| { (if c.accessible { &mut accessible_path_strings } else { &mut inaccessible_path_strings }) - .push((path_names_to_string(&c.path), c.descr)) + .push((path_names_to_string(&c.path), c.descr, c.did)) }); // we want consistent results across executions, but candidates are produced // by iterating through a hash map, so make sure they are ordered: for path_strings in [&mut accessible_path_strings, &mut inaccessible_path_strings] { - path_strings.sort(); + path_strings.sort_by(|a, b| a.0.cmp(&b.0)); let core_path_strings = - path_strings.drain_filter(|p| p.starts_with("core::")).collect::>(); + path_strings.drain_filter(|p| p.0.starts_with("core::")).collect::>(); path_strings.extend(core_path_strings); - path_strings.dedup(); + path_strings.dedup_by(|a, b| a.0 == b.0); } if !accessible_path_strings.is_empty() { @@ -1755,19 +1765,56 @@ crate fn show_candidates( } else { assert!(!inaccessible_path_strings.is_empty()); - let (determiner, kind, verb1, verb2) = if inaccessible_path_strings.len() == 1 { - ("this", inaccessible_path_strings[0].1, "exists", "is") + if inaccessible_path_strings.len() == 1 { + let (name, descr, def_id) = &inaccessible_path_strings[0]; + let msg = format!("{} `{}` exists but is inaccessible", descr, name); + + if let Some(local_def_id) = def_id.and_then(|did| did.as_local()) { + let span = definitions.def_span(local_def_id); + let span = session.source_map().guess_head_span(span); + let mut multi_span = MultiSpan::from_span(span); + multi_span.push_span_label(span, "not accessible".to_string()); + err.span_note(multi_span, &msg); + } else { + err.note(&msg); + } } else { - ("these", "items", "exist", "are") - }; + let (_, descr_first, _) = &inaccessible_path_strings[0]; + let descr = if inaccessible_path_strings + .iter() + .skip(1) + .all(|(_, descr, _)| descr == descr_first) + { + format!("{}", descr_first) + } else { + "item".to_string() + }; - let mut msg = format!("{} {} {} but {} inaccessible:", determiner, kind, verb1, verb2); + let mut msg = format!("these {}s exist but are inaccessible", descr); + let mut has_colon = false; - for candidate in inaccessible_path_strings { - msg.push('\n'); - msg.push_str(&candidate.0); - } + let mut spans = Vec::new(); + for (name, _, def_id) in &inaccessible_path_strings { + if let Some(local_def_id) = def_id.and_then(|did| did.as_local()) { + let span = definitions.def_span(local_def_id); + let span = session.source_map().guess_head_span(span); + spans.push((name, span)); + } else { + if !has_colon { + msg.push(':'); + has_colon = true; + } + msg.push('\n'); + msg.push_str(name); + } + } + + let mut multi_span = MultiSpan::from_spans(spans.iter().map(|(_, sp)| *sp).collect()); + for (name, span) in spans { + multi_span.push_span_label(span, format!("`{}`: not accessible", name)); + } - err.note(&msg); + err.span_note(multi_span, &msg); + } } } diff --git a/compiler/rustc_resolve/src/lib.rs b/compiler/rustc_resolve/src/lib.rs index 1cbe8f41d92b0..5ca2392a60ab2 100644 --- a/compiler/rustc_resolve/src/lib.rs +++ b/compiler/rustc_resolve/src/lib.rs @@ -2966,7 +2966,15 @@ impl<'a> Resolver<'a> { (None, false) }; if !candidates.is_empty() { - diagnostics::show_candidates(&mut err, span, &candidates, instead, found_use); + diagnostics::show_candidates( + &self.definitions, + self.session, + &mut err, + span, + &candidates, + instead, + found_use, + ); } else if let Some((span, msg, sugg, appl)) = suggestion { err.span_suggestion(span, msg, sugg, appl); } diff --git a/src/test/ui/imports/glob-resolve1.stderr b/src/test/ui/imports/glob-resolve1.stderr index 3e23c278d669d..3b66a5e315050 100644 --- a/src/test/ui/imports/glob-resolve1.stderr +++ b/src/test/ui/imports/glob-resolve1.stderr @@ -4,8 +4,11 @@ error[E0425]: cannot find function `fpriv` in this scope LL | fpriv(); | ^^^^^ not found in this scope | - = note: this function exists but is inaccessible: - bar::fpriv +note: function `bar::fpriv` exists but is inaccessible + --> $DIR/glob-resolve1.rs:7:5 + | +LL | fn fpriv() {} + | ^^^^^^^^^^ not accessible error[E0425]: cannot find function `epriv` in this scope --> $DIR/glob-resolve1.rs:27:5 @@ -13,8 +16,11 @@ error[E0425]: cannot find function `epriv` in this scope LL | epriv(); | ^^^^^ not found in this scope | - = note: this function exists but is inaccessible: - bar::epriv +note: function `bar::epriv` exists but is inaccessible + --> $DIR/glob-resolve1.rs:9:9 + | +LL | fn epriv(); + | ^^^^^^^^^^^ not accessible error[E0423]: expected value, found enum `B` --> $DIR/glob-resolve1.rs:28:5 @@ -40,8 +46,11 @@ error[E0425]: cannot find value `C` in this scope LL | C; | ^ not found in this scope | - = note: this unit struct exists but is inaccessible: - bar::C +note: unit struct `bar::C` exists but is inaccessible + --> $DIR/glob-resolve1.rs:18:5 + | +LL | struct C; + | ^^^^^^^^^ not accessible error[E0425]: cannot find function `import` in this scope --> $DIR/glob-resolve1.rs:30:5 @@ -63,8 +72,11 @@ LL | pub enum B { LL | foo::(); | ^ help: an enum with a similar name exists: `B` | - = note: this enum exists but is inaccessible: - bar::A +note: enum `bar::A` exists but is inaccessible + --> $DIR/glob-resolve1.rs:11:5 + | +LL | enum A { + | ^^^^^^ not accessible error[E0412]: cannot find type `C` in this scope --> $DIR/glob-resolve1.rs:33:11 @@ -75,8 +87,11 @@ LL | pub enum B { LL | foo::(); | ^ help: an enum with a similar name exists: `B` | - = note: this struct exists but is inaccessible: - bar::C +note: struct `bar::C` exists but is inaccessible + --> $DIR/glob-resolve1.rs:18:5 + | +LL | struct C; + | ^^^^^^^^^ not accessible error[E0412]: cannot find type `D` in this scope --> $DIR/glob-resolve1.rs:34:11 @@ -87,8 +102,11 @@ LL | pub enum B { LL | foo::(); | ^ help: an enum with a similar name exists: `B` | - = note: this type alias exists but is inaccessible: - bar::D +note: type alias `bar::D` exists but is inaccessible + --> $DIR/glob-resolve1.rs:20:5 + | +LL | type D = isize; + | ^^^^^^^^^^^^^^^ not accessible error: aborting due to 8 previous errors diff --git a/src/test/ui/imports/issue-4366-2.stderr b/src/test/ui/imports/issue-4366-2.stderr index 225104d0dde24..4c94634ee60f7 100644 --- a/src/test/ui/imports/issue-4366-2.stderr +++ b/src/test/ui/imports/issue-4366-2.stderr @@ -4,8 +4,11 @@ error[E0412]: cannot find type `Bar` in this scope LL | fn sub() -> Bar { 1 } | ^^^ not found in this scope | - = note: this type alias exists but is inaccessible: - a::b::Bar +note: type alias `a::b::Bar` exists but is inaccessible + --> $DIR/issue-4366-2.rs:11:9 + | +LL | type Bar = isize; + | ^^^^^^^^^^^^^^^^^ not accessible error[E0423]: expected function, found module `foo` --> $DIR/issue-4366-2.rs:25:5 diff --git a/src/test/ui/resolve/issue-42944.stderr b/src/test/ui/resolve/issue-42944.stderr index 1ca7a6d34f782..cad3ccc4a0ef8 100644 --- a/src/test/ui/resolve/issue-42944.stderr +++ b/src/test/ui/resolve/issue-42944.stderr @@ -16,8 +16,11 @@ error[E0425]: cannot find function, tuple struct or tuple variant `Bx` in this s LL | Bx(()); | ^^ not found in this scope | - = note: this tuple struct exists but is inaccessible: - foo::Bx +note: tuple struct `foo::Bx` exists but is inaccessible + --> $DIR/issue-42944.rs:2:5 + | +LL | pub struct Bx(()); + | ^^^^^^^^^^^^^^^^^^ not accessible error: aborting due to 2 previous errors diff --git a/src/test/ui/resolve/issue-88472.rs b/src/test/ui/resolve/issue-88472.rs index 53cfec5746664..6bf7caeddbfcd 100644 --- a/src/test/ui/resolve/issue-88472.rs +++ b/src/test/ui/resolve/issue-88472.rs @@ -6,6 +6,8 @@ mod a { struct Foo; + //~^ NOTE: struct `a::Foo` exists but is inaccessible + //~| NOTE: not accessible } mod b { @@ -14,14 +16,16 @@ mod b { type Bar = Foo; //~^ ERROR: cannot find type `Foo` in this scope [E0412] //~| NOTE: not found in this scope - //~| NOTE: this struct exists but is inaccessible } mod c { enum Eee {} + //~^ NOTE: these enums exist but are inaccessible + //~| NOTE: `c::Eee`: not accessible mod d { enum Eee {} + //~^ NOTE: `c::d::Eee`: not accessible } } @@ -29,7 +33,6 @@ mod e { type Baz = Eee; //~^ ERROR: cannot find type `Eee` in this scope [E0412] //~| NOTE: not found in this scope - //~| NOTE: these items exist but are inaccessible } fn main() {} diff --git a/src/test/ui/resolve/issue-88472.stderr b/src/test/ui/resolve/issue-88472.stderr index 6846210b302be..8431fc97766f7 100644 --- a/src/test/ui/resolve/issue-88472.stderr +++ b/src/test/ui/resolve/issue-88472.stderr @@ -1,24 +1,32 @@ error[E0412]: cannot find type `Foo` in this scope - --> $DIR/issue-88472.rs:14:16 + --> $DIR/issue-88472.rs:16:16 | LL | type Bar = Foo; | ^^^ not found in this scope | - = note: this struct exists but is inaccessible: - a::Foo +note: struct `a::Foo` exists but is inaccessible + --> $DIR/issue-88472.rs:8:5 + | +LL | struct Foo; + | ^^^^^^^^^^^ not accessible error[E0412]: cannot find type `Eee` in this scope - --> $DIR/issue-88472.rs:29:16 + --> $DIR/issue-88472.rs:33:16 | LL | type Baz = Eee; | ^^^ not found in this scope | - = note: these items exist but are inaccessible: - c::Eee - c::d::Eee +note: these enums exist but are inaccessible + --> $DIR/issue-88472.rs:22:5 + | +LL | enum Eee {} + | ^^^^^^^^ `c::Eee`: not accessible +... +LL | enum Eee {} + | ^^^^^^^^ `c::d::Eee`: not accessible warning: unused import: `crate::a::*` - --> $DIR/issue-88472.rs:12:9 + --> $DIR/issue-88472.rs:14:9 | LL | use crate::a::*; | ^^^^^^^^^^^ diff --git a/src/test/ui/resolve/privacy-enum-ctor.stderr b/src/test/ui/resolve/privacy-enum-ctor.stderr index e79f953970a43..ff72b0b563ab1 100644 --- a/src/test/ui/resolve/privacy-enum-ctor.stderr +++ b/src/test/ui/resolve/privacy-enum-ctor.stderr @@ -171,8 +171,11 @@ LL | pub enum E { LL | let _: Z = m::n::Z; | ^ help: an enum with a similar name exists: `E` | - = note: this enum exists but is inaccessible: - m::Z +note: enum `m::Z` exists but is inaccessible + --> $DIR/privacy-enum-ctor.rs:11:9 + | +LL | pub(in m) enum Z { + | ^^^^^^^^^^^^^^^^ not accessible error[E0423]: expected value, found enum `m::n::Z` --> $DIR/privacy-enum-ctor.rs:57:16 @@ -211,8 +214,11 @@ LL | pub enum E { LL | let _: Z = m::n::Z::Fn; | ^ help: an enum with a similar name exists: `E` | - = note: this enum exists but is inaccessible: - m::Z +note: enum `m::Z` exists but is inaccessible + --> $DIR/privacy-enum-ctor.rs:11:9 + | +LL | pub(in m) enum Z { + | ^^^^^^^^^^^^^^^^ not accessible error[E0412]: cannot find type `Z` in this scope --> $DIR/privacy-enum-ctor.rs:64:12 @@ -223,8 +229,11 @@ LL | pub enum E { LL | let _: Z = m::n::Z::Struct; | ^ help: an enum with a similar name exists: `E` | - = note: this enum exists but is inaccessible: - m::Z +note: enum `m::Z` exists but is inaccessible + --> $DIR/privacy-enum-ctor.rs:11:9 + | +LL | pub(in m) enum Z { + | ^^^^^^^^^^^^^^^^ not accessible error[E0423]: expected value, found struct variant `m::n::Z::Struct` --> $DIR/privacy-enum-ctor.rs:64:16 @@ -246,8 +255,11 @@ LL | pub enum E { LL | let _: Z = m::n::Z::Unit {}; | ^ help: an enum with a similar name exists: `E` | - = note: this enum exists but is inaccessible: - m::Z +note: enum `m::Z` exists but is inaccessible + --> $DIR/privacy-enum-ctor.rs:11:9 + | +LL | pub(in m) enum Z { + | ^^^^^^^^^^^^^^^^ not accessible error[E0603]: enum `Z` is private --> $DIR/privacy-enum-ctor.rs:57:22 diff --git a/src/test/ui/resolve/privacy-struct-ctor.stderr b/src/test/ui/resolve/privacy-struct-ctor.stderr index bf1d6753cc1d0..ada053014ef5e 100644 --- a/src/test/ui/resolve/privacy-struct-ctor.stderr +++ b/src/test/ui/resolve/privacy-struct-ctor.stderr @@ -33,8 +33,11 @@ error[E0423]: expected value, found struct `xcrate::S` LL | xcrate::S; | ^^^^^^^^^ constructor is not visible here due to private fields | - = note: this tuple struct exists but is inaccessible: - m::S +note: tuple struct `m::S` exists but is inaccessible + --> $DIR/privacy-struct-ctor.rs:6:5 + | +LL | pub struct S(u8); + | ^^^^^^^^^^^^^^^^^ not accessible error[E0603]: tuple struct constructor `Z` is private --> $DIR/privacy-struct-ctor.rs:18:12 diff --git a/src/test/ui/self/self_type_keyword.stderr b/src/test/ui/self/self_type_keyword.stderr index 57b46058777ae..aca08d81163fd 100644 --- a/src/test/ui/self/self_type_keyword.stderr +++ b/src/test/ui/self/self_type_keyword.stderr @@ -66,8 +66,11 @@ error[E0531]: cannot find unit struct, unit variant or constant `Self` in this s LL | mut Self => (), | ^^^^ not found in this scope | - = note: this unit struct exists but is inaccessible: - foo::Self +note: unit struct `foo::Self` exists but is inaccessible + --> $DIR/self_type_keyword.rs:2:3 + | +LL | struct Self; + | ^^^^^^^^^^^^ not accessible error[E0392]: parameter `'Self` is never used --> $DIR/self_type_keyword.rs:6:12 From 8901ea29b93eeff11c86986f620108e9f34330df Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Mi=C4=85sko?= Date: Tue, 28 Sep 2021 00:00:00 +0000 Subject: [PATCH 4/9] Rebase resume argument projections during state transform When remapping a resume argument with projections rebase them on top of the new base. The case where resume argument has projections is unusual, but might arise with box syntax where the assignment is performed directly into the box without an intermediate temporary. --- compiler/rustc_mir_transform/src/generator.rs | 5 +++-- src/test/ui/generator/yield-in-box.rs | 10 ++++++++-- src/test/ui/generator/yield-in-box.stderr | 2 +- 3 files changed, 12 insertions(+), 5 deletions(-) diff --git a/compiler/rustc_mir_transform/src/generator.rs b/compiler/rustc_mir_transform/src/generator.rs index 06366b6fc31d5..bc72e9d94a953 100644 --- a/compiler/rustc_mir_transform/src/generator.rs +++ b/compiler/rustc_mir_transform/src/generator.rs @@ -342,7 +342,7 @@ impl MutVisitor<'tcx> for TransformVisitor<'tcx> { let source_info = data.terminator().source_info; // We must assign the value first in case it gets declared dead below data.statements.extend(self.make_state(state_idx, v, source_info)); - let state = if let Some((resume, resume_arg)) = resume { + let state = if let Some((resume, mut resume_arg)) = resume { // Yield let state = 3 + self.suspension_points.len(); @@ -350,7 +350,8 @@ impl MutVisitor<'tcx> for TransformVisitor<'tcx> { // live across a yield. let resume_arg = if let Some(&(ty, variant, idx)) = self.remap.get(&resume_arg.local) { - self.make_field(variant, idx, ty) + replace_base(&mut resume_arg, self.make_field(variant, idx, ty), self.tcx); + resume_arg } else { resume_arg }; diff --git a/src/test/ui/generator/yield-in-box.rs b/src/test/ui/generator/yield-in-box.rs index 65f368df9cb33..dd6fa7c151aa7 100644 --- a/src/test/ui/generator/yield-in-box.rs +++ b/src/test/ui/generator/yield-in-box.rs @@ -1,8 +1,10 @@ // run-pass - // Test that box-statements with yields in them work. -#![feature(generators, box_syntax)] +#![feature(generators, box_syntax, generator_trait)] +use std::pin::Pin; +use std::ops::Generator; +use std::ops::GeneratorState; fn main() { let x = 0i32; @@ -15,4 +17,8 @@ fn main() { _t => {} } }; + + let mut g = |_| box yield; + assert_eq!(Pin::new(&mut g).resume(1), GeneratorState::Yielded(())); + assert_eq!(Pin::new(&mut g).resume(2), GeneratorState::Complete(box 2)); } diff --git a/src/test/ui/generator/yield-in-box.stderr b/src/test/ui/generator/yield-in-box.stderr index 24de18edb0f8c..7602e803945dc 100644 --- a/src/test/ui/generator/yield-in-box.stderr +++ b/src/test/ui/generator/yield-in-box.stderr @@ -1,5 +1,5 @@ warning: unused generator that must be used - --> $DIR/yield-in-box.rs:9:5 + --> $DIR/yield-in-box.rs:11:5 | LL | / || { LL | | let y = 2u32; From e19d82f1bf190de6c67114811e12256cba6831e8 Mon Sep 17 00:00:00 2001 From: Esteban Kuber Date: Tue, 28 Sep 2021 16:13:39 +0000 Subject: [PATCH 5/9] review comments --- compiler/rustc_typeck/src/check/place_op.rs | 72 ++++++++++--------- .../suggestions/negative-literal-index.stderr | 6 +- 2 files changed, 41 insertions(+), 37 deletions(-) diff --git a/compiler/rustc_typeck/src/check/place_op.rs b/compiler/rustc_typeck/src/check/place_op.rs index e5a5066544a8f..64775d7aba9f8 100644 --- a/compiler/rustc_typeck/src/check/place_op.rs +++ b/compiler/rustc_typeck/src/check/place_op.rs @@ -49,7 +49,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { expr: &hir::Expr<'_>, base_expr: &'tcx hir::Expr<'tcx>, base_ty: Ty<'tcx>, - idx_expr: &'tcx hir::Expr<'tcx>, + index_expr: &'tcx hir::Expr<'tcx>, idx_ty: Ty<'tcx>, ) -> Option<(/*index type*/ Ty<'tcx>, /*element type*/ Ty<'tcx>)> { // FIXME(#18741) -- this is almost but not quite the same as the @@ -59,12 +59,42 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { let mut autoderef = self.autoderef(base_expr.span, base_ty); let mut result = None; while result.is_none() && autoderef.next().is_some() { - result = self.try_index_step(expr, base_expr, &autoderef, idx_ty, idx_expr); + result = self.try_index_step(expr, base_expr, &autoderef, idx_ty, index_expr); } self.register_predicates(autoderef.into_obligations()); result } + fn negative_index( + &self, + ty: Ty<'tcx>, + span: Span, + base_expr: &hir::Expr<'_>, + ) -> Option<(Ty<'tcx>, Ty<'tcx>)> { + let ty = self.resolve_vars_if_possible(ty); + let mut err = self.tcx.sess.struct_span_err( + span, + &format!("negative integers cannot be used to index on a `{}`", ty), + ); + err.span_label(span, &format!("cannot use a negative integer for indexing on `{}`", ty)); + if let (hir::ExprKind::Path(..), Ok(snippet)) = + (&base_expr.kind, self.tcx.sess.source_map().span_to_snippet(base_expr.span)) + { + // `foo[-1]` to `foo[foo.len() - 1]` + err.span_suggestion_verbose( + span.shrink_to_lo(), + &format!( + "to access an element starting from the end of the `{}`, compute the index", + ty, + ), + format!("{}.len() ", snippet), + Applicability::MachineApplicable, + ); + } + err.emit(); + Some((self.tcx.ty_error(), self.tcx.ty_error())) + } + /// To type-check `base_expr[index_expr]`, we progressively autoderef /// (and otherwise adjust) `base_expr`, looking for a type which either /// supports builtin indexing or overloaded indexing. @@ -76,7 +106,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { base_expr: &hir::Expr<'_>, autoderef: &Autoderef<'a, 'tcx>, index_ty: Ty<'tcx>, - idx_expr: &hir::Expr<'_>, + index_expr: &hir::Expr<'_>, ) -> Option<(/*index type*/ Ty<'tcx>, /*element type*/ Ty<'tcx>)> { let adjusted_ty = self.structurally_resolved_type(autoderef.span(), autoderef.final_ty(false)); @@ -86,49 +116,23 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { expr, base_expr, adjusted_ty, index_ty ); - let negative_index = || { - let ty = self.resolve_vars_if_possible(adjusted_ty); - let mut err = self.tcx.sess.struct_span_err( - idx_expr.span, - &format!("negative integers cannot be used to index on a `{}`", ty), - ); - err.span_label( - idx_expr.span, - &format!("cannot use a negative integer for indexing on `{}`", ty), - ); - if let (hir::ExprKind::Path(..), Ok(snippet)) = - (&base_expr.kind, self.tcx.sess.source_map().span_to_snippet(base_expr.span)) - { - // `foo[-1]` to `foo[foo.len() - 1]` - err.span_suggestion_verbose( - idx_expr.span.shrink_to_lo(), - &format!( - "if you wanted to access an element starting from the end of the `{}`, you \ - must compute it", - ty, - ), - format!("{}.len() ", snippet), - Applicability::MachineApplicable, - ); - } - err.emit(); - Some((self.tcx.ty_error(), self.tcx.ty_error())) - }; if let hir::ExprKind::Unary( hir::UnOp::Neg, hir::Expr { kind: hir::ExprKind::Lit(hir::Lit { node: ast::LitKind::Int(..), .. }), .. }, - ) = idx_expr.kind + ) = index_expr.kind { match adjusted_ty.kind() { ty::Adt(ty::AdtDef { did, .. }, _) if self.tcx.is_diagnostic_item(sym::vec_type, *did) => { - return negative_index(); + return self.negative_index(adjusted_ty, index_expr.span, base_expr); + } + ty::Slice(_) | ty::Array(_, _) => { + return self.negative_index(adjusted_ty, index_expr.span, base_expr); } - ty::Slice(_) | ty::Array(_, _) => return negative_index(), _ => {} } } diff --git a/src/test/ui/suggestions/negative-literal-index.stderr b/src/test/ui/suggestions/negative-literal-index.stderr index f5ea980048b5b..2b51bf7b7ce87 100644 --- a/src/test/ui/suggestions/negative-literal-index.stderr +++ b/src/test/ui/suggestions/negative-literal-index.stderr @@ -4,7 +4,7 @@ error: negative integers cannot be used to index on a `Vec<{integer}>` LL | x[-1]; | ^^ cannot use a negative integer for indexing on `Vec<{integer}>` | -help: if you wanted to access an element starting from the end of the `Vec<{integer}>`, you must compute it +help: to access an element starting from the end of the `Vec<{integer}>`, compute the index | LL | x[x.len() -1]; | +++++++ @@ -15,7 +15,7 @@ error: negative integers cannot be used to index on a `[{integer}; 3]` LL | x[-1]; | ^^ cannot use a negative integer for indexing on `[{integer}; 3]` | -help: if you wanted to access an element starting from the end of the `[{integer}; 3]`, you must compute it +help: to access an element starting from the end of the `[{integer}; 3]`, compute the index | LL | x[x.len() -1]; | +++++++ @@ -26,7 +26,7 @@ error: negative integers cannot be used to index on a `[{integer}; 3]` LL | x[-1]; | ^^ cannot use a negative integer for indexing on `[{integer}; 3]` | -help: if you wanted to access an element starting from the end of the `[{integer}; 3]`, you must compute it +help: to access an element starting from the end of the `[{integer}; 3]`, compute the index | LL | x[x.len() -1]; | +++++++ From 87a4a79554b03e2002691b8734d23fb7a556fab1 Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Tue, 28 Sep 2021 16:24:13 +0000 Subject: [PATCH 6/9] Pick one possible lifetime in case there are multiple choices --- .../src/region_infer/opaque_types.rs | 17 ++++++++++++++++- .../ordinary-bounds-unrelated.nll.stderr | 6 +++++- .../ordinary-bounds-unsuited.nll.stderr | 6 +++++- 3 files changed, 26 insertions(+), 3 deletions(-) diff --git a/compiler/rustc_borrowck/src/region_infer/opaque_types.rs b/compiler/rustc_borrowck/src/region_infer/opaque_types.rs index b35e76b96ad91..4eb7be542e7a1 100644 --- a/compiler/rustc_borrowck/src/region_infer/opaque_types.rs +++ b/compiler/rustc_borrowck/src/region_infer/opaque_types.rs @@ -124,7 +124,22 @@ impl<'tcx> RegionInferenceContext<'tcx> { ty::ReVar(vid) => { // Find something that we can name let upper_bound = self.approx_universal_upper_bound(vid); - self.definitions[upper_bound].external_name.unwrap_or(region) + let upper_bound = &self.definitions[upper_bound]; + match upper_bound.external_name { + Some(reg) => reg, + None => { + // Nothing exact found, so we pick the first one that we find. + let scc = self.constraint_sccs.scc(vid); + for vid in self.rev_scc_graph.as_ref().unwrap().upper_bounds(scc) { + match self.definitions[vid].external_name { + None => {} + Some(&ty::ReStatic) => {} + Some(region) => return region, + } + } + region + } + } } _ => region, }) diff --git a/src/test/ui/impl-trait/multiple-lifetimes/ordinary-bounds-unrelated.nll.stderr b/src/test/ui/impl-trait/multiple-lifetimes/ordinary-bounds-unrelated.nll.stderr index 8cf89f164b16d..0fe9b06355f0b 100644 --- a/src/test/ui/impl-trait/multiple-lifetimes/ordinary-bounds-unrelated.nll.stderr +++ b/src/test/ui/impl-trait/multiple-lifetimes/ordinary-bounds-unrelated.nll.stderr @@ -4,7 +4,11 @@ error[E0700]: hidden type for `impl Trait` captures lifetime that does not appea LL | fn upper_bounds<'a, 'b, 'c, 'd, 'e>(a: Ordinary<'a>, b: Ordinary<'b>) -> impl Trait<'d, 'e> | ^^^^^^^^^^^^^^^^^^ | - = note: hidden type `Ordinary<'_>` captures lifetime '_#9r +note: hidden type `Ordinary<'b>` captures the lifetime `'b` as defined on the function body at 16:21 + --> $DIR/ordinary-bounds-unrelated.rs:16:21 + | +LL | fn upper_bounds<'a, 'b, 'c, 'd, 'e>(a: Ordinary<'a>, b: Ordinary<'b>) -> impl Trait<'d, 'e> + | ^^ error: aborting due to previous error diff --git a/src/test/ui/impl-trait/multiple-lifetimes/ordinary-bounds-unsuited.nll.stderr b/src/test/ui/impl-trait/multiple-lifetimes/ordinary-bounds-unsuited.nll.stderr index 1bcb28120ed1b..6de77523db577 100644 --- a/src/test/ui/impl-trait/multiple-lifetimes/ordinary-bounds-unsuited.nll.stderr +++ b/src/test/ui/impl-trait/multiple-lifetimes/ordinary-bounds-unsuited.nll.stderr @@ -4,7 +4,11 @@ error[E0700]: hidden type for `impl Trait` captures lifetime that does not appea LL | fn upper_bounds<'a, 'b>(a: Ordinary<'a>, b: Ordinary<'b>) -> impl Trait<'a, 'b> | ^^^^^^^^^^^^^^^^^^ | - = note: hidden type `Ordinary<'_>` captures lifetime '_#6r +note: hidden type `Ordinary<'b>` captures the lifetime `'b` as defined on the function body at 18:21 + --> $DIR/ordinary-bounds-unsuited.rs:18:21 + | +LL | fn upper_bounds<'a, 'b>(a: Ordinary<'a>, b: Ordinary<'b>) -> impl Trait<'a, 'b> + | ^^ error: aborting due to previous error From e1a9ecca2642a21eb56739d0360066bf27c86342 Mon Sep 17 00:00:00 2001 From: jackh726 Date: Tue, 28 Sep 2021 17:15:28 -0400 Subject: [PATCH 7/9] Cleanup lower_generics_mut and make span be the bound itself, not the type --- compiler/rustc_ast_lowering/src/item.rs | 63 +++++++++++++++---------- compiler/rustc_ast_lowering/src/lib.rs | 2 +- compiler/rustc_resolve/src/lib.rs | 2 +- src/test/ui/maybe-bounds-where.stderr | 20 ++++---- 4 files changed, 50 insertions(+), 37 deletions(-) diff --git a/compiler/rustc_ast_lowering/src/item.rs b/compiler/rustc_ast_lowering/src/item.rs index b7497c713f3df..9fd7ca7c09e7f 100644 --- a/compiler/rustc_ast_lowering/src/item.rs +++ b/compiler/rustc_ast_lowering/src/item.rs @@ -1356,32 +1356,45 @@ impl<'hir> LoweringContext<'_, 'hir> { // keep track of the Span info. Now, `add_implicitly_sized` in `AstConv` checks both param bounds and // where clauses for `?Sized`. for pred in &generics.where_clause.predicates { - if let WherePredicate::BoundPredicate(ref bound_pred) = *pred { - 'next_bound: for bound in &bound_pred.bounds { - if let GenericBound::Trait(_, TraitBoundModifier::Maybe) = *bound { - // Check if the where clause type is a plain type parameter. - match self - .resolver - .get_partial_res(bound_pred.bounded_ty.id) - .map(|d| (d.base_res(), d.unresolved_segments())) - { - Some((Res::Def(DefKind::TyParam, def_id), 0)) - if bound_pred.bound_generic_params.is_empty() => - { - for param in &generics.params { - if def_id == self.resolver.local_def_id(param.id).to_def_id() { - continue 'next_bound; - } - } - } - _ => {} - } - self.diagnostic().span_err( - bound_pred.bounded_ty.span, - "`?Trait` bounds are only permitted at the \ - point where a type parameter is declared", - ); + let bound_pred = match *pred { + WherePredicate::BoundPredicate(ref bound_pred) => bound_pred, + _ => continue, + }; + let compute_is_param = || { + // Check if the where clause type is a plain type parameter. + match self + .resolver + .get_partial_res(bound_pred.bounded_ty.id) + .map(|d| (d.base_res(), d.unresolved_segments())) + { + Some((Res::Def(DefKind::TyParam, def_id), 0)) + if bound_pred.bound_generic_params.is_empty() => + { + generics + .params + .iter() + .find(|p| def_id == self.resolver.local_def_id(p.id).to_def_id()) + .is_some() } + // Either the `bounded_ty` is not a plain type parameter, or + // it's not found in the generic type parameters list. + _ => false, + } + }; + // We only need to compute this once per `WherePredicate`, but don't + // need to compute this at all unless there is a Maybe bound. + let mut is_param: Option = None; + for bound in &bound_pred.bounds { + if !matches!(*bound, GenericBound::Trait(_, TraitBoundModifier::Maybe)) { + continue; + } + let is_param = *is_param.get_or_insert_with(compute_is_param); + if !is_param { + self.diagnostic().span_err( + bound.span(), + "`?Trait` bounds are only permitted at the \ + point where a type parameter is declared", + ); } } } diff --git a/compiler/rustc_ast_lowering/src/lib.rs b/compiler/rustc_ast_lowering/src/lib.rs index 8d731d7a57895..b81dff9958ec7 100644 --- a/compiler/rustc_ast_lowering/src/lib.rs +++ b/compiler/rustc_ast_lowering/src/lib.rs @@ -172,7 +172,7 @@ pub trait ResolverAstLowering { fn legacy_const_generic_args(&mut self, expr: &Expr) -> Option>; /// Obtains resolution for a `NodeId` with a single resolution. - fn get_partial_res(&mut self, id: NodeId) -> Option; + fn get_partial_res(&self, id: NodeId) -> Option; /// Obtains per-namespace resolutions for `use` statement with the given `NodeId`. fn get_import_res(&mut self, id: NodeId) -> PerNS>>; diff --git a/compiler/rustc_resolve/src/lib.rs b/compiler/rustc_resolve/src/lib.rs index d76ba80e42eab..65650780cc322 100644 --- a/compiler/rustc_resolve/src/lib.rs +++ b/compiler/rustc_resolve/src/lib.rs @@ -1132,7 +1132,7 @@ impl ResolverAstLowering for Resolver<'_> { self.legacy_const_generic_args(expr) } - fn get_partial_res(&mut self, id: NodeId) -> Option { + fn get_partial_res(&self, id: NodeId) -> Option { self.partial_res_map.get(&id).cloned() } diff --git a/src/test/ui/maybe-bounds-where.stderr b/src/test/ui/maybe-bounds-where.stderr index 2aa6a8a38223d..39bc1b88e56d7 100644 --- a/src/test/ui/maybe-bounds-where.stderr +++ b/src/test/ui/maybe-bounds-where.stderr @@ -1,32 +1,32 @@ error: `?Trait` bounds are only permitted at the point where a type parameter is declared - --> $DIR/maybe-bounds-where.rs:1:23 + --> $DIR/maybe-bounds-where.rs:1:28 | LL | struct S1(T) where (T): ?Sized; - | ^^^ + | ^^^^^^ error: `?Trait` bounds are only permitted at the point where a type parameter is declared - --> $DIR/maybe-bounds-where.rs:4:23 + --> $DIR/maybe-bounds-where.rs:4:27 | LL | struct S2(T) where u8: ?Sized; - | ^^ + | ^^^^^^ error: `?Trait` bounds are only permitted at the point where a type parameter is declared - --> $DIR/maybe-bounds-where.rs:7:23 + --> $DIR/maybe-bounds-where.rs:7:35 | LL | struct S3(T) where &'static T: ?Sized; - | ^^^^^^^^^^ + | ^^^^^^ error: `?Trait` bounds are only permitted at the point where a type parameter is declared - --> $DIR/maybe-bounds-where.rs:12:31 + --> $DIR/maybe-bounds-where.rs:12:34 | LL | struct S4(T) where for<'a> T: ?Trait<'a>; - | ^ + | ^^^^^^^^^^ error: `?Trait` bounds are only permitted at the point where a type parameter is declared - --> $DIR/maybe-bounds-where.rs:21:18 + --> $DIR/maybe-bounds-where.rs:21:21 | LL | fn f() where T: ?Sized {} - | ^ + | ^^^^^^ warning: default bound relaxed for a type parameter, but this does nothing because the given bound is not a default; only `?Sized` is supported --> $DIR/maybe-bounds-where.rs:12:11 From c20eebac2db594ee4370eeafb9b0436a0ece0cde Mon Sep 17 00:00:00 2001 From: Wesley Wiser Date: Thu, 30 Sep 2021 10:03:12 -0400 Subject: [PATCH 8/9] Update `llvm` submodule to fix function name mangling on x86 Windows Fixes #89307 --- src/llvm-project | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/llvm-project b/src/llvm-project index cba558df777a0..522c3e3d9c097 160000 --- a/src/llvm-project +++ b/src/llvm-project @@ -1 +1 @@ -Subproject commit cba558df777a045b5657d56c29944e9e8fd3a776 +Subproject commit 522c3e3d9c097b53ede7682cc28544b461597b20 From bec19a72dda2c9e3251ed606e1a017d957b2e8f9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Rakic?= Date: Thu, 30 Sep 2021 22:12:06 +0200 Subject: [PATCH 9/9] add regression test for issue 89119 --- .../issue_89119_intercrate_caching.rs | 60 +++++++++++++++++++ src/test/ui/traits/issue-89119.rs | 11 ++++ 2 files changed, 71 insertions(+) create mode 100644 src/test/ui/traits/auxiliary/issue_89119_intercrate_caching.rs create mode 100644 src/test/ui/traits/issue-89119.rs diff --git a/src/test/ui/traits/auxiliary/issue_89119_intercrate_caching.rs b/src/test/ui/traits/auxiliary/issue_89119_intercrate_caching.rs new file mode 100644 index 0000000000000..769e897316327 --- /dev/null +++ b/src/test/ui/traits/auxiliary/issue_89119_intercrate_caching.rs @@ -0,0 +1,60 @@ +// This is the auxiliary crate for the regression test for issue #89119, minimized +// from `zvariant-2.8.0`. + +use std::convert::TryFrom; +use std::borrow::Cow; + +pub struct Str<'a>(Cow<'a, str>); +impl<'a> Str<'a> { + pub fn to_owned(&self) -> Str<'static> { + todo!() + } +} + +pub enum Value<'a> { + Str(Str<'a>), + Value(Box>), +} +impl<'a> Value<'a> { + pub fn to_owned(&self) -> Value<'static> { + match self { + Value::Str(v) => Value::Str(v.to_owned()), + Value::Value(v) => { + let o = OwnedValue::from(&**v); + Value::Value(Box::new(o.into_inner())) + } + } + } +} + +struct OwnedValue(Value<'static>); +impl OwnedValue { + pub(crate) fn into_inner(self) -> Value<'static> { + todo!() + } +} +impl<'a, T> TryFrom for Vec +where + T: TryFrom, Error = ()>, +{ + type Error = (); + fn try_from(_: OwnedValue) -> Result { + todo!() + } +} +impl TryFrom for Vec { + type Error = (); + fn try_from(_: OwnedValue) -> Result { + todo!() + } +} +impl<'a> From> for OwnedValue { + fn from(_: Value<'a>) -> Self { + todo!() + } +} +impl<'a> From<&Value<'a>> for OwnedValue { + fn from(_: &Value<'a>) -> Self { + todo!() + } +} diff --git a/src/test/ui/traits/issue-89119.rs b/src/test/ui/traits/issue-89119.rs new file mode 100644 index 0000000000000..170f69915e2ab --- /dev/null +++ b/src/test/ui/traits/issue-89119.rs @@ -0,0 +1,11 @@ +// This is a regression test for issue #89119: an issue in intercrate mode caching. +// +// It requires multiple crates, of course, but the bug is triggered by the code in the dependency, +// not the main crate. This is why this file is empty. +// +// The auxiliary crate used in the test contains the code minimized from `zvariant-2.8.0`. + +// check-pass +// aux-build: issue_89119_intercrate_caching.rs + +fn main() {}