From bb75c20763f58d06a6fe73e5c372d0cebb6e6253 Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Wed, 11 Dec 2019 21:58:00 +0300 Subject: [PATCH 01/12] resolve: Always resolve visibilities on impl items --- src/librustc_resolve/build_reduced_graph.rs | 24 +++++++++--------- .../ui/resolve/impl-items-vis-unresolved.rs | 25 +++++++++++++++++++ .../resolve/impl-items-vis-unresolved.stderr | 9 +++++++ 3 files changed, 46 insertions(+), 12 deletions(-) create mode 100644 src/test/ui/resolve/impl-items-vis-unresolved.rs create mode 100644 src/test/ui/resolve/impl-items-vis-unresolved.stderr diff --git a/src/librustc_resolve/build_reduced_graph.rs b/src/librustc_resolve/build_reduced_graph.rs index 648c5104b1af7..c7942d8abb95a 100644 --- a/src/librustc_resolve/build_reduced_graph.rs +++ b/src/librustc_resolve/build_reduced_graph.rs @@ -658,8 +658,6 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> { self.r.define(parent, ident, TypeNS, imported_binding); } - ItemKind::GlobalAsm(..) => {} - ItemKind::Mod(..) if ident.name == kw::Invalid => {} // Crate root ItemKind::Mod(..) => { @@ -678,9 +676,6 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> { self.parent_scope.module = module; } - // Handled in `rustc_metadata::{native_libs,link_args}` - ItemKind::ForeignMod(..) => {} - // These items live in the value namespace. ItemKind::Static(..) => { let res = Res::Def(DefKind::Static, self.r.definitions.local_def_id(item.id)); @@ -781,12 +776,6 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> { self.insert_field_names(item_def_id, field_names); } - ItemKind::Impl(.., ref impl_items) => { - for impl_item in impl_items { - self.resolve_visibility(&impl_item.vis); - } - } - ItemKind::Trait(..) => { let def_id = self.r.definitions.local_def_id(item.id); @@ -801,6 +790,9 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> { self.parent_scope.module = module; } + // These items do not add names to modules. + ItemKind::Impl(..) | ItemKind::ForeignMod(..) | ItemKind::GlobalAsm(..) => {} + ItemKind::MacroDef(..) | ItemKind::Mac(_) => unreachable!(), } } @@ -1134,7 +1126,6 @@ macro_rules! method { } impl<'a, 'b> Visitor<'b> for BuildReducedGraphVisitor<'a, 'b> { - method!(visit_impl_item: ast::ImplItem, ast::ImplItemKind::Macro, walk_impl_item); method!(visit_expr: ast::Expr, ast::ExprKind::Mac, walk_expr); method!(visit_pat: ast::Pat, ast::PatKind::Mac, walk_pat); method!(visit_ty: ast::Ty, ast::TyKind::Mac, walk_ty); @@ -1218,6 +1209,15 @@ impl<'a, 'b> Visitor<'b> for BuildReducedGraphVisitor<'a, 'b> { visit::walk_trait_item(self, item); } + fn visit_impl_item(&mut self, item: &'b ast::ImplItem) { + if let ast::ImplItemKind::Macro(..) = item.kind { + self.visit_invoc(item.id); + } else { + self.resolve_visibility(&item.vis); + visit::walk_impl_item(self, item); + } + } + fn visit_token(&mut self, t: Token) { if let token::Interpolated(nt) = t.kind { if let token::NtExpr(ref expr) = *nt { diff --git a/src/test/ui/resolve/impl-items-vis-unresolved.rs b/src/test/ui/resolve/impl-items-vis-unresolved.rs new file mode 100644 index 0000000000000..9b4fe498239b6 --- /dev/null +++ b/src/test/ui/resolve/impl-items-vis-unresolved.rs @@ -0,0 +1,25 @@ +// Visibilities on impl items expanded from macros are resolved (issue #64705). + +macro_rules! perftools_inline { + ($($item:tt)*) => ( + $($item)* + ); +} + +mod state { + pub struct RawFloatState; + impl RawFloatState { + perftools_inline! { + pub(super) fn new() {} // OK + } + } +} + +pub struct RawFloatState; +impl RawFloatState { + perftools_inline! { + pub(super) fn new() {} //~ ERROR failed to resolve: there are too many initial `super`s + } +} + +fn main() {} diff --git a/src/test/ui/resolve/impl-items-vis-unresolved.stderr b/src/test/ui/resolve/impl-items-vis-unresolved.stderr new file mode 100644 index 0000000000000..8e285e5312400 --- /dev/null +++ b/src/test/ui/resolve/impl-items-vis-unresolved.stderr @@ -0,0 +1,9 @@ +error[E0433]: failed to resolve: there are too many initial `super`s. + --> $DIR/impl-items-vis-unresolved.rs:21:13 + | +LL | pub(super) fn new() {} + | ^^^^^ there are too many initial `super`s. + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0433`. From dc17e5defda10f12f2a15f37ab63304877e9306b Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Sat, 7 Dec 2019 00:03:58 +0300 Subject: [PATCH 02/12] resolve: Resolve visibilities on fields with non-builtin attributes --- src/librustc_resolve/build_reduced_graph.rs | 21 ++++++++++++--- .../field-attributes-vis-unresolved.rs | 26 +++++++++++++++++++ .../field-attributes-vis-unresolved.stderr | 22 ++++++++++++++++ 3 files changed, 65 insertions(+), 4 deletions(-) create mode 100644 src/test/ui/attributes/field-attributes-vis-unresolved.rs create mode 100644 src/test/ui/attributes/field-attributes-vis-unresolved.stderr diff --git a/src/librustc_resolve/build_reduced_graph.rs b/src/librustc_resolve/build_reduced_graph.rs index c7942d8abb95a..55d2345f200b8 100644 --- a/src/librustc_resolve/build_reduced_graph.rs +++ b/src/librustc_resolve/build_reduced_graph.rs @@ -194,6 +194,14 @@ impl<'a> AsMut> for BuildReducedGraphVisitor<'a, '_> { impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> { fn resolve_visibility(&mut self, vis: &ast::Visibility) -> ty::Visibility { + self.resolve_visibility_speculative(vis, false) + } + + fn resolve_visibility_speculative( + &mut self, + vis: &ast::Visibility, + speculative: bool, + ) -> ty::Visibility { let parent_scope = &self.parent_scope; match vis.node { ast::VisibilityKind::Public => ty::Visibility::Public, @@ -241,13 +249,15 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> { &segments, Some(TypeNS), parent_scope, - true, + !speculative, path.span, CrateLint::SimplePath(id), ) { PathResult::Module(ModuleOrUniformRoot::Module(module)) => { let res = module.res().expect("visibility resolved to unnamed block"); - self.r.record_partial_res(id, PartialRes::new(res)); + if !speculative { + self.r.record_partial_res(id, PartialRes::new(res)); + } if module.is_normal() { if res == Res::Err { ty::Visibility::Public @@ -742,7 +752,10 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> { // Record field names for error reporting. let field_names = struct_def.fields().iter().map(|field| { - let field_vis = self.resolve_visibility(&field.vis); + // NOTE: The field may be an expansion placeholder, but expansion sets correct + // visibilities for unnamed field placeholders specifically, so the constructor + // visibility should still be determined correctly. + let field_vis = self.resolve_visibility_speculative(&field.vis, true); if ctor_vis.is_at_least(field_vis, &*self.r) { ctor_vis = field_vis; } @@ -769,7 +782,6 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> { // Record field names for error reporting. let field_names = vdata.fields().iter().map(|field| { - self.resolve_visibility(&field.vis); respan(field.span, field.ident.map_or(kw::Invalid, |ident| ident.name)) }).collect(); let item_def_id = self.r.definitions.local_def_id(item.id); @@ -1279,6 +1291,7 @@ impl<'a, 'b> Visitor<'b> for BuildReducedGraphVisitor<'a, 'b> { if sf.is_placeholder { self.visit_invoc(sf.id); } else { + self.resolve_visibility(&sf.vis); visit::walk_struct_field(self, sf); } } diff --git a/src/test/ui/attributes/field-attributes-vis-unresolved.rs b/src/test/ui/attributes/field-attributes-vis-unresolved.rs new file mode 100644 index 0000000000000..36f3feb973d6d --- /dev/null +++ b/src/test/ui/attributes/field-attributes-vis-unresolved.rs @@ -0,0 +1,26 @@ +// Non-builtin attributes do not mess with field visibility resolution (issue #67006). + +mod internal { + struct S { + #[rustfmt::skip] + pub(in crate::internal) field: u8 // OK + } + + struct Z( + #[rustfmt::skip] + pub(in crate::internal) u8 // OK + ); +} + +struct S { + #[rustfmt::skip] + pub(in nonexistent) field: u8 //~ ERROR failed to resolve +} + +struct Z( + #[rustfmt::skip] + pub(in nonexistent) u8 //~ ERROR failed to resolve + //~| ERROR cannot determine resolution for the visibility +); + +fn main() {} diff --git a/src/test/ui/attributes/field-attributes-vis-unresolved.stderr b/src/test/ui/attributes/field-attributes-vis-unresolved.stderr new file mode 100644 index 0000000000000..8c283e9bcb991 --- /dev/null +++ b/src/test/ui/attributes/field-attributes-vis-unresolved.stderr @@ -0,0 +1,22 @@ +error[E0578]: cannot determine resolution for the visibility + --> $DIR/field-attributes-vis-unresolved.rs:22:12 + | +LL | pub(in nonexistent) u8 + | ^^^^^^^^^^^ + +error[E0433]: failed to resolve: maybe a missing crate `nonexistent`? + --> $DIR/field-attributes-vis-unresolved.rs:17:12 + | +LL | pub(in nonexistent) field: u8 + | ^^^^^^^^^^^ maybe a missing crate `nonexistent`? + +error[E0433]: failed to resolve: maybe a missing crate `nonexistent`? + --> $DIR/field-attributes-vis-unresolved.rs:22:12 + | +LL | pub(in nonexistent) u8 + | ^^^^^^^^^^^ maybe a missing crate `nonexistent`? + +error: aborting due to 3 previous errors + +Some errors have detailed explanations: E0433, E0578. +For more information about an error, try `rustc --explain E0433`. From 0b8fb216e907d37d914ab80a885ceb68201c0abf Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Thu, 5 Dec 2019 04:36:53 +0100 Subject: [PATCH 03/12] E0023: handle expected != pat-tup-type --- src/librustc_typeck/check/demand.rs | 16 +++++++++++--- src/librustc_typeck/check/pat.rs | 21 ++++++++++++------ ...67037-pat-tup-scrut-ty-diff-less-fields.rs | 21 ++++++++++++++++++ ...7-pat-tup-scrut-ty-diff-less-fields.stderr | 22 +++++++++++++++++++ 4 files changed, 70 insertions(+), 10 deletions(-) create mode 100644 src/test/ui/issues/issue-67037-pat-tup-scrut-ty-diff-less-fields.rs create mode 100644 src/test/ui/issues/issue-67037-pat-tup-scrut-ty-diff-less-fields.stderr diff --git a/src/librustc_typeck/check/demand.rs b/src/librustc_typeck/check/demand.rs index b4e07e4a0dfb4..037531515c1e0 100644 --- a/src/librustc_typeck/check/demand.rs +++ b/src/librustc_typeck/check/demand.rs @@ -65,13 +65,13 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { } } - pub fn demand_eqtype_pat( + pub fn demand_eqtype_pat_diag( &self, cause_span: Span, expected: Ty<'tcx>, actual: Ty<'tcx>, match_expr_span: Option, - ) { + ) -> Option> { let cause = if let Some(span) = match_expr_span { self.cause( cause_span, @@ -80,9 +80,19 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { } else { self.misc(cause_span) }; - self.demand_eqtype_with_origin(&cause, expected, actual).map(|mut err| err.emit()); + self.demand_eqtype_with_origin(&cause, expected, actual) } + pub fn demand_eqtype_pat( + &self, + cause_span: Span, + expected: Ty<'tcx>, + actual: Ty<'tcx>, + match_expr_span: Option, + ) { + self.demand_eqtype_pat_diag(cause_span, expected, actual, match_expr_span) + .map(|mut err| err.emit()); + } pub fn demand_coerce(&self, expr: &hir::Expr, diff --git a/src/librustc_typeck/check/pat.rs b/src/librustc_typeck/check/pat.rs index 950ae7c1d62e2..ca2ec1ea4492f 100644 --- a/src/librustc_typeck/check/pat.rs +++ b/src/librustc_typeck/check/pat.rs @@ -669,7 +669,10 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { let pat_ty = pat_ty.fn_sig(tcx).output(); let pat_ty = pat_ty.no_bound_vars().expect("expected fn type"); - self.demand_eqtype_pat(pat.span, expected, pat_ty, match_arm_pat_span); + // Type-check the tuple struct pattern against the expected type. + let diag = self.demand_eqtype_pat_diag(pat.span, expected, pat_ty, match_arm_pat_span); + let had_err = diag.is_some(); + diag.map(|mut err| err.emit()); // Type-check subpatterns. if subpats.len() == variant.fields.len() @@ -687,7 +690,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { } } else { // Pattern has wrong number of fields. - self.e0023(pat.span, res, qpath, subpats, &variant.fields, expected); + self.e0023(pat.span, res, qpath, subpats, &variant.fields, expected, had_err); on_error(); return tcx.types.err; } @@ -700,8 +703,9 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { res: Res, qpath: &hir::QPath, subpats: &'tcx [P], - fields: &[ty::FieldDef], - expected: Ty<'tcx> + fields: &'tcx [ty::FieldDef], + expected: Ty<'tcx>, + had_err: bool, ) { let subpats_ending = pluralise!(subpats.len()); let fields_ending = pluralise!(fields.len()); @@ -729,9 +733,12 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { // More generally, the expected type wants a tuple variant with one field of an // N-arity-tuple, e.g., `V_i((p_0, .., p_N))`. Meanwhile, the user supplied a pattern // with the subpatterns directly in the tuple variant pattern, e.g., `V_i(p_0, .., p_N)`. - let missing_parenthesis = match expected.kind { - ty::Adt(_, substs) if fields.len() == 1 => { - let field_ty = fields[0].ty(self.tcx, substs); + let missing_parenthesis = match (&expected.kind, fields, had_err) { + // #67037: only do this if we could sucessfully type-check the expected type against + // the tuple struct pattern. Otherwise the substs could get out of range on e.g., + // `let P() = U;` where `P != U` with `struct P(T);`. + (ty::Adt(_, substs), [field], false) => { + let field_ty = self.field_ty(pat_span, field, substs); match field_ty.kind { ty::Tuple(_) => field_ty.tuple_fields().count() == subpats.len(), _ => false, diff --git a/src/test/ui/issues/issue-67037-pat-tup-scrut-ty-diff-less-fields.rs b/src/test/ui/issues/issue-67037-pat-tup-scrut-ty-diff-less-fields.rs new file mode 100644 index 0000000000000..44bd645598ae0 --- /dev/null +++ b/src/test/ui/issues/issue-67037-pat-tup-scrut-ty-diff-less-fields.rs @@ -0,0 +1,21 @@ +// Regression test for #67037. +// +// In type checking patterns, E0023 occurs when the tuple pattern and the expected +// tuple pattern have different number of fields. For example, as below, `P()`, +// the tuple struct pattern, has 0 fields, but requires 1 field. +// +// In emitting E0023, we try to see if this is a case of e.g., `Some(a, b, c)` but where +// the scrutinee was of type `Some((a, b, c))`, and suggest that parenthesis be added. +// +// However, we did not account for the expected type being different than the tuple pattern type. +// This caused an issue when the tuple pattern type (`P`) was generic. +// Specifically, we tried deriving the 0th field's type using the `substs` of the expected type. +// When attempting to substitute `T`, there was no such substitution, so "out of range" occured. + +struct U {} // 0 type parameters offered +struct P(T); // 1 type parameter wanted + +fn main() { + let P() = U {}; //~ ERROR mismatched types + //~^ ERROR this pattern has 0 fields, but the corresponding tuple struct has 1 field +} diff --git a/src/test/ui/issues/issue-67037-pat-tup-scrut-ty-diff-less-fields.stderr b/src/test/ui/issues/issue-67037-pat-tup-scrut-ty-diff-less-fields.stderr new file mode 100644 index 0000000000000..36540820b2567 --- /dev/null +++ b/src/test/ui/issues/issue-67037-pat-tup-scrut-ty-diff-less-fields.stderr @@ -0,0 +1,22 @@ +error[E0308]: mismatched types + --> $DIR/issue-67037-pat-tup-scrut-ty-diff-less-fields.rs:19:9 + | +LL | let P() = U {}; + | ^^^ expected struct `U`, found struct `P` + | + = note: expected type `U` + found type `P<_>` + +error[E0023]: this pattern has 0 fields, but the corresponding tuple struct has 1 field + --> $DIR/issue-67037-pat-tup-scrut-ty-diff-less-fields.rs:19:9 + | +LL | struct P(T); // 1 type parameter wanted + | --------------- tuple struct defined here +... +LL | let P() = U {}; + | ^^^ expected 1 field, found 0 + +error: aborting due to 2 previous errors + +Some errors have detailed explanations: E0023, E0308. +For more information about an error, try `rustc --explain E0023`. From aa8c758a51354b116123ad1e6b50286bd1ee8fc6 Mon Sep 17 00:00:00 2001 From: Georg Semmler Date: Tue, 3 Dec 2019 12:35:09 +0100 Subject: [PATCH 04/12] Fix #66295 --- src/librustc_lint/unused.rs | 4 +++- ...775-nested-macro-unnecessary-parens-arg.rs | 3 --- ...nested-macro-unnecessary-parens-arg.stderr | 15 ------------- src/test/ui/lint/lint-unnecessary-parens.rs | 9 ++++++++ .../ui/lint/lint-unnecessary-parens.stderr | 22 +++++++++---------- 5 files changed, 23 insertions(+), 30 deletions(-) delete mode 100644 src/test/ui/lint/issue-47775-nested-macro-unnecessary-parens-arg.stderr diff --git a/src/librustc_lint/unused.rs b/src/librustc_lint/unused.rs index af43030d0f2ae..5b29cff9dac0d 100644 --- a/src/librustc_lint/unused.rs +++ b/src/librustc_lint/unused.rs @@ -374,7 +374,9 @@ impl UnusedParens { match value.kind { ast::ExprKind::Paren(ref inner) => { if !Self::is_expr_parens_necessary(inner, followed_by_block) && - value.attrs.is_empty() { + value.attrs.is_empty() && + !value.span.from_expansion() + { let expr_text = if let Ok(snippet) = cx.sess().source_map() .span_to_snippet(value.span) { snippet diff --git a/src/test/ui/lint/issue-47775-nested-macro-unnecessary-parens-arg.rs b/src/test/ui/lint/issue-47775-nested-macro-unnecessary-parens-arg.rs index 47063a7c26786..e36a8468815ce 100644 --- a/src/test/ui/lint/issue-47775-nested-macro-unnecessary-parens-arg.rs +++ b/src/test/ui/lint/issue-47775-nested-macro-unnecessary-parens-arg.rs @@ -17,10 +17,7 @@ macro_rules! the_worship_the_heart_lifts_above { macro_rules! and_the_heavens_reject_not { () => { - // ↓ But let's test that we still lint for unused parens around - // function args inside of simple, one-deep macros. #[allow(dead_code)] fn the_night_for_the_morrow() -> Option { Some((2)) } - //~^ WARN unnecessary parentheses around function argument } } diff --git a/src/test/ui/lint/issue-47775-nested-macro-unnecessary-parens-arg.stderr b/src/test/ui/lint/issue-47775-nested-macro-unnecessary-parens-arg.stderr deleted file mode 100644 index 57cdcd70e9db4..0000000000000 --- a/src/test/ui/lint/issue-47775-nested-macro-unnecessary-parens-arg.stderr +++ /dev/null @@ -1,15 +0,0 @@ -warning: unnecessary parentheses around function argument - --> $DIR/issue-47775-nested-macro-unnecessary-parens-arg.rs:22:83 - | -LL | #[allow(dead_code)] fn the_night_for_the_morrow() -> Option { Some((2)) } - | ^^^ help: remove these parentheses -... -LL | and_the_heavens_reject_not!(); - | ------------------------------ in this macro invocation - | -note: lint level defined here - --> $DIR/issue-47775-nested-macro-unnecessary-parens-arg.rs:3:9 - | -LL | #![warn(unused_parens)] - | ^^^^^^^^^^^^^ - diff --git a/src/test/ui/lint/lint-unnecessary-parens.rs b/src/test/ui/lint/lint-unnecessary-parens.rs index 9f42b855a870d..12ffb6d3c6655 100644 --- a/src/test/ui/lint/lint-unnecessary-parens.rs +++ b/src/test/ui/lint/lint-unnecessary-parens.rs @@ -25,6 +25,12 @@ fn passes_unused_parens_lint() -> &'static (dyn Trait) { panic!() } +macro_rules! baz { + ($($foo:expr),+) => { + ($($foo),*) + } +} + fn main() { foo(); bar((true)); //~ ERROR unnecessary parentheses around function argument @@ -55,4 +61,7 @@ fn main() { let mut _a = (0); //~ ERROR unnecessary parentheses around assigned value _a = (0); //~ ERROR unnecessary parentheses around assigned value _a += (1); //~ ERROR unnecessary parentheses around assigned value + + let _a = baz!(3, 4); + let _b = baz!(3); } diff --git a/src/test/ui/lint/lint-unnecessary-parens.stderr b/src/test/ui/lint/lint-unnecessary-parens.stderr index adc1069b64d62..541ae7aa4b54a 100644 --- a/src/test/ui/lint/lint-unnecessary-parens.stderr +++ b/src/test/ui/lint/lint-unnecessary-parens.stderr @@ -23,25 +23,25 @@ LL | fn unused_parens_around_return_type() -> (u32) { | ^^^^^ help: remove these parentheses error: unnecessary parentheses around function argument - --> $DIR/lint-unnecessary-parens.rs:30:9 + --> $DIR/lint-unnecessary-parens.rs:36:9 | LL | bar((true)); | ^^^^^^ help: remove these parentheses error: unnecessary parentheses around `if` condition - --> $DIR/lint-unnecessary-parens.rs:32:8 + --> $DIR/lint-unnecessary-parens.rs:38:8 | LL | if (true) {} | ^^^^^^ help: remove these parentheses error: unnecessary parentheses around `while` condition - --> $DIR/lint-unnecessary-parens.rs:33:11 + --> $DIR/lint-unnecessary-parens.rs:39:11 | LL | while (true) {} | ^^^^^^ help: remove these parentheses warning: denote infinite loops with `loop { ... }` - --> $DIR/lint-unnecessary-parens.rs:33:5 + --> $DIR/lint-unnecessary-parens.rs:39:5 | LL | while (true) {} | ^^^^^^^^^^^^ help: use `loop` @@ -49,43 +49,43 @@ LL | while (true) {} = note: `#[warn(while_true)]` on by default error: unnecessary parentheses around `match` head expression - --> $DIR/lint-unnecessary-parens.rs:35:11 + --> $DIR/lint-unnecessary-parens.rs:41:11 | LL | match (true) { | ^^^^^^ help: remove these parentheses error: unnecessary parentheses around `let` head expression - --> $DIR/lint-unnecessary-parens.rs:38:16 + --> $DIR/lint-unnecessary-parens.rs:44:16 | LL | if let 1 = (1) {} | ^^^ help: remove these parentheses error: unnecessary parentheses around `let` head expression - --> $DIR/lint-unnecessary-parens.rs:39:19 + --> $DIR/lint-unnecessary-parens.rs:45:19 | LL | while let 1 = (2) {} | ^^^ help: remove these parentheses error: unnecessary parentheses around method argument - --> $DIR/lint-unnecessary-parens.rs:53:24 + --> $DIR/lint-unnecessary-parens.rs:59:24 | LL | X { y: false }.foo((true)); | ^^^^^^ help: remove these parentheses error: unnecessary parentheses around assigned value - --> $DIR/lint-unnecessary-parens.rs:55:18 + --> $DIR/lint-unnecessary-parens.rs:61:18 | LL | let mut _a = (0); | ^^^ help: remove these parentheses error: unnecessary parentheses around assigned value - --> $DIR/lint-unnecessary-parens.rs:56:10 + --> $DIR/lint-unnecessary-parens.rs:62:10 | LL | _a = (0); | ^^^ help: remove these parentheses error: unnecessary parentheses around assigned value - --> $DIR/lint-unnecessary-parens.rs:57:11 + --> $DIR/lint-unnecessary-parens.rs:63:11 | LL | _a += (1); | ^^^ help: remove these parentheses From e49abf25d48453cdb295e81add198331cba2d797 Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Sat, 23 Nov 2019 14:16:38 +0300 Subject: [PATCH 05/12] def_collector: Do not forget to save indices of fields with multiple attributes --- src/librustc/hir/map/def_collector.rs | 16 +++++++--------- .../attributes/unnamed-field-attributes-dup.rs | 11 +++++++++++ 2 files changed, 18 insertions(+), 9 deletions(-) create mode 100644 src/test/ui/attributes/unnamed-field-attributes-dup.rs diff --git a/src/librustc/hir/map/def_collector.rs b/src/librustc/hir/map/def_collector.rs index e9970e30bf9e5..70dc2248e0f3b 100644 --- a/src/librustc/hir/map/def_collector.rs +++ b/src/librustc/hir/map/def_collector.rs @@ -75,15 +75,16 @@ impl<'a> DefCollector<'a> { } fn collect_field(&mut self, field: &'a StructField, index: Option) { + let index = |this: &Self| index.unwrap_or_else(|| { + let node_id = NodeId::placeholder_from_expn_id(this.expansion); + this.definitions.placeholder_field_index(node_id) + }); + if field.is_placeholder { + self.definitions.set_placeholder_field_index(field.id, index(self)); self.visit_macro_invoc(field.id); } else { - let name = field.ident.map(|ident| ident.name) - .or_else(|| index.map(sym::integer)) - .unwrap_or_else(|| { - let node_id = NodeId::placeholder_from_expn_id(self.expansion); - sym::integer(self.definitions.placeholder_field_indices[&node_id]) - }); + let name = field.ident.map_or_else(|| sym::integer(index(self)), |ident| ident.name); let def = self.create_def(field.id, DefPathData::ValueNs(name), field.span); self.with_parent(def, |this| visit::walk_struct_field(this, field)); } @@ -190,9 +191,6 @@ impl<'a> visit::Visitor<'a> for DefCollector<'a> { // and every such attribute expands into a single field after it's resolved. for (index, field) in data.fields().iter().enumerate() { self.collect_field(field, Some(index)); - if field.is_placeholder && field.ident.is_none() { - self.definitions.placeholder_field_indices.insert(field.id, index); - } } } diff --git a/src/test/ui/attributes/unnamed-field-attributes-dup.rs b/src/test/ui/attributes/unnamed-field-attributes-dup.rs new file mode 100644 index 0000000000000..7edfd0337945b --- /dev/null +++ b/src/test/ui/attributes/unnamed-field-attributes-dup.rs @@ -0,0 +1,11 @@ +// Duplicate non-builtin attributes can be used on unnamed fields. + +// check-pass + +struct S ( + #[rustfmt::skip] + #[rustfmt::skip] + u8 +); + +fn main() {} From a17c556bd088570c5c742eee8d36838dfa874878 Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Sat, 23 Nov 2019 16:54:24 +0300 Subject: [PATCH 06/12] expand: Fully preserve visibilities on unnamed fields with attributes --- src/librustc_resolve/build_reduced_graph.rs | 2 +- src/libsyntax_expand/expand.rs | 22 +++++++++++++++++-- src/libsyntax_expand/placeholders.rs | 5 +++-- .../unnamed-field-attributes-vis.rs | 11 ++++++++++ 4 files changed, 35 insertions(+), 5 deletions(-) create mode 100644 src/test/ui/attributes/unnamed-field-attributes-vis.rs diff --git a/src/librustc_resolve/build_reduced_graph.rs b/src/librustc_resolve/build_reduced_graph.rs index 55d2345f200b8..1bef810b24084 100644 --- a/src/librustc_resolve/build_reduced_graph.rs +++ b/src/librustc_resolve/build_reduced_graph.rs @@ -755,7 +755,7 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> { // NOTE: The field may be an expansion placeholder, but expansion sets correct // visibilities for unnamed field placeholders specifically, so the constructor // visibility should still be determined correctly. - let field_vis = self.resolve_visibility_speculative(&field.vis, true); + let field_vis = self.resolve_visibility(&field.vis); if ctor_vis.is_at_least(field_vis, &*self.r) { ctor_vis = field_vis; } diff --git a/src/libsyntax_expand/expand.rs b/src/libsyntax_expand/expand.rs index bdb50dbfb4f47..c96b4a93dbddc 100644 --- a/src/libsyntax_expand/expand.rs +++ b/src/libsyntax_expand/expand.rs @@ -85,7 +85,7 @@ macro_rules! ast_fragments { // mention some macro variable from those arguments even if it's not used. #[cfg_attr(bootstrap, allow(unused_macros))] macro _repeating($flat_map_ast_elt) {} - placeholder(AstFragmentKind::$Kind, *id).$make_ast() + placeholder(AstFragmentKind::$Kind, *id, None).$make_ast() })),)?)* _ => panic!("unexpected AST fragment kind") } @@ -274,6 +274,23 @@ pub enum InvocationKind { }, } +impl InvocationKind { + fn placeholder_visibility(&self) -> Option { + // HACK: For unnamed fields placeholders should have the same visibility as the actual + // fields because for tuple structs/variants resolve determines visibilities of their + // constructor using these field visibilities before attributes on them are are expanded. + // The assumption is that the attribute expansion cannot change field visibilities, + // and it holds because only inert attributes are supported in this position. + match self { + InvocationKind::Attr { item: Annotatable::StructField(field), .. } | + InvocationKind::Derive { item: Annotatable::StructField(field), .. } | + InvocationKind::DeriveContainer { item: Annotatable::StructField(field), .. } + if field.ident.is_none() => Some(field.vis.clone()), + _ => None, + } + } +} + impl Invocation { pub fn span(&self) -> Span { match &self.kind { @@ -941,6 +958,7 @@ impl<'a, 'b> InvocationCollector<'a, 'b> { _ => None, }; let expn_id = ExpnId::fresh(expn_data); + let vis = kind.placeholder_visibility(); self.invocations.push(Invocation { kind, fragment_kind, @@ -950,7 +968,7 @@ impl<'a, 'b> InvocationCollector<'a, 'b> { ..self.cx.current_expansion.clone() }, }); - placeholder(fragment_kind, NodeId::placeholder_from_expn_id(expn_id)) + placeholder(fragment_kind, NodeId::placeholder_from_expn_id(expn_id), vis) } fn collect_bang(&mut self, mac: ast::Mac, span: Span, kind: AstFragmentKind) -> AstFragment { diff --git a/src/libsyntax_expand/placeholders.rs b/src/libsyntax_expand/placeholders.rs index e595888dae7d1..2d2c2cbc83476 100644 --- a/src/libsyntax_expand/placeholders.rs +++ b/src/libsyntax_expand/placeholders.rs @@ -12,7 +12,8 @@ use smallvec::{smallvec, SmallVec}; use rustc_data_structures::fx::FxHashMap; -pub fn placeholder(kind: AstFragmentKind, id: ast::NodeId) -> AstFragment { +pub fn placeholder(kind: AstFragmentKind, id: ast::NodeId, vis: Option) + -> AstFragment { fn mac_placeholder() -> ast::Mac { ast::Mac { path: ast::Path { span: DUMMY_SP, segments: Vec::new() }, @@ -26,7 +27,7 @@ pub fn placeholder(kind: AstFragmentKind, id: ast::NodeId) -> AstFragment { let ident = ast::Ident::invalid(); let attrs = Vec::new(); let generics = ast::Generics::default(); - let vis = dummy_spanned(ast::VisibilityKind::Inherited); + let vis = vis.unwrap_or_else(|| dummy_spanned(ast::VisibilityKind::Inherited)); let span = DUMMY_SP; let expr_placeholder = || P(ast::Expr { id, span, diff --git a/src/test/ui/attributes/unnamed-field-attributes-vis.rs b/src/test/ui/attributes/unnamed-field-attributes-vis.rs new file mode 100644 index 0000000000000..d12155f6d81fd --- /dev/null +++ b/src/test/ui/attributes/unnamed-field-attributes-vis.rs @@ -0,0 +1,11 @@ +// Unnamed fields don't lose their visibility due to non-builtin attributes on them. + +// check-pass + +mod m { + pub struct S(#[rustfmt::skip] pub u8); +} + +fn main() { + m::S(0); +} From 04a46929f2c53fb9e20c182f8be02566eb31c65f Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Sun, 8 Dec 2019 01:55:14 +0100 Subject: [PATCH 07/12] Ensure that we get a hard error on generic ZST constants if their body causes an error during evaluation --- src/librustc_codegen_ssa/mir/constant.rs | 5 ++++- src/librustc_mir/transform/simplify.rs | 11 ++++++++--- .../ui/consts/assoc_const_generic_impl.rs | 19 +++++++++++++++++++ .../ui/consts/assoc_const_generic_impl.stderr | 8 ++++++++ 4 files changed, 39 insertions(+), 4 deletions(-) create mode 100644 src/test/ui/consts/assoc_const_generic_impl.rs create mode 100644 src/test/ui/consts/assoc_const_generic_impl.stderr diff --git a/src/librustc_codegen_ssa/mir/constant.rs b/src/librustc_codegen_ssa/mir/constant.rs index d06359ab0ce89..72d098eb31117 100644 --- a/src/librustc_codegen_ssa/mir/constant.rs +++ b/src/librustc_codegen_ssa/mir/constant.rs @@ -23,7 +23,10 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { instance, promoted: None, }; - self.cx.tcx().const_eval(ty::ParamEnv::reveal_all().and(cid)) + self.cx.tcx().const_eval(ty::ParamEnv::reveal_all().and(cid)).map_err(|err| { + self.cx.tcx().sess.span_err(constant.span, "erroneous constant encountered"); + err + }) }, _ => Ok(self.monomorphize(&constant.literal)), } diff --git a/src/librustc_mir/transform/simplify.rs b/src/librustc_mir/transform/simplify.rs index 1b90ea78c6450..385fc7ed2cd00 100644 --- a/src/librustc_mir/transform/simplify.rs +++ b/src/librustc_mir/transform/simplify.rs @@ -363,9 +363,14 @@ impl<'a, 'tcx> Visitor<'tcx> for DeclMarker<'a, 'tcx> { let stmt = &self.body.basic_blocks()[location.block].statements[location.statement_index]; if let StatementKind::Assign(box (p, Rvalue::Use(Operand::Constant(c)))) = &stmt.kind { - if p.as_local().is_some() { - trace!("skipping store of const value {:?} to {:?}", c, local); - return; + match c.literal.val { + // Keep assignments from unevaluated constants around, since the evaluation + // may report errors, even if the use of the constant is dead code. + interpret::ConstValue::Unevaluated(..) => {} + _ => if p.as_local().is_some() { + trace!("skipping store of const value {:?} to {:?}", c, p); + return; + }, } } } diff --git a/src/test/ui/consts/assoc_const_generic_impl.rs b/src/test/ui/consts/assoc_const_generic_impl.rs new file mode 100644 index 0000000000000..cce0cdbf8c559 --- /dev/null +++ b/src/test/ui/consts/assoc_const_generic_impl.rs @@ -0,0 +1,19 @@ +#![allow(const_err)] + +trait ZeroSized: Sized { + const I_AM_ZERO_SIZED: (); + fn requires_zero_size(self); +} + +impl ZeroSized for T { + const I_AM_ZERO_SIZED: () = [()][std::mem::size_of::()]; + fn requires_zero_size(self) { + let () = Self::I_AM_ZERO_SIZED; //~ ERROR erroneous constant encountered + println!("requires_zero_size called"); + } +} + +fn main() { + ().requires_zero_size(); + 42_u32.requires_zero_size(); +} diff --git a/src/test/ui/consts/assoc_const_generic_impl.stderr b/src/test/ui/consts/assoc_const_generic_impl.stderr new file mode 100644 index 0000000000000..3765a3703c7fc --- /dev/null +++ b/src/test/ui/consts/assoc_const_generic_impl.stderr @@ -0,0 +1,8 @@ +error: erroneous constant encountered + --> $DIR/assoc_const_generic_impl.rs:11:18 + | +LL | let () = Self::I_AM_ZERO_SIZED; + | ^^^^^^^^^^^^^^^^^^^^^ + +error: aborting due to previous error + From f4ff238a2e45e6412b9bf164e801a2e4809bdd3b Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Mon, 9 Dec 2019 13:05:41 +0100 Subject: [PATCH 08/12] Show const_err lint in addition to the hard error --- src/test/ui/consts/assoc_const_generic_impl.rs | 4 ++-- src/test/ui/consts/assoc_const_generic_impl.stderr | 14 ++++++++++++++ 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/src/test/ui/consts/assoc_const_generic_impl.rs b/src/test/ui/consts/assoc_const_generic_impl.rs index cce0cdbf8c559..62702a8ec5cb1 100644 --- a/src/test/ui/consts/assoc_const_generic_impl.rs +++ b/src/test/ui/consts/assoc_const_generic_impl.rs @@ -1,4 +1,4 @@ -#![allow(const_err)] +#![warn(const_err)] trait ZeroSized: Sized { const I_AM_ZERO_SIZED: (); @@ -6,7 +6,7 @@ trait ZeroSized: Sized { } impl ZeroSized for T { - const I_AM_ZERO_SIZED: () = [()][std::mem::size_of::()]; + const I_AM_ZERO_SIZED: () = [()][std::mem::size_of::()]; //~ WARN any use of this value fn requires_zero_size(self) { let () = Self::I_AM_ZERO_SIZED; //~ ERROR erroneous constant encountered println!("requires_zero_size called"); diff --git a/src/test/ui/consts/assoc_const_generic_impl.stderr b/src/test/ui/consts/assoc_const_generic_impl.stderr index 3765a3703c7fc..a114d5c6ccd14 100644 --- a/src/test/ui/consts/assoc_const_generic_impl.stderr +++ b/src/test/ui/consts/assoc_const_generic_impl.stderr @@ -1,3 +1,17 @@ +warning: any use of this value will cause an error + --> $DIR/assoc_const_generic_impl.rs:9:34 + | +LL | const I_AM_ZERO_SIZED: () = [()][std::mem::size_of::()]; + | -----------------------------^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^- + | | + | index out of bounds: the len is 1 but the index is 4 + | +note: lint level defined here + --> $DIR/assoc_const_generic_impl.rs:1:9 + | +LL | #![warn(const_err)] + | ^^^^^^^^^ + error: erroneous constant encountered --> $DIR/assoc_const_generic_impl.rs:11:18 | From 7463e29efe65e4db9051f75d59799051e0a411b0 Mon Sep 17 00:00:00 2001 From: Mark Rousskov Date: Sat, 14 Dec 2019 10:05:40 -0500 Subject: [PATCH 09/12] Rebase fixes --- src/librustc/hir/map/definitions.rs | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/librustc/hir/map/definitions.rs b/src/librustc/hir/map/definitions.rs index be8d82173e481..450ab9471720f 100644 --- a/src/librustc/hir/map/definitions.rs +++ b/src/librustc/hir/map/definitions.rs @@ -105,7 +105,7 @@ pub struct Definitions { /// we know what parent node that fragment should be attached to thanks to this table. invocation_parents: FxHashMap, /// Indices of unnamed struct or variant fields with unresolved attributes. - pub(super) placeholder_field_indices: NodeMap, + pub placeholder_field_indices: NodeMap, } /// A unique identifier that we can use to lookup a definition @@ -544,6 +544,15 @@ impl Definitions { let old_parent = self.invocation_parents.insert(invoc_id, parent); assert!(old_parent.is_none(), "parent `DefIndex` is reset for an invocation"); } + + pub fn placeholder_field_index(&self, node_id: ast::NodeId) -> usize { + self.placeholder_field_indices[&node_id] + } + + pub fn set_placeholder_field_index(&mut self, node_id: ast::NodeId, index: usize) { + let old_index = self.placeholder_field_indices.insert(node_id, index); + assert!(old_index.is_none(), "placeholder field index is reset for a node ID"); + } } impl DefPathData { From d9d7203d4747abc6fd1fa685b4eff76b0400f65e Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Sat, 7 Dec 2019 00:55:02 +0300 Subject: [PATCH 10/12] resolve: Cleanup some field processing code --- src/librustc_resolve/build_reduced_graph.rs | 63 ++++++++++----------- 1 file changed, 31 insertions(+), 32 deletions(-) diff --git a/src/librustc_resolve/build_reduced_graph.rs b/src/librustc_resolve/build_reduced_graph.rs index 1bef810b24084..28fbd6fc1fd3f 100644 --- a/src/librustc_resolve/build_reduced_graph.rs +++ b/src/librustc_resolve/build_reduced_graph.rs @@ -301,6 +301,13 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> { } } + fn insert_field_names_local(&mut self, def_id: DefId, vdata: &ast::VariantData) { + let field_names = vdata.fields().iter().map(|field| { + respan(field.span, field.ident.map_or(kw::Invalid, |ident| ident.name)) + }).collect(); + self.insert_field_names(def_id, field_names); + } + fn insert_field_names(&mut self, def_id: DefId, field_names: Vec>) { if !field_names.is_empty() { self.r.field_names.insert(def_id, field_names); @@ -734,58 +741,50 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> { } // These items live in both the type and value namespaces. - ItemKind::Struct(ref struct_def, _) => { + ItemKind::Struct(ref vdata, _) => { // Define a name in the type namespace. let def_id = self.r.definitions.local_def_id(item.id); let res = Res::Def(DefKind::Struct, def_id); self.r.define(parent, ident, TypeNS, (res, vis, sp, expansion)); - let mut ctor_vis = vis; - - let has_non_exhaustive = attr::contains_name(&item.attrs, sym::non_exhaustive); - - // If the structure is marked as non_exhaustive then lower the visibility - // to within the crate. - if has_non_exhaustive && vis == ty::Visibility::Public { - ctor_vis = ty::Visibility::Restricted(DefId::local(CRATE_DEF_INDEX)); - } - // Record field names for error reporting. - let field_names = struct_def.fields().iter().map(|field| { - // NOTE: The field may be an expansion placeholder, but expansion sets correct - // visibilities for unnamed field placeholders specifically, so the constructor - // visibility should still be determined correctly. - let field_vis = self.resolve_visibility(&field.vis); - if ctor_vis.is_at_least(field_vis, &*self.r) { - ctor_vis = field_vis; - } - respan(field.span, field.ident.map_or(kw::Invalid, |ident| ident.name)) - }).collect(); - let item_def_id = self.r.definitions.local_def_id(item.id); - self.insert_field_names(item_def_id, field_names); + self.insert_field_names_local(def_id, vdata); // If this is a tuple or unit struct, define a name // in the value namespace as well. - if let Some(ctor_node_id) = struct_def.ctor_id() { + if let Some(ctor_node_id) = vdata.ctor_id() { + let mut ctor_vis = vis; + // If the structure is marked as non_exhaustive then lower the visibility + // to within the crate. + if vis == ty::Visibility::Public && + attr::contains_name(&item.attrs, sym::non_exhaustive) { + ctor_vis = ty::Visibility::Restricted(DefId::local(CRATE_DEF_INDEX)); + } + for field in vdata.fields() { + // NOTE: The field may be an expansion placeholder, but expansion sets + // correct visibilities for unnamed field placeholders specifically, so the + // constructor visibility should still be determined correctly. + let field_vis = self.resolve_visibility_speculative(&field.vis, true); + if ctor_vis.is_at_least(field_vis, &*self.r) { + ctor_vis = field_vis; + } + } let ctor_res = Res::Def( - DefKind::Ctor(CtorOf::Struct, CtorKind::from_ast(struct_def)), + DefKind::Ctor(CtorOf::Struct, CtorKind::from_ast(vdata)), self.r.definitions.local_def_id(ctor_node_id), ); self.r.define(parent, ident, ValueNS, (ctor_res, ctor_vis, sp, expansion)); - self.r.struct_constructors.insert(res.def_id(), (ctor_res, ctor_vis)); + self.r.struct_constructors.insert(def_id, (ctor_res, ctor_vis)); } } ItemKind::Union(ref vdata, _) => { - let res = Res::Def(DefKind::Union, self.r.definitions.local_def_id(item.id)); + let def_id = self.r.definitions.local_def_id(item.id); + let res = Res::Def(DefKind::Union, def_id); self.r.define(parent, ident, TypeNS, (res, vis, sp, expansion)); // Record field names for error reporting. - let field_names = vdata.fields().iter().map(|field| { - respan(field.span, field.ident.map_or(kw::Invalid, |ident| ident.name)) - }).collect(); - let item_def_id = self.r.definitions.local_def_id(item.id); - self.insert_field_names(item_def_id, field_names); + self.insert_field_names_local(def_id, vdata); } ItemKind::Trait(..) => { From 8209dbf1fa564adf5973f1074355d21f33520c5b Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Sat, 7 Dec 2019 01:49:21 +0300 Subject: [PATCH 11/12] resolve: Make visibility resolution more speculative To avoid potential duplicate diagnostics and separate the error reporting logic --- src/librustc_resolve/build_reduced_graph.rs | 95 +++++++------------ src/librustc_resolve/diagnostics.rs | 40 ++++++++ src/librustc_resolve/lib.rs | 9 ++ .../field-attributes-vis-unresolved.rs | 1 - .../field-attributes-vis-unresolved.stderr | 11 +-- 5 files changed, 87 insertions(+), 69 deletions(-) diff --git a/src/librustc_resolve/build_reduced_graph.rs b/src/librustc_resolve/build_reduced_graph.rs index 28fbd6fc1fd3f..f9e9e84ed3298 100644 --- a/src/librustc_resolve/build_reduced_graph.rs +++ b/src/librustc_resolve/build_reduced_graph.rs @@ -11,7 +11,7 @@ use crate::resolve_imports::ImportDirectiveSubclass::{self, GlobImport, SingleIm use crate::{Module, ModuleData, ModuleKind, NameBinding, NameBindingKind, Segment, ToNameBinding}; use crate::{ModuleOrUniformRoot, ParentScope, PerNS, Resolver, ResolverArenas, ExternPreludeEntry}; use crate::Namespace::{self, TypeNS, ValueNS, MacroNS}; -use crate::{ResolutionError, Determinacy, PathResult, CrateLint}; +use crate::{ResolutionError, VisResolutionError, Determinacy, PathResult, CrateLint}; use rustc::bug; use rustc::hir::def::{self, *}; @@ -35,7 +35,7 @@ use syntax::ast::{MetaItemKind, StmtKind, TraitItem, TraitItemKind}; use syntax::feature_gate::is_builtin_attr; use syntax::parse::token::{self, Token}; use syntax::print::pprust; -use syntax::{span_err, struct_span_err}; +use syntax::span_err; use syntax::source_map::{respan, Spanned}; use syntax::symbol::{kw, sym}; use syntax::visit::{self, Visitor}; @@ -194,22 +194,25 @@ impl<'a> AsMut> for BuildReducedGraphVisitor<'a, '_> { impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> { fn resolve_visibility(&mut self, vis: &ast::Visibility) -> ty::Visibility { - self.resolve_visibility_speculative(vis, false) + self.resolve_visibility_speculative(vis, false).unwrap_or_else(|err| { + self.r.report_vis_error(err); + ty::Visibility::Public + }) } - fn resolve_visibility_speculative( + fn resolve_visibility_speculative<'ast>( &mut self, - vis: &ast::Visibility, + vis: &'ast ast::Visibility, speculative: bool, - ) -> ty::Visibility { + ) -> Result> { let parent_scope = &self.parent_scope; match vis.node { - ast::VisibilityKind::Public => ty::Visibility::Public, + ast::VisibilityKind::Public => Ok(ty::Visibility::Public), ast::VisibilityKind::Crate(..) => { - ty::Visibility::Restricted(DefId::local(CRATE_DEF_INDEX)) + Ok(ty::Visibility::Restricted(DefId::local(CRATE_DEF_INDEX))) } ast::VisibilityKind::Inherited => { - ty::Visibility::Restricted(parent_scope.module.normal_ancestor_id) + Ok(ty::Visibility::Restricted(parent_scope.module.normal_ancestor_id)) } ast::VisibilityKind::Restricted { ref path, id, .. } => { // For visibilities we are not ready to provide correct implementation of "uniform @@ -219,32 +222,19 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> { let ident = path.segments.get(0).expect("empty path in visibility").ident; let crate_root = if ident.is_path_segment_keyword() { None - } else if ident.span.rust_2018() { - let msg = "relative paths are not supported in visibilities on 2018 edition"; - self.r.session.struct_span_err(ident.span, msg) - .span_suggestion( - path.span, - "try", - format!("crate::{}", pprust::path_to_string(&path)), - Applicability::MaybeIncorrect, - ) - .emit(); - return ty::Visibility::Public; - } else { - let ctxt = ident.span.ctxt(); + } else if ident.span.rust_2015() { Some(Segment::from_ident(Ident::new( - kw::PathRoot, path.span.shrink_to_lo().with_ctxt(ctxt) + kw::PathRoot, path.span.shrink_to_lo().with_ctxt(ident.span.ctxt()) ))) + } else { + return Err(VisResolutionError::Relative2018(ident.span, path)); }; let segments = crate_root.into_iter() .chain(path.segments.iter().map(|seg| seg.into())).collect::>(); - let expected_found_error = |this: &Self, res: Res| { - let path_str = Segment::names_to_string(&segments); - struct_span_err!(this.r.session, path.span, E0577, - "expected module, found {} `{}`", res.descr(), path_str) - .span_label(path.span, "not a module").emit(); - }; + let expected_found_error = |res| Err(VisResolutionError::ExpectedFound( + path.span, Segment::names_to_string(&segments), res + )); match self.r.resolve_path( &segments, Some(TypeNS), @@ -260,42 +250,27 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> { } if module.is_normal() { if res == Res::Err { - ty::Visibility::Public + Ok(ty::Visibility::Public) } else { let vis = ty::Visibility::Restricted(res.def_id()); if self.r.is_accessible_from(vis, parent_scope.module) { - vis + Ok(vis) } else { - struct_span_err!(self.r.session, path.span, E0742, - "visibilities can only be restricted to ancestor modules") - .emit(); - ty::Visibility::Public + Err(VisResolutionError::AncestorOnly(path.span)) } } } else { - expected_found_error(self, res); - ty::Visibility::Public + expected_found_error(res) } } - PathResult::Module(..) => { - self.r.session.span_err(path.span, "visibility must resolve to a module"); - ty::Visibility::Public - } - PathResult::NonModule(partial_res) => { - expected_found_error(self, partial_res.base_res()); - ty::Visibility::Public - } - PathResult::Failed { span, label, suggestion, .. } => { - self.r.report_error( - span, ResolutionError::FailedToResolve { label, suggestion } - ); - ty::Visibility::Public - } - PathResult::Indeterminate => { - span_err!(self.r.session, path.span, E0578, - "cannot determine resolution for the visibility"); - ty::Visibility::Public - } + PathResult::Module(..) => + Err(VisResolutionError::ModuleOnly(path.span)), + PathResult::NonModule(partial_res) => + expected_found_error(partial_res.base_res()), + PathResult::Failed { span, label, suggestion, .. } => + Err(VisResolutionError::FailedToResolve(span, label, suggestion)), + PathResult::Indeterminate => + Err(VisResolutionError::Indeterminate(path.span)), } } } @@ -764,9 +739,11 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> { // NOTE: The field may be an expansion placeholder, but expansion sets // correct visibilities for unnamed field placeholders specifically, so the // constructor visibility should still be determined correctly. - let field_vis = self.resolve_visibility_speculative(&field.vis, true); - if ctor_vis.is_at_least(field_vis, &*self.r) { - ctor_vis = field_vis; + if let Ok(field_vis) = + self.resolve_visibility_speculative(&field.vis, true) { + if ctor_vis.is_at_least(field_vis, &*self.r) { + ctor_vis = field_vis; + } } } let ctor_res = Res::Def( diff --git a/src/librustc_resolve/diagnostics.rs b/src/librustc_resolve/diagnostics.rs index 3d68b72a655fb..6a8a678da0454 100644 --- a/src/librustc_resolve/diagnostics.rs +++ b/src/librustc_resolve/diagnostics.rs @@ -11,6 +11,7 @@ use rustc::ty::{self, DefIdTree}; use rustc::util::nodemap::FxHashSet; use syntax::ast::{self, Ident, Path}; use syntax::feature_gate::BUILTIN_ATTRIBUTES; +use syntax::print::pprust; use syntax::source_map::SourceMap; use syntax::struct_span_err; use syntax::symbol::{Symbol, kw}; @@ -22,6 +23,7 @@ use crate::resolve_imports::{ImportDirective, ImportDirectiveSubclass, ImportRes use crate::{path_names_to_string, KNOWN_TOOLS}; use crate::{BindingError, CrateLint, HasGenericParams, LegacyScope, Module, ModuleOrUniformRoot}; use crate::{PathResult, ParentScope, ResolutionError, Resolver, Scope, ScopeSet, Segment}; +use crate::VisResolutionError; type Res = def::Res; @@ -355,6 +357,44 @@ impl<'a> Resolver<'a> { } } + crate fn report_vis_error(&self, vis_resolution_error: VisResolutionError<'_>) { + match vis_resolution_error { + VisResolutionError::Relative2018(span, path) => { + let mut err = self.session.struct_span_err(span, + "relative paths are not supported in visibilities on 2018 edition"); + err.span_suggestion( + path.span, + "try", + format!("crate::{}", pprust::path_to_string(&path)), + Applicability::MaybeIncorrect, + ); + err + } + VisResolutionError::AncestorOnly(span) => { + struct_span_err!(self.session, span, E0742, + "visibilities can only be restricted to ancestor modules") + } + VisResolutionError::FailedToResolve(span, label, suggestion) => { + self.into_struct_error( + span, ResolutionError::FailedToResolve { label, suggestion } + ) + } + VisResolutionError::ExpectedFound(span, path_str, res) => { + let mut err = struct_span_err!(self.session, span, E0577, + "expected module, found {} `{}`", res.descr(), path_str); + err.span_label(span, "not a module"); + err + } + VisResolutionError::Indeterminate(span) => { + struct_span_err!(self.session, span, E0578, + "cannot determine resolution for the visibility") + } + VisResolutionError::ModuleOnly(span) => { + self.session.struct_span_err(span, "visibility must resolve to a module") + } + }.emit() + } + /// Lookup typo candidate in scope for a macro or import. fn early_lookup_typo_candidate( &mut self, diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs index b45eb356bdbd0..49685fe8077df 100644 --- a/src/librustc_resolve/lib.rs +++ b/src/librustc_resolve/lib.rs @@ -218,6 +218,15 @@ enum ResolutionError<'a> { SelfInTyParamDefault, } +enum VisResolutionError<'a> { + Relative2018(Span, &'a ast::Path), + AncestorOnly(Span), + FailedToResolve(Span, String, Option), + ExpectedFound(Span, String, Res), + Indeterminate(Span), + ModuleOnly(Span), +} + // A minimal representation of a path segment. We use this in resolve because // we synthesize 'path segments' which don't have the rest of an AST or HIR // `PathSegment`. diff --git a/src/test/ui/attributes/field-attributes-vis-unresolved.rs b/src/test/ui/attributes/field-attributes-vis-unresolved.rs index 36f3feb973d6d..d1bd2a1e72724 100644 --- a/src/test/ui/attributes/field-attributes-vis-unresolved.rs +++ b/src/test/ui/attributes/field-attributes-vis-unresolved.rs @@ -20,7 +20,6 @@ struct S { struct Z( #[rustfmt::skip] pub(in nonexistent) u8 //~ ERROR failed to resolve - //~| ERROR cannot determine resolution for the visibility ); fn main() {} diff --git a/src/test/ui/attributes/field-attributes-vis-unresolved.stderr b/src/test/ui/attributes/field-attributes-vis-unresolved.stderr index 8c283e9bcb991..41c3cea302135 100644 --- a/src/test/ui/attributes/field-attributes-vis-unresolved.stderr +++ b/src/test/ui/attributes/field-attributes-vis-unresolved.stderr @@ -1,9 +1,3 @@ -error[E0578]: cannot determine resolution for the visibility - --> $DIR/field-attributes-vis-unresolved.rs:22:12 - | -LL | pub(in nonexistent) u8 - | ^^^^^^^^^^^ - error[E0433]: failed to resolve: maybe a missing crate `nonexistent`? --> $DIR/field-attributes-vis-unresolved.rs:17:12 | @@ -16,7 +10,6 @@ error[E0433]: failed to resolve: maybe a missing crate `nonexistent`? LL | pub(in nonexistent) u8 | ^^^^^^^^^^^ maybe a missing crate `nonexistent`? -error: aborting due to 3 previous errors +error: aborting due to 2 previous errors -Some errors have detailed explanations: E0433, E0578. -For more information about an error, try `rustc --explain E0433`. +For more information about this error, try `rustc --explain E0433`. From c403f6255c9628074a4d413e49ee58e5f3a31a02 Mon Sep 17 00:00:00 2001 From: Mark Rousskov Date: Sat, 14 Dec 2019 10:36:05 -0500 Subject: [PATCH 12/12] Rebase fixes --- src/librustc_resolve/build_reduced_graph.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/librustc_resolve/build_reduced_graph.rs b/src/librustc_resolve/build_reduced_graph.rs index f9e9e84ed3298..0a966b252e2ed 100644 --- a/src/librustc_resolve/build_reduced_graph.rs +++ b/src/librustc_resolve/build_reduced_graph.rs @@ -34,7 +34,6 @@ use syntax::ast::{self, Block, ForeignItem, ForeignItemKind, Item, ItemKind, Nod use syntax::ast::{MetaItemKind, StmtKind, TraitItem, TraitItemKind}; use syntax::feature_gate::is_builtin_attr; use syntax::parse::token::{self, Token}; -use syntax::print::pprust; use syntax::span_err; use syntax::source_map::{respan, Spanned}; use syntax::symbol::{kw, sym};