Give better diagnostic when using a private tuple struct constructor

This commit is contained in:
Gus Wynn 2020-09-08 15:14:09 -07:00
parent 130359cb05
commit c63f634a4b
9 changed files with 152 additions and 29 deletions

View File

@ -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![]));
}
}
_ => {}

View File

@ -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);

View File

@ -89,7 +89,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),
@ -622,12 +622,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);
@ -639,7 +639,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),
@ -704,7 +704,7 @@ impl<'a> LateResolutionVisitor<'a, '_, '_> {
}
(
Res::Def(DefKind::Enum, def_id),
PathSource::TupleStruct(_) | PathSource::Expr(..),
PathSource::TupleStruct(..) | PathSource::Expr(..),
) => {
if self
.diagnostic_metadata
@ -744,15 +744,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);

View File

@ -999,7 +999,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>,
@ -1036,6 +1037,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> {
@ -1067,6 +1069,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> {

View File

@ -0,0 +1,5 @@
pub struct Bar(pub u8, u8, u8);
pub fn make_bar() -> Bar {
Bar(1, 12, 10)
}

View File

@ -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
}

View File

@ -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`.

View File

@ -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
}

View File

@ -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`.