Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve message when attempting to instantiate tuple structs with private fields #65153

Merged
merged 2 commits into from
Oct 10, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@ -420,8 +421,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 @@ -30,7 +30,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 @@ -47,6 +47,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 @@ -840,7 +841,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 @@ -1005,7 +1006,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 @@ -2390,23 +2391,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