Skip to content

Commit

Permalink
Auto merge of #65153 - da-x:issue-58017, r=petrochenkov
Browse files Browse the repository at this point in the history
Improve message when attempting to instantiate tuple structs with private fields

Fixes #58017, fixes #39703.

```
error[E0603]: tuple struct `Error` is private
  --> main.rs:22:16
   |
2  |     pub struct Error(usize, pub usize, usize);
   |                      -----             ----- field is private
   |                      |
   |                      field is private
...
22 |     let x = a::Error(3, 1, 2);
   |                ^^^^^
   |
   = note: a tuple struct constructor is private if any of its fields is private
```
  • Loading branch information
bors committed Oct 10, 2019
2 parents 58b5491 + 48f8bed commit 898f36c
Show file tree
Hide file tree
Showing 9 changed files with 222 additions and 120 deletions.
5 changes: 3 additions & 2 deletions src/librustc_metadata/cstore_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ use syntax::source_map;
use syntax::edition::Edition;
use syntax::parse::source_file_to_stream;
use syntax::parse::parser::emit_unclosed_delims;
use syntax::source_map::Spanned;
use syntax::symbol::Symbol;
use syntax_pos::{Span, FileName};
use rustc_index::bit_set::BitSet;
Expand Down Expand Up @@ -424,8 +425,8 @@ impl cstore::CStore {
self.get_crate_data(cnum).root.edition
}

pub fn struct_field_names_untracked(&self, def: DefId) -> Vec<ast::Name> {
self.get_crate_data(def.krate).get_struct_field_names(def.index)
pub fn struct_field_names_untracked(&self, def: DefId, sess: &Session) -> Vec<Spanned<Symbol>> {
self.get_crate_data(def.krate).get_struct_field_names(def.index, sess)
}

pub fn ctor_kind_untracked(&self, def: DefId) -> def::CtorKind {
Expand Down
6 changes: 3 additions & 3 deletions src/librustc_metadata/decoder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ use std::u32;
use rustc_serialize::{Decodable, Decoder, SpecializedDecoder, opaque};
use syntax::attr;
use syntax::ast::{self, Ident};
use syntax::source_map;
use syntax::source_map::{self, respan, Spanned};
use syntax::symbol::{Symbol, sym};
use syntax::ext::base::{MacroKind, SyntaxExtensionKind, SyntaxExtension};
use syntax_pos::{self, Span, BytePos, Pos, DUMMY_SP, symbol::{InternedString}};
Expand Down Expand Up @@ -1021,11 +1021,11 @@ impl<'a, 'tcx> CrateMetadata {
Lrc::from(self.get_attributes(&item, sess))
}

pub fn get_struct_field_names(&self, id: DefIndex) -> Vec<ast::Name> {
pub fn get_struct_field_names(&self, id: DefIndex, sess: &Session) -> Vec<Spanned<ast::Name>> {
self.entry(id)
.children
.decode(self)
.map(|index| self.item_name(index))
.map(|index| respan(self.get_span(index, sess), self.item_name(index)))
.collect()
}

Expand Down
14 changes: 8 additions & 6 deletions src/librustc_resolve/build_reduced_graph.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ use syntax::ext::hygiene::ExpnId;
use syntax::feature_gate::is_builtin_attr;
use syntax::parse::token::{self, Token};
use syntax::{span_err, struct_span_err};
use syntax::source_map::{respan, Spanned};
use syntax::symbol::{kw, sym};
use syntax::visit::{self, Visitor};

Expand Down Expand Up @@ -301,7 +302,7 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> {
}
}

fn insert_field_names(&mut self, def_id: DefId, field_names: Vec<Name>) {
fn insert_field_names(&mut self, def_id: DefId, field_names: Vec<Spanned<Name>>) {
if !field_names.is_empty() {
self.r.field_names.insert(def_id, field_names);
}
Expand Down Expand Up @@ -752,12 +753,12 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> {
}

// Record field names for error reporting.
let field_names = struct_def.fields().iter().filter_map(|field| {
let field_names = struct_def.fields().iter().map(|field| {
let field_vis = self.resolve_visibility(&field.vis);
if ctor_vis.is_at_least(field_vis, &*self.r) {
ctor_vis = field_vis;
}
field.ident.map(|ident| ident.name)
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);
Expand All @@ -779,9 +780,9 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> {
self.r.define(parent, ident, TypeNS, (res, vis, sp, expansion));

// Record field names for error reporting.
let field_names = vdata.fields().iter().filter_map(|field| {
let field_names = vdata.fields().iter().map(|field| {
self.resolve_visibility(&field.vis);
field.ident.map(|ident| ident.name)
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);
Expand Down Expand Up @@ -895,7 +896,8 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> {
// Record some extra data for better diagnostics.
match res {
Res::Def(DefKind::Struct, def_id) | Res::Def(DefKind::Union, def_id) => {
let field_names = self.r.cstore.struct_field_names_untracked(def_id);
let field_names =
self.r.cstore.struct_field_names_untracked(def_id, self.r.session);
self.insert_field_names(def_id, field_names);
}
Res::Def(DefKind::Method, def_id) => {
Expand Down
3 changes: 2 additions & 1 deletion src/librustc_resolve/late/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -497,7 +497,8 @@ impl<'a> LateResolutionVisitor<'a, '_> {
Res::Def(DefKind::Struct, did) | Res::Def(DefKind::Union, did)
if resolution.unresolved_segments() == 0 => {
if let Some(field_names) = self.r.field_names.get(&did) {
if field_names.iter().any(|&field_name| ident.name == field_name) {
if field_names.iter()
.any(|&field_name| ident.name == field_name.node) {
return Some(AssocSuggestion::Field);
}
}
Expand Down
33 changes: 14 additions & 19 deletions src/librustc_resolve/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ use rustc::hir::def::{self, DefKind, PartialRes, CtorKind, CtorOf, NonMacroAttrK
use rustc::hir::def::Namespace::*;
use rustc::hir::def_id::{CRATE_DEF_INDEX, LOCAL_CRATE, DefId};
use rustc::hir::{TraitMap, GlobMap};
use rustc::ty;
use rustc::ty::{self, DefIdTree};
use rustc::util::nodemap::{NodeMap, NodeSet, FxHashMap, FxHashSet, DefIdMap};
use rustc::span_bug;

Expand All @@ -46,6 +46,7 @@ use syntax::attr;
use syntax::ast::{CRATE_NODE_ID, Crate};
use syntax::ast::{ItemKind, Path};
use syntax::{struct_span_err, unwrap_or};
use syntax::source_map::Spanned;

use syntax_pos::{Span, DUMMY_SP};
use errors::{Applicability, DiagnosticBuilder};
Expand Down Expand Up @@ -829,7 +830,7 @@ pub struct Resolver<'a> {

/// Names of fields of an item `DefId` accessible with dot syntax.
/// Used for hints during error reporting.
field_names: FxHashMap<DefId, Vec<Name>>,
field_names: FxHashMap<DefId, Vec<Spanned<Name>>>,

/// All imports known to succeed or fail.
determined_imports: Vec<&'a ImportDirective<'a>>,
Expand Down Expand Up @@ -994,7 +995,7 @@ impl<'a> AsMut<Resolver<'a>> for Resolver<'a> {
fn as_mut(&mut self) -> &mut Resolver<'a> { self }
}

impl<'a, 'b> ty::DefIdTree for &'a Resolver<'b> {
impl<'a, 'b> DefIdTree for &'a Resolver<'b> {
fn parent(self, id: DefId) -> Option<DefId> {
match id.krate {
LOCAL_CRATE => self.definitions.def_key(id.index).parent,
Expand Down Expand Up @@ -2386,23 +2387,17 @@ impl<'a> Resolver<'a> {
binding.res().descr(),
ident.name,
);
// FIXME: use the ctor's `def_id` to check wether any of the fields is not visible
match binding.kind {
NameBindingKind::Res(Res::Def(DefKind::Ctor(
CtorOf::Struct,
CtorKind::Fn,
), _def_id), _) => {
err.note("a tuple struct constructor is private if any of its fields \
is private");
}
NameBindingKind::Res(Res::Def(DefKind::Ctor(
CtorOf::Variant,
CtorKind::Fn,
), _def_id), _) => {
err.note("a tuple variant constructor is private if any of its fields \
is private");
if let NameBindingKind::Res(
Res::Def(DefKind::Ctor(CtorOf::Struct, CtorKind::Fn), ctor_def_id), _
) = binding.kind {
let def_id = (&*self).parent(ctor_def_id).expect("no parent for a constructor");
if let Some(fields) = self.field_names.get(&def_id) {
let first_field = fields.first().expect("empty field list in the map");
err.span_label(
fields.iter().fold(first_field.span, |acc, field| acc.to(field.span)),
"a tuple struct constructor is private if any of its fields is private",
);
}
_ => {}
}
err.emit();
}
Expand Down
Loading

0 comments on commit 898f36c

Please sign in to comment.