Rollup merge of #155353 - petrochenkov:globdedup, r=mu001999

resolve: Remove `inaccessible_ctor_reexport` resolver field

Collect the necessary information during error reporting instead of doing it on a good path from the core name resolution infra.

Also cleanup the related diagnostic code for struct constructors in general, see the individual commits.

This mitigates most of the harm brought by https://github.com/rust-lang/rust/pull/133477.
This commit is contained in:
Jacob Pratt
2026-04-16 01:54:07 -04:00
committed by GitHub
8 changed files with 104 additions and 162 deletions
@@ -32,17 +32,16 @@
use crate::Namespace::{MacroNS, TypeNS, ValueNS};
use crate::def_collector::collect_definitions;
use crate::diagnostics::StructCtor;
use crate::imports::{ImportData, ImportKind, OnUnknownData};
use crate::macros::{MacroRulesDecl, MacroRulesScope, MacroRulesScopeRef};
use crate::ref_mut::CmCell;
use crate::{
BindingKey, Decl, DeclData, DeclKind, ExternPreludeEntry, Finalize, IdentKey, MacroData,
Module, ModuleKind, ModuleOrUniformRoot, ParentScope, PathResult, ResolutionError, Resolver,
Segment, Used, VisResolutionError, errors,
Module, ModuleKind, ModuleOrUniformRoot, ParentScope, PathResult, Res, ResolutionError,
Resolver, Segment, Used, VisResolutionError, errors,
};
type Res = def::Res<NodeId>;
impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
/// Attempt to put the declaration with the given name and namespace into the module,
/// and report an error in case of a collision.
@@ -929,7 +928,7 @@ fn build_reduced_graph_for_item(&mut self, item: &'a Item) {
vis
};
let mut ret_fields = Vec::with_capacity(vdata.fields().len());
let mut field_visibilities = Vec::with_capacity(vdata.fields().len());
for field in vdata.fields() {
// NOTE: The field may be an expansion placeholder, but expansion sets
@@ -941,7 +940,7 @@ fn build_reduced_graph_for_item(&mut self, item: &'a Item) {
if ctor_vis.is_at_least(field_vis, self.r.tcx) {
ctor_vis = field_vis;
}
ret_fields.push(field_vis.to_def_id());
field_visibilities.push(field_vis.to_def_id());
}
let feed = self.r.feed(ctor_node_id);
let ctor_def_id = feed.key();
@@ -951,9 +950,9 @@ fn build_reduced_graph_for_item(&mut self, item: &'a Item) {
// We need the field visibility spans also for the constructor for E0603.
self.insert_field_visibilities_local(ctor_def_id.to_def_id(), vdata.fields());
self.r
.struct_constructors
.insert(local_def_id, (ctor_res, ctor_vis.to_def_id(), ret_fields));
let ctor =
StructCtor { res: ctor_res, vis: ctor_vis.to_def_id(), field_visibilities };
self.r.struct_ctors.insert(local_def_id, ctor);
}
self.r.struct_generics.insert(local_def_id, generics.clone());
}
+35 -5
View File
@@ -17,11 +17,11 @@
use rustc_feature::BUILTIN_ATTRIBUTES;
use rustc_hir::attrs::{CfgEntry, StrippedCfgItem};
use rustc_hir::def::Namespace::{self, *};
use rustc_hir::def::{self, CtorKind, CtorOf, DefKind, MacroKinds, NonMacroAttrKind, PerNS};
use rustc_hir::def::{CtorKind, CtorOf, DefKind, MacroKinds, NonMacroAttrKind, PerNS};
use rustc_hir::def_id::{CRATE_DEF_ID, DefId};
use rustc_hir::{PrimTy, Stability, StabilityLevel, find_attr};
use rustc_middle::bug;
use rustc_middle::ty::TyCtxt;
use rustc_middle::ty::{TyCtxt, Visibility};
use rustc_session::Session;
use rustc_session::lint::builtin::{
ABSOLUTE_PATHS_NOT_STARTING_WITH_CRATE, AMBIGUOUS_GLOB_IMPORTS, AMBIGUOUS_IMPORT_VISIBILITIES,
@@ -49,13 +49,11 @@
use crate::{
AmbiguityError, AmbiguityKind, AmbiguityWarning, BindingError, BindingKey, Decl, DeclKind,
Finalize, ForwardGenericParamBanReason, HasGenericParams, IdentKey, LateDecl, MacroRulesScope,
Module, ModuleKind, ModuleOrUniformRoot, ParentScope, PathResult, PrivacyError,
Module, ModuleKind, ModuleOrUniformRoot, ParentScope, PathResult, PrivacyError, Res,
ResolutionError, Resolver, Scope, ScopeSet, Segment, UseError, Used, VisResolutionError,
errors as errs, path_names_to_string,
};
type Res = def::Res<ast::NodeId>;
/// A vector of spans and replacements, a message and applicability.
pub(crate) type Suggestion = (Vec<(Span, String)>, String, Applicability);
@@ -63,6 +61,19 @@
/// similarly named label and whether or not it is reachable.
pub(crate) type LabelSuggestion = (Ident, bool);
#[derive(Clone)]
pub(crate) struct StructCtor {
pub res: Res,
pub vis: Visibility<DefId>,
pub field_visibilities: Vec<Visibility<DefId>>,
}
impl StructCtor {
pub(crate) fn has_private_fields<'ra>(&self, m: Module<'ra>, r: &Resolver<'ra, '_>) -> bool {
self.field_visibilities.iter().any(|&vis| !r.is_accessible_from(vis, m))
}
}
#[derive(Debug)]
pub(crate) enum SuggestionTarget {
/// The target has a similar name as the name used by the programmer (probably a typo)
@@ -3176,6 +3187,25 @@ fn comes_from_same_module_for_glob(
err.subdiagnostic(note);
}
}
pub(crate) fn struct_ctor(&self, def_id: DefId) -> Option<StructCtor> {
match def_id.as_local() {
Some(def_id) => self.struct_ctors.get(&def_id).cloned(),
None => {
self.cstore().ctor_untracked(self.tcx, def_id).map(|(ctor_kind, ctor_def_id)| {
let res = Res::Def(DefKind::Ctor(CtorOf::Struct, ctor_kind), ctor_def_id);
let vis = self.tcx.visibility(ctor_def_id);
let field_visibilities = self
.tcx
.associated_item_def_ids(def_id)
.iter()
.map(|&field_id| self.tcx.visibility(field_id))
.collect();
StructCtor { res, vis, field_visibilities }
})
}
}
}
}
/// Given a `binding_span` of a binding within a use statement:
-34
View File
@@ -1087,7 +1087,6 @@ fn resolve_ident_in_module_non_globs_unadjusted<'r>(
orig_ident_span,
binding,
parent_scope,
module,
finalize,
shadowing,
);
@@ -1150,7 +1149,6 @@ fn resolve_ident_in_module_globs_unadjusted<'r>(
orig_ident_span,
binding,
parent_scope,
module,
finalize,
shadowing,
);
@@ -1260,7 +1258,6 @@ fn finalize_module_binding(
orig_ident_span: Span,
binding: Option<Decl<'ra>>,
parent_scope: &ParentScope<'ra>,
module: Module<'ra>,
finalize: Finalize,
shadowing: Shadowing,
) -> Result<Decl<'ra>, ControlFlow<Determinacy, Determinacy>> {
@@ -1295,37 +1292,6 @@ fn finalize_module_binding(
self.macro_expanded_macro_export_errors.insert((path_span, binding.span));
}
// If we encounter a re-export for a type with private fields, it will not be able to
// be constructed through this re-export. We track that case here to expand later
// privacy errors with appropriate information.
if let Res::Def(_, def_id) = binding.res() {
let struct_ctor = match def_id.as_local() {
Some(def_id) => self.struct_constructors.get(&def_id).cloned(),
None => {
let ctor = self.cstore().ctor_untracked(self.tcx(), def_id);
ctor.map(|(ctor_kind, ctor_def_id)| {
let ctor_res = Res::Def(
DefKind::Ctor(rustc_hir::def::CtorOf::Struct, ctor_kind),
ctor_def_id,
);
let ctor_vis = self.tcx.visibility(ctor_def_id);
let field_visibilities = self
.tcx
.associated_item_def_ids(def_id)
.iter()
.map(|&field_id| self.tcx.visibility(field_id))
.collect();
(ctor_res, ctor_vis, field_visibilities)
})
}
};
if let Some((_, _, fields)) = struct_ctor
&& fields.iter().any(|vis| !self.is_accessible_from(*vis, module))
{
self.inaccessible_ctor_reexport.insert(path_span, binding.span);
}
}
self.record_use(ident, binding, used);
return Ok(binding);
}
+1 -3
View File
@@ -37,12 +37,10 @@
use crate::ref_mut::CmCell;
use crate::{
AmbiguityError, BindingKey, CmResolver, Decl, DeclData, DeclKind, Determinacy, Finalize,
IdentKey, ImportSuggestion, Module, ModuleOrUniformRoot, ParentScope, PathResult, PerNS,
IdentKey, ImportSuggestion, Module, ModuleOrUniformRoot, ParentScope, PathResult, PerNS, Res,
ResolutionError, Resolver, ScopeSet, Segment, Used, module_to_string, names_to_string,
};
type Res = def::Res<NodeId>;
/// A potential import declaration in the process of being planted into a module.
/// Also used for lazily planting names from `--extern` flags to extern prelude.
#[derive(Clone, Copy, Default, PartialEq, Debug)]
+2 -4
View File
@@ -25,7 +25,7 @@
StashKey, Suggestions, elided_lifetime_in_path_suggestion, pluralize,
};
use rustc_hir::def::Namespace::{self, *};
use rustc_hir::def::{self, CtorKind, DefKind, LifetimeRes, NonMacroAttrKind, PartialRes, PerNS};
use rustc_hir::def::{CtorKind, DefKind, LifetimeRes, NonMacroAttrKind, PartialRes, PerNS};
use rustc_hir::def_id::{CRATE_DEF_ID, DefId, LOCAL_CRATE, LocalDefId};
use rustc_hir::{MissingLifetimeKind, PrimTy, TraitCandidate};
use rustc_middle::middle::resolve_bound_vars::Set1;
@@ -41,14 +41,12 @@
use crate::{
BindingError, BindingKey, Decl, DelegationFnSig, Finalize, IdentKey, LateDecl, Module,
ModuleOrUniformRoot, ParentScope, PathResult, ResolutionError, Resolver, Segment, Stage,
ModuleOrUniformRoot, ParentScope, PathResult, Res, ResolutionError, Resolver, Segment, Stage,
TyCtxt, UseError, Used, errors, path_names_to_string, rustdoc,
};
mod diagnostics;
type Res = def::Res<NodeId>;
use diagnostics::{ElisionFnParameter, LifetimeElisionCandidate, MissingLifetime};
#[derive(Copy, Clone, Debug)]
+53 -95
View File
@@ -19,7 +19,7 @@
};
use rustc_hir as hir;
use rustc_hir::def::Namespace::{self, *};
use rustc_hir::def::{self, CtorKind, CtorOf, DefKind, MacroKinds};
use rustc_hir::def::{CtorKind, CtorOf, DefKind, MacroKinds};
use rustc_hir::def_id::{CRATE_DEF_ID, DefId};
use rustc_hir::{MissingLifetimeKind, PrimTy, find_attr};
use rustc_middle::ty;
@@ -38,12 +38,10 @@
};
use crate::ty::fast_reject::SimplifiedType;
use crate::{
Finalize, Module, ModuleKind, ModuleOrUniformRoot, ParentScope, PathResult, PathSource,
Finalize, Module, ModuleKind, ModuleOrUniformRoot, ParentScope, PathResult, PathSource, Res,
Resolver, ScopeSet, Segment, errors, path_names_to_string,
};
type Res = def::Res<ast::NodeId>;
/// A field or associated item from self type suggested in case of resolution failure.
enum AssocSuggestion {
Field(Span),
@@ -1040,14 +1038,15 @@ fn try_lookup_name_relaxed(
}
if let Some(Res::Def(DefKind::Struct, def_id)) = res {
let private_fields = self.has_private_fields(def_id);
let adjust_error_message =
private_fields && self.is_struct_with_fn_ctor(def_id);
if adjust_error_message {
self.update_err_for_private_tuple_struct_fields(err, &source, def_id);
}
if private_fields {
if let Some(ctor) = self.r.struct_ctor(def_id)
&& ctor.has_private_fields(self.parent_scope.module, self.r)
{
if matches!(
ctor.res,
Res::Def(DefKind::Ctor(CtorOf::Struct, CtorKind::Fn), _)
) {
self.update_err_for_private_tuple_struct_fields(err, &source, def_id);
}
err.note("constructor is not visible here due to private fields");
}
} else {
@@ -1892,10 +1891,7 @@ fn restrict_assoc_type_in_where_clause(&self, span: Span, err: &mut Diag<'_>) ->
let ast::TyKind::Path(Some(qself), path) = &bounded_ty.kind else { return false };
// use this to verify that ident is a type param.
let Some(partial_res) = self.r.partial_res_map.get(&bounded_ty.id) else { return false };
if !matches!(
partial_res.full_res(),
Some(hir::def::Res::Def(hir::def::DefKind::AssocTy, _))
) {
if !matches!(partial_res.full_res(), Some(Res::Def(DefKind::AssocTy, _))) {
return false;
}
@@ -1905,10 +1901,7 @@ fn restrict_assoc_type_in_where_clause(&self, span: Span, err: &mut Diag<'_>) ->
let Some(partial_res) = self.r.partial_res_map.get(&peeled_ty.id) else {
return false;
};
if !matches!(
partial_res.full_res(),
Some(hir::def::Res::Def(hir::def::DefKind::TyParam, _))
) {
if !matches!(partial_res.full_res(), Some(Res::Def(DefKind::TyParam, _))) {
return false;
}
let ([ast::PathSegment { args: None, .. }], [ast::GenericBound::Trait(poly_trait_ref)]) =
@@ -1928,7 +1921,7 @@ fn restrict_assoc_type_in_where_clause(&self, span: Span, err: &mut Diag<'_>) ->
let Some(partial_res) = self.r.partial_res_map.get(&id) else {
return false;
};
if !matches!(partial_res.full_res(), Some(hir::def::Res::Def(..))) {
if !matches!(partial_res.full_res(), Some(Res::Def(..))) {
return false;
}
@@ -2015,19 +2008,6 @@ fn followed_by_brace(&self, span: Span) -> (bool, Option<Span>) {
}
}
fn is_struct_with_fn_ctor(&mut self, def_id: DefId) -> bool {
def_id
.as_local()
.and_then(|local_id| self.r.struct_constructors.get(&local_id))
.map(|struct_ctor| {
matches!(
struct_ctor.0,
def::Res::Def(DefKind::Ctor(CtorOf::Struct, CtorKind::Fn), _)
)
})
.unwrap_or(false)
}
fn update_err_for_private_tuple_struct_fields(
&mut self,
err: &mut Diag<'_>,
@@ -2201,7 +2181,17 @@ fn smart_resolve_context_dependent_help(
_ => (": val", "literal", Applicability::HasPlaceholders, None),
};
if !this.has_private_fields(def_id) {
// Imprecise for local structs without ctors, we don't keep fields for them.
let has_private_fields = match def_id.as_local() {
Some(def_id) => this.r.struct_ctors.get(&def_id).is_some_and(|ctor| {
ctor.has_private_fields(this.parent_scope.module, this.r)
}),
None => this.r.tcx.associated_item_def_ids(def_id).iter().any(|field_id| {
let vis = this.r.tcx.visibility(*field_id);
!this.r.is_accessible_from(vis, this.parent_scope.module)
}),
};
if !has_private_fields {
// If the fields of the type are private, we shouldn't be suggesting using
// the struct literal syntax at all, as that will cause a subsequent error.
let fields = this.r.field_idents(def_id);
@@ -2367,53 +2357,39 @@ fn smart_resolve_context_dependent_help(
self.suggest_using_enum_variant(err, source, def_id, span);
}
(Res::Def(DefKind::Struct, def_id), source) if ns == ValueNS => {
let struct_ctor = match def_id.as_local() {
Some(def_id) => self.r.struct_constructors.get(&def_id).cloned(),
None => {
let ctor = self.r.cstore().ctor_untracked(self.r.tcx(), def_id);
ctor.map(|(ctor_kind, ctor_def_id)| {
let ctor_res =
Res::Def(DefKind::Ctor(CtorOf::Struct, ctor_kind), ctor_def_id);
let ctor_vis = self.r.tcx.visibility(ctor_def_id);
let field_visibilities = self
.r
.tcx
.associated_item_def_ids(def_id)
.iter()
.map(|&field_id| self.r.tcx.visibility(field_id))
.collect();
(ctor_res, ctor_vis, field_visibilities)
})
}
};
let (ctor_def, ctor_vis, fields) = if let Some(struct_ctor) = struct_ctor {
if let PathSource::Expr(Some(parent)) = source
&& let ExprKind::Field(..) | ExprKind::MethodCall(..) = parent.kind
{
bad_struct_syntax_suggestion(self, err, def_id);
return true;
}
struct_ctor
} else {
if let PathSource::Expr(Some(parent)) = source
&& let ExprKind::Field(..) | ExprKind::MethodCall(..) = parent.kind
{
bad_struct_syntax_suggestion(self, err, def_id);
return true;
}
let Some(ctor) = self.r.struct_ctor(def_id) else {
bad_struct_syntax_suggestion(self, err, def_id);
return true;
};
let is_accessible = self.r.is_accessible_from(ctor_vis, self.parent_scope.module);
if let Some(use_span) = self.r.inaccessible_ctor_reexport.get(&span)
&& is_accessible
// A type is re-exported and has an inaccessible constructor because it has fields
// that are inaccessible from the reexport's scope, extend the diagnostic.
let is_accessible = self.r.is_accessible_from(ctor.vis, self.parent_scope.module);
if is_accessible
&& let mod_path = &path[..path.len() - 1]
&& let PathResult::Module(ModuleOrUniformRoot::Module(import_mod)) =
self.resolve_path(mod_path, Some(TypeNS), None, PathSource::Module)
&& ctor.has_private_fields(import_mod, self.r)
&& let Ok(import_decl) = self.r.cm().maybe_resolve_ident_in_module(
ModuleOrUniformRoot::Module(import_mod),
path.last().unwrap().ident,
TypeNS,
&self.parent_scope,
None,
)
{
err.span_note(
*use_span,
import_decl.span,
"the type is accessed through this re-export, but the type's constructor \
is not visible in this import's scope due to private fields",
);
if is_accessible
&& fields
.iter()
.all(|vis| self.r.is_accessible_from(*vis, self.parent_scope.module))
{
if !ctor.has_private_fields(self.parent_scope.module, self.r) {
err.span_suggestion_verbose(
span,
"the type can be constructed directly, because its fields are \
@@ -2430,17 +2406,17 @@ fn smart_resolve_context_dependent_help(
}
self.update_err_for_private_tuple_struct_fields(err, &source, def_id);
}
if !is_expected(ctor_def) || is_accessible {
if !is_expected(ctor.res) || is_accessible {
return true;
}
let field_spans =
self.update_err_for_private_tuple_struct_fields(err, &source, def_id);
if let Some(spans) =
field_spans.filter(|spans| spans.len() > 0 && fields.len() == spans.len())
if let Some(spans) = field_spans
.filter(|spans| spans.len() > 0 && ctor.field_visibilities.len() == spans.len())
{
let non_visible_spans: Vec<Span> = iter::zip(&fields, &spans)
let non_visible_spans: Vec<Span> = iter::zip(&ctor.field_visibilities, &spans)
.filter(|(vis, _)| {
!self.r.is_accessible_from(**vis, self.parent_scope.module)
})
@@ -2715,24 +2691,6 @@ fn suggest_alternative_construction_methods(
}
}
fn has_private_fields(&self, def_id: DefId) -> bool {
let fields = match def_id.as_local() {
Some(def_id) => self.r.struct_constructors.get(&def_id).cloned().map(|(_, _, f)| f),
None => Some(
self.r
.tcx
.associated_item_def_ids(def_id)
.iter()
.map(|&field_id| self.r.tcx.visibility(field_id))
.collect(),
),
};
fields.is_some_and(|fields| {
fields.iter().any(|vis| !self.r.is_accessible_from(*vis, self.parent_scope.module))
})
}
/// Given the target `ident` and `kind`, search for the similarly named associated item
/// in `self.current_trait_ref`.
pub(crate) fn find_similarly_named_assoc_item(
+2 -7
View File
@@ -26,7 +26,7 @@
use std::ops::ControlFlow;
use std::sync::Arc;
use diagnostics::{ImportSuggestion, LabelSuggestion, Suggestion};
use diagnostics::{ImportSuggestion, LabelSuggestion, StructCtor, Suggestion};
use effective_visibilities::EffectiveVisibilitiesVisitor;
use errors::{ParamKindInEnumDiscriminant, ParamKindInNonTrivialAnonConst};
use hygiene::Macros20NormalizedSyntaxContext;
@@ -1285,11 +1285,6 @@ pub struct Resolver<'ra, 'tcx> {
/// Crate-local macro expanded `macro_export` referred to by a module-relative path.
macro_expanded_macro_export_errors: BTreeSet<(Span, Span)> = BTreeSet::new(),
/// When a type is re-exported that has an inaccessible constructor because it has fields that
/// are inaccessible from the import's scope, we mark that as the type won't be able to be built
/// through the re-export. We use this information to extend the existing diagnostic.
inaccessible_ctor_reexport: FxHashMap<Span, Span> = default::fx_hash_map(),
arenas: &'ra ResolverArenas<'ra>,
dummy_decl: Decl<'ra>,
builtin_type_decls: FxHashMap<Symbol, Decl<'ra>>,
@@ -1346,7 +1341,7 @@ pub struct Resolver<'ra, 'tcx> {
/// Table for mapping struct IDs into struct constructor IDs,
/// it's not used during normal resolution, only for better error reporting.
/// Also includes of list of each fields visibility
struct_constructors: LocalDefIdMap<(Res, Visibility<DefId>, Vec<Visibility<DefId>>)> = Default::default(),
struct_ctors: LocalDefIdMap<StructCtor> = Default::default(),
/// for all the struct
/// it's not used during normal resolution, only for better error reporting.
+3 -5
View File
@@ -18,7 +18,7 @@
};
use rustc_feature::Features;
use rustc_hir::attrs::{AttributeKind, CfgEntry, StrippedCfgItem};
use rustc_hir::def::{self, DefKind, MacroKinds, Namespace, NonMacroAttrKind};
use rustc_hir::def::{DefKind, MacroKinds, Namespace, NonMacroAttrKind};
use rustc_hir::def_id::{CrateNum, DefId, LocalDefId};
use rustc_hir::{Attribute, StabilityLevel};
use rustc_middle::middle::stability;
@@ -43,12 +43,10 @@
use crate::imports::Import;
use crate::{
BindingKey, CacheCell, CmResolver, Decl, DeclKind, DeriveData, Determinacy, Finalize, IdentKey,
InvocationParent, MacroData, ModuleKind, ModuleOrUniformRoot, ParentScope, PathResult,
InvocationParent, MacroData, ModuleKind, ModuleOrUniformRoot, ParentScope, PathResult, Res,
ResolutionError, Resolver, ScopeSet, Segment, Used,
};
type Res = def::Res<NodeId>;
/// Name declaration produced by a `macro_rules` item definition.
/// Not modularized, can shadow previous `macro_rules` definitions, etc.
#[derive(Debug)]
@@ -880,7 +878,7 @@ fn resolve_macro_or_delegation_path<'r>(
let res = res?;
let ext = match deleg_impl {
Some((impl_def_id, star_span)) => match res {
def::Res::Def(DefKind::Trait, def_id) => {
Res::Def(DefKind::Trait, def_id) => {
let edition = self.tcx.sess.edition();
Some(Arc::new(SyntaxExtension::glob_delegation(
def_id,