diff options
| -rw-r--r-- | compiler/rustc_resolve/src/build_reduced_graph.rs | 17 | ||||
| -rw-r--r-- | compiler/rustc_resolve/src/late.rs | 30 | ||||
| -rw-r--r-- | compiler/rustc_resolve/src/late/diagnostics.rs | 55 | ||||
| -rw-r--r-- | compiler/rustc_resolve/src/lib.rs | 7 | ||||
| -rw-r--r-- | src/test/ui/issues/auxiliary/issue-75907.rs | 5 | ||||
| -rw-r--r-- | src/test/ui/issues/issue-75907.rs | 18 | ||||
| -rw-r--r-- | src/test/ui/issues/issue-75907.stderr | 29 | ||||
| -rw-r--r-- | src/test/ui/issues/issue-75907_b.rs | 11 | ||||
| -rw-r--r-- | src/test/ui/issues/issue-75907_b.stderr | 9 |
9 files changed, 152 insertions, 29 deletions
diff --git a/compiler/rustc_resolve/src/build_reduced_graph.rs b/compiler/rustc_resolve/src/build_reduced_graph.rs index 03c2915f848..565313902a4 100644 --- a/compiler/rustc_resolve/src/build_reduced_graph.rs +++ b/compiler/rustc_resolve/src/build_reduced_graph.rs @@ -796,23 +796,26 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> { vis }; + let mut ret_fields = Vec::with_capacity(vdata.fields().len()); + 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. - 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 field_vis = self + .resolve_visibility_speculative(&field.vis, true) + .unwrap_or(ty::Visibility::Public); + if ctor_vis.is_at_least(field_vis, &*self.r) { + ctor_vis = field_vis; } + ret_fields.push(field_vis); } let ctor_res = Res::Def( DefKind::Ctor(CtorOf::Struct, CtorKind::from_ast(vdata)), self.r.local_def_id(ctor_node_id).to_def_id(), ); self.r.define(parent, ident, ValueNS, (ctor_res, ctor_vis, sp, expansion)); - self.r.struct_constructors.insert(def_id, (ctor_res, ctor_vis)); + self.r.struct_constructors.insert(def_id, (ctor_res, ctor_vis, ret_fields)); } } @@ -964,7 +967,7 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> { Res::Def(DefKind::Ctor(CtorOf::Struct, ..), def_id) => { let parent = cstore.def_key(def_id).parent; if let Some(struct_def_id) = parent.map(|index| DefId { index, ..def_id }) { - self.r.struct_constructors.insert(struct_def_id, (res, vis)); + self.r.struct_constructors.insert(struct_def_id, (res, vis, vec![])); } } _ => {} diff --git a/compiler/rustc_resolve/src/late.rs b/compiler/rustc_resolve/src/late.rs index dda084214b8..07f36c7b7ad 100644 --- a/compiler/rustc_resolve/src/late.rs +++ b/compiler/rustc_resolve/src/late.rs @@ -188,7 +188,7 @@ crate enum PathSource<'a> { // Paths in struct expressions and patterns `Path { .. }`. Struct, // Paths in tuple struct patterns `Path(..)`. - TupleStruct(Span), + TupleStruct(Span, &'a [Span]), // `m::A::B` in `<T as m::A>::B::C`. TraitItem(Namespace), } @@ -197,7 +197,7 @@ impl<'a> PathSource<'a> { fn namespace(self) -> Namespace { match self { PathSource::Type | PathSource::Trait(_) | PathSource::Struct => TypeNS, - PathSource::Expr(..) | PathSource::Pat | PathSource::TupleStruct(_) => ValueNS, + PathSource::Expr(..) | PathSource::Pat | PathSource::TupleStruct(..) => ValueNS, PathSource::TraitItem(ns) => ns, } } @@ -208,7 +208,7 @@ impl<'a> PathSource<'a> { | PathSource::Expr(..) | PathSource::Pat | PathSource::Struct - | PathSource::TupleStruct(_) => true, + | PathSource::TupleStruct(..) => true, PathSource::Trait(_) | PathSource::TraitItem(..) => false, } } @@ -219,7 +219,7 @@ impl<'a> PathSource<'a> { PathSource::Trait(_) => "trait", PathSource::Pat => "unit struct, unit variant or constant", PathSource::Struct => "struct, variant or union type", - PathSource::TupleStruct(_) => "tuple struct or tuple variant", + PathSource::TupleStruct(..) => "tuple struct or tuple variant", PathSource::TraitItem(ns) => match ns { TypeNS => "associated type", ValueNS => "method or associated constant", @@ -305,7 +305,7 @@ impl<'a> PathSource<'a> { | Res::SelfCtor(..) => true, _ => false, }, - PathSource::TupleStruct(_) => match res { + PathSource::TupleStruct(..) => match res { Res::Def(DefKind::Ctor(_, CtorKind::Fn), _) | Res::SelfCtor(..) => true, _ => false, }, @@ -340,8 +340,8 @@ impl<'a> PathSource<'a> { (PathSource::Struct, false) => error_code!(E0422), (PathSource::Expr(..), true) => error_code!(E0423), (PathSource::Expr(..), false) => error_code!(E0425), - (PathSource::Pat | PathSource::TupleStruct(_), true) => error_code!(E0532), - (PathSource::Pat | PathSource::TupleStruct(_), false) => error_code!(E0531), + (PathSource::Pat | PathSource::TupleStruct(..), true) => error_code!(E0532), + (PathSource::Pat | PathSource::TupleStruct(..), false) => error_code!(E0531), (PathSource::TraitItem(..), true) => error_code!(E0575), (PathSource::TraitItem(..), false) => error_code!(E0576), } @@ -411,7 +411,7 @@ struct LateResolutionVisitor<'a, 'b, 'ast> { } /// Walks the whole crate in DFS order, visiting each item, resolving names as it goes. -impl<'a, 'ast> Visitor<'ast> for LateResolutionVisitor<'a, '_, 'ast> { +impl<'a: 'ast, 'ast> Visitor<'ast> for LateResolutionVisitor<'a, '_, 'ast> { fn visit_item(&mut self, item: &'ast Item) { let prev = replace(&mut self.diagnostic_metadata.current_item, Some(item)); // Always report errors in items we just entered. @@ -659,7 +659,7 @@ impl<'a, 'ast> Visitor<'ast> for LateResolutionVisitor<'a, '_, 'ast> { } } -impl<'a, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> { +impl<'a: 'ast, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> { fn new(resolver: &'b mut Resolver<'a>) -> LateResolutionVisitor<'a, 'b, 'ast> { // During late resolution we only track the module component of the parent scope, // although it may be useful to track other components as well for diagnostics. @@ -1539,8 +1539,16 @@ impl<'a, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> { .unwrap_or_else(|| self.fresh_binding(ident, pat.id, pat_src, bindings)); self.r.record_partial_res(pat.id, PartialRes::new(res)); } - PatKind::TupleStruct(ref path, ..) => { - self.smart_resolve_path(pat.id, None, path, PathSource::TupleStruct(pat.span)); + PatKind::TupleStruct(ref path, ref sub_patterns) => { + self.smart_resolve_path( + pat.id, + None, + path, + PathSource::TupleStruct( + pat.span, + self.r.arenas.alloc_pattern_spans(sub_patterns.iter().map(|p| p.span)), + ), + ); } PatKind::Path(ref qself, ref path) => { self.smart_resolve_path(pat.id, qself.as_ref(), path, PathSource::Pat); diff --git a/compiler/rustc_resolve/src/late/diagnostics.rs b/compiler/rustc_resolve/src/late/diagnostics.rs index 9f631705b2b..9b5650c260c 100644 --- a/compiler/rustc_resolve/src/late/diagnostics.rs +++ b/compiler/rustc_resolve/src/late/diagnostics.rs @@ -90,7 +90,7 @@ fn import_candidate_to_enum_paths(suggestion: &ImportSuggestion) -> (String, Str (variant_path_string, enum_path_string) } -impl<'a> LateResolutionVisitor<'a, '_, '_> { +impl<'a: 'ast, 'ast> LateResolutionVisitor<'a, '_, 'ast> { fn def_span(&self, def_id: DefId) -> Option<Span> { match def_id.krate { LOCAL_CRATE => self.r.opt_span(def_id), @@ -623,12 +623,12 @@ impl<'a> LateResolutionVisitor<'a, '_, '_> { ); } } - PathSource::Expr(_) | PathSource::TupleStruct(_) | PathSource::Pat => { + PathSource::Expr(_) | PathSource::TupleStruct(..) | PathSource::Pat => { let span = match &source { PathSource::Expr(Some(Expr { span, kind: ExprKind::Call(_, _), .. })) - | PathSource::TupleStruct(span) => { + | PathSource::TupleStruct(span, _) => { // We want the main underline to cover the suggested code as well for // cleaner output. err.set_span(*span); @@ -640,7 +640,7 @@ impl<'a> LateResolutionVisitor<'a, '_, '_> { err.span_label(span, &format!("`{}` defined here", path_str)); } let (tail, descr, applicability) = match source { - PathSource::Pat | PathSource::TupleStruct(_) => { + PathSource::Pat | PathSource::TupleStruct(..) => { ("", "pattern", Applicability::MachineApplicable) } _ => (": val", "literal", Applicability::HasPlaceholders), @@ -705,7 +705,7 @@ impl<'a> LateResolutionVisitor<'a, '_, '_> { } ( Res::Def(DefKind::Enum, def_id), - PathSource::TupleStruct(_) | PathSource::Expr(..), + PathSource::TupleStruct(..) | PathSource::Expr(..), ) => { if self .diagnostic_metadata @@ -745,15 +745,50 @@ impl<'a> LateResolutionVisitor<'a, '_, '_> { } } (Res::Def(DefKind::Struct, def_id), _) if ns == ValueNS => { - if let Some((ctor_def, ctor_vis)) = self.r.struct_constructors.get(&def_id).cloned() + if let Some((ctor_def, ctor_vis, fields)) = + self.r.struct_constructors.get(&def_id).cloned() { let accessible_ctor = self.r.is_accessible_from(ctor_vis, self.parent_scope.module); if is_expected(ctor_def) && !accessible_ctor { - err.span_label( - span, - "constructor is not visible here due to private fields".to_string(), - ); + let mut better_diag = false; + if let PathSource::TupleStruct(_, pattern_spans) = source { + if pattern_spans.len() > 0 && fields.len() == pattern_spans.len() { + let non_visible_spans: Vec<Span> = fields + .iter() + .zip(pattern_spans.iter()) + .filter_map(|(vis, span)| { + match self + .r + .is_accessible_from(*vis, self.parent_scope.module) + { + true => None, + false => Some(*span), + } + }) + .collect(); + // Extra check to be sure + if non_visible_spans.len() > 0 { + let mut m: rustc_span::MultiSpan = + non_visible_spans.clone().into(); + non_visible_spans.into_iter().for_each(|s| { + m.push_span_label(s, "private field".to_string()) + }); + err.span_note( + m, + "constructor is not visible here due to private fields", + ); + better_diag = true; + } + } + } + + if !better_diag { + err.span_label( + span, + "constructor is not visible here due to private fields".to_string(), + ); + } } } else { bad_struct_syntax_suggestion(def_id); diff --git a/compiler/rustc_resolve/src/lib.rs b/compiler/rustc_resolve/src/lib.rs index c7913e84455..0f5b9b51816 100644 --- a/compiler/rustc_resolve/src/lib.rs +++ b/compiler/rustc_resolve/src/lib.rs @@ -1005,7 +1005,8 @@ pub struct Resolver<'a> { /// Table for mapping struct IDs into struct constructor IDs, /// it's not used during normal resolution, only for better error reporting. - struct_constructors: DefIdMap<(Res, ty::Visibility)>, + /// Also includes of list of each fields visibility + struct_constructors: DefIdMap<(Res, ty::Visibility, Vec<ty::Visibility>)>, /// Features enabled for this crate. active_features: FxHashSet<Symbol>, @@ -1042,6 +1043,7 @@ pub struct ResolverArenas<'a> { name_resolutions: TypedArena<RefCell<NameResolution<'a>>>, macro_rules_bindings: TypedArena<MacroRulesBinding<'a>>, ast_paths: TypedArena<ast::Path>, + pattern_spans: TypedArena<Span>, } impl<'a> ResolverArenas<'a> { @@ -1073,6 +1075,9 @@ impl<'a> ResolverArenas<'a> { fn alloc_ast_paths(&'a self, paths: &[ast::Path]) -> &'a [ast::Path] { self.ast_paths.alloc_from_iter(paths.iter().cloned()) } + fn alloc_pattern_spans(&'a self, spans: impl Iterator<Item = Span>) -> &'a [Span] { + self.pattern_spans.alloc_from_iter(spans) + } } impl<'a> AsMut<Resolver<'a>> for Resolver<'a> { diff --git a/src/test/ui/issues/auxiliary/issue-75907.rs b/src/test/ui/issues/auxiliary/issue-75907.rs new file mode 100644 index 00000000000..0b70452a24d --- /dev/null +++ b/src/test/ui/issues/auxiliary/issue-75907.rs @@ -0,0 +1,5 @@ +pub struct Bar(pub u8, u8, u8); + +pub fn make_bar() -> Bar { + Bar(1, 12, 10) +} diff --git a/src/test/ui/issues/issue-75907.rs b/src/test/ui/issues/issue-75907.rs new file mode 100644 index 00000000000..8c155d9be35 --- /dev/null +++ b/src/test/ui/issues/issue-75907.rs @@ -0,0 +1,18 @@ +// Test for for diagnostic improvement issue #75907 + +mod foo { + pub(crate) struct Foo(u8); + pub(crate) struct Bar(pub u8, u8, Foo); + + pub(crate) fn make_bar() -> Bar { + Bar(1, 12, Foo(10)) + } +} + +use foo::{make_bar, Bar, Foo}; + +fn main() { + let Bar(x, y, Foo(z)) = make_bar(); + //~^ ERROR expected tuple struct + //~| ERROR expected tuple struct +} diff --git a/src/test/ui/issues/issue-75907.stderr b/src/test/ui/issues/issue-75907.stderr new file mode 100644 index 00000000000..65b9a51e01d --- /dev/null +++ b/src/test/ui/issues/issue-75907.stderr @@ -0,0 +1,29 @@ +error[E0532]: expected tuple struct or tuple variant, found struct `Bar` + --> $DIR/issue-75907.rs:15:9 + | +LL | let Bar(x, y, Foo(z)) = make_bar(); + | ^^^ + | +note: constructor is not visible here due to private fields + --> $DIR/issue-75907.rs:15:16 + | +LL | let Bar(x, y, Foo(z)) = make_bar(); + | ^ ^^^^^^ private field + | | + | private field + +error[E0532]: expected tuple struct or tuple variant, found struct `Foo` + --> $DIR/issue-75907.rs:15:19 + | +LL | let Bar(x, y, Foo(z)) = make_bar(); + | ^^^ + | +note: constructor is not visible here due to private fields + --> $DIR/issue-75907.rs:15:23 + | +LL | let Bar(x, y, Foo(z)) = make_bar(); + | ^ private field + +error: aborting due to 2 previous errors + +For more information about this error, try `rustc --explain E0532`. diff --git a/src/test/ui/issues/issue-75907_b.rs b/src/test/ui/issues/issue-75907_b.rs new file mode 100644 index 00000000000..fdd3bc6d724 --- /dev/null +++ b/src/test/ui/issues/issue-75907_b.rs @@ -0,0 +1,11 @@ +// Test for for diagnostic improvement issue #75907, extern crate +// aux-build:issue-75907.rs + +extern crate issue_75907 as a; + +use a::{make_bar, Bar}; + +fn main() { + let Bar(x, y, z) = make_bar(); + //~^ ERROR expected tuple struct +} diff --git a/src/test/ui/issues/issue-75907_b.stderr b/src/test/ui/issues/issue-75907_b.stderr new file mode 100644 index 00000000000..cdd21de6c33 --- /dev/null +++ b/src/test/ui/issues/issue-75907_b.stderr @@ -0,0 +1,9 @@ +error[E0532]: expected tuple struct or tuple variant, found struct `Bar` + --> $DIR/issue-75907_b.rs:9:9 + | +LL | let Bar(x, y, z) = make_bar(); + | ^^^ constructor is not visible here due to private fields + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0532`. |