Skip to content

Keep track of parse errors in mods and don't emit resolve errors for paths involving them #133937

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

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
2 changes: 1 addition & 1 deletion compiler/rustc_ast/src/ast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2877,7 +2877,7 @@ pub enum ModKind {
/// or with definition outlined to a separate file `mod foo;` and already loaded from it.
/// The inner span is from the first token past `{` to the last token until `}`,
/// or from the first to the last token in the loaded file.
Loaded(ThinVec<P<Item>>, Inline, ModSpans),
Loaded(ThinVec<P<Item>>, Inline, ModSpans, Result<(), ErrorGuaranteed>),
/// Module with definition outlined to a separate file `mod foo;` but not yet loaded from it.
Unloaded,
}
Expand Down
7 changes: 6 additions & 1 deletion compiler/rustc_ast/src/mut_visit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1212,7 +1212,12 @@ impl WalkItemKind for ItemKind {
ItemKind::Mod(safety, mod_kind) => {
visit_safety(vis, safety);
match mod_kind {
ModKind::Loaded(items, _inline, ModSpans { inner_span, inject_use_span }) => {
ModKind::Loaded(
items,
_inline,
ModSpans { inner_span, inject_use_span },
_,
) => {
items.flat_map_in_place(|item| vis.flat_map_item(item));
vis.visit_span(inner_span);
vis.visit_span(inject_use_span);
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_ast/src/visit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -380,7 +380,7 @@ impl WalkItemKind for ItemKind {
try_visit!(visitor.visit_fn(kind, span, id));
}
ItemKind::Mod(_unsafety, mod_kind) => match mod_kind {
ModKind::Loaded(items, _inline, _inner_span) => {
ModKind::Loaded(items, _inline, _inner_span, _) => {
walk_list!(visitor, visit_item, items);
}
ModKind::Unloaded => {}
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_ast_lowering/src/item.rs
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
})
}
ItemKind::Mod(_, mod_kind) => match mod_kind {
ModKind::Loaded(items, _, spans) => {
ModKind::Loaded(items, _, spans, _) => {
hir::ItemKind::Mod(self.lower_mod(items, spans))
}
ModKind::Unloaded => panic!("`mod` items should have been loaded by now"),
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_ast_passes/src/ast_validation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1029,7 +1029,7 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
self.dcx().emit_err(errors::UnsafeItem { span, kind: "module" });
}
// Ensure that `path` attributes on modules are recorded as used (cf. issue #35584).
if !matches!(mod_kind, ModKind::Loaded(_, Inline::Yes, _))
if !matches!(mod_kind, ModKind::Loaded(_, Inline::Yes, _, _))
&& !attr::contains_name(&item.attrs, sym::path)
{
self.check_mod_file_item_asciionly(item.ident);
Expand Down
6 changes: 4 additions & 2 deletions compiler/rustc_builtin_macros/src/test_harness.rs
Original file line number Diff line number Diff line change
Expand Up @@ -141,8 +141,10 @@ impl<'a> MutVisitor for TestHarnessGenerator<'a> {

// We don't want to recurse into anything other than mods, since
// mods or tests inside of functions will break things
if let ast::ItemKind::Mod(_, ModKind::Loaded(.., ast::ModSpans { inner_span: span, .. })) =
item.kind
if let ast::ItemKind::Mod(
_,
ModKind::Loaded(.., ast::ModSpans { inner_span: span, .. }, _),
) = item.kind
{
let prev_tests = mem::take(&mut self.tests);
walk_item_kind(
Expand Down
32 changes: 19 additions & 13 deletions compiler/rustc_expand/src/expand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -723,7 +723,7 @@ impl<'a, 'b> MacroExpander<'a, 'b> {
item_inner.kind,
ItemKind::Mod(
_,
ModKind::Unloaded | ModKind::Loaded(_, Inline::No, _),
ModKind::Unloaded | ModKind::Loaded(_, Inline::No, _, _),
)
) =>
{
Expand Down Expand Up @@ -889,7 +889,7 @@ impl<'a, 'b> MacroExpander<'a, 'b> {
fn visit_item(&mut self, item: &'ast ast::Item) {
match &item.kind {
ItemKind::Mod(_, mod_kind)
if !matches!(mod_kind, ModKind::Loaded(_, Inline::Yes, _)) =>
if !matches!(mod_kind, ModKind::Loaded(_, Inline::Yes, _, _)) =>
{
feature_err(
self.sess,
Expand Down Expand Up @@ -1195,7 +1195,7 @@ impl InvocationCollectorNode for P<ast::Item> {

let ecx = &mut collector.cx;
let (file_path, dir_path, dir_ownership) = match mod_kind {
ModKind::Loaded(_, inline, _) => {
ModKind::Loaded(_, inline, _, _) => {
// Inline `mod foo { ... }`, but we still need to push directories.
let (dir_path, dir_ownership) = mod_dir_path(
ecx.sess,
Expand All @@ -1217,15 +1217,21 @@ impl InvocationCollectorNode for P<ast::Item> {
ModKind::Unloaded => {
// We have an outline `mod foo;` so we need to parse the file.
let old_attrs_len = attrs.len();
let ParsedExternalMod { items, spans, file_path, dir_path, dir_ownership } =
parse_external_mod(
ecx.sess,
ident,
span,
&ecx.current_expansion.module,
ecx.current_expansion.dir_ownership,
&mut attrs,
);
let ParsedExternalMod {
items,
spans,
file_path,
dir_path,
dir_ownership,
had_parse_error,
} = parse_external_mod(
ecx.sess,
ident,
span,
&ecx.current_expansion.module,
ecx.current_expansion.dir_ownership,
&mut attrs,
);

if let Some(lint_store) = ecx.lint_store {
lint_store.pre_expansion_lint(
Expand All @@ -1239,7 +1245,7 @@ impl InvocationCollectorNode for P<ast::Item> {
);
}

*mod_kind = ModKind::Loaded(items, Inline::No, spans);
*mod_kind = ModKind::Loaded(items, Inline::No, spans, had_parse_error);
node.attrs = attrs;
if node.attrs.len() > old_attrs_len {
// If we loaded an out-of-line module and added some inner attributes,
Expand Down
10 changes: 7 additions & 3 deletions compiler/rustc_expand/src/module.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ pub(crate) struct ParsedExternalMod {
pub file_path: PathBuf,
pub dir_path: PathBuf,
pub dir_ownership: DirOwnership,
pub had_parse_error: Result<(), ErrorGuaranteed>,
}

pub enum ModError<'a> {
Expand Down Expand Up @@ -74,14 +75,17 @@ pub(crate) fn parse_external_mod(
attrs.extend(inner_attrs);
(items, inner_span, mp.file_path)
};

// (1) ...instead, we return a dummy module.
let (items, spans, file_path) =
result.map_err(|err| err.report(sess, span)).unwrap_or_default();
let ((items, spans, file_path), had_parse_error) = match result {
Err(err) => (Default::default(), Err(err.report(sess, span))),
Ok(result) => (result, Ok(())),
};

// Extract the directory path for submodules of the module.
let dir_path = file_path.parent().unwrap_or(&file_path).to_owned();

ParsedExternalMod { items, spans, file_path, dir_path, dir_ownership }
ParsedExternalMod { items, spans, file_path, dir_path, dir_ownership, had_parse_error }
}

pub(crate) fn mod_dir_path(
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_lint/src/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3038,7 +3038,7 @@ impl EarlyLintPass for SpecialModuleName {
for item in &krate.items {
if let ast::ItemKind::Mod(
_,
ast::ModKind::Unloaded | ast::ModKind::Loaded(_, ast::Inline::No, _),
ast::ModKind::Unloaded | ast::ModKind::Loaded(_, ast::Inline::No, _, _),
) = item.kind
{
if item.attrs.iter().any(|a| a.has_name(sym::path)) {
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_parse/src/parser/item.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ impl<'a> Parser<'a> {
let (inner_attrs, items, inner_span) =
self.parse_mod(&token::CloseDelim(Delimiter::Brace))?;
attrs.extend(inner_attrs);
ModKind::Loaded(items, Inline::Yes, inner_span)
ModKind::Loaded(items, Inline::Yes, inner_span, Ok(()))
};
Ok((id, ItemKind::Mod(safety, mod_kind)))
}
Expand Down
6 changes: 5 additions & 1 deletion compiler/rustc_resolve/src/build_reduced_graph.rs
Original file line number Diff line number Diff line change
Expand Up @@ -770,7 +770,7 @@ impl<'a, 'ra, 'tcx> BuildReducedGraphVisitor<'a, 'ra, 'tcx> {
);
}

ItemKind::Mod(..) => {
ItemKind::Mod(.., ref mod_kind) => {
let module = self.r.new_module(
Some(parent),
ModuleKind::Def(def_kind, def_id, ident.name),
Expand All @@ -781,6 +781,10 @@ impl<'a, 'ra, 'tcx> BuildReducedGraphVisitor<'a, 'ra, 'tcx> {
);
self.r.define(parent, ident, TypeNS, (module, vis, sp, expansion));

if let ast::ModKind::Loaded(_, _, _, Err(_)) = mod_kind {
self.r.mods_with_parse_errors.insert(def_id);
}

// Descend into the module.
self.parent_scope.module = module;
}
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_resolve/src/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3056,7 +3056,7 @@ impl<'tcx> visit::Visitor<'tcx> for UsePlacementFinder {

fn visit_item(&mut self, item: &'tcx ast::Item) {
if self.target_module == item.id {
if let ItemKind::Mod(_, ModKind::Loaded(items, _inline, mod_spans)) = &item.kind {
if let ItemKind::Mod(_, ModKind::Loaded(items, _inline, mod_spans, _)) = &item.kind {
let inject = mod_spans.inject_use_span;
if is_span_suitable_for_use_injection(inject) {
self.first_legal_span = Some(inject);
Expand Down
82 changes: 53 additions & 29 deletions compiler/rustc_resolve/src/ident.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1428,6 +1428,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
ignore_import: Option<Import<'ra>>,
) -> PathResult<'ra> {
let mut module = None;
let mut module_had_parse_errors = false;
let mut allow_super = true;
let mut second_binding = None;

Expand Down Expand Up @@ -1471,9 +1472,14 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
continue;
}
}
return PathResult::failed(ident, false, finalize.is_some(), module, || {
("there are too many leading `super` keywords".to_string(), None)
});
return PathResult::failed(
ident,
false,
finalize.is_some(),
module_had_parse_errors,
module,
|| ("there are too many leading `super` keywords".to_string(), None),
);
}
if segment_idx == 0 {
if name == kw::SelfLower {
Expand Down Expand Up @@ -1511,19 +1517,26 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {

// Report special messages for path segment keywords in wrong positions.
if ident.is_path_segment_keyword() && segment_idx != 0 {
return PathResult::failed(ident, false, finalize.is_some(), module, || {
let name_str = if name == kw::PathRoot {
"crate root".to_string()
} else {
format!("`{name}`")
};
let label = if segment_idx == 1 && path[0].ident.name == kw::PathRoot {
format!("global paths cannot start with {name_str}")
} else {
format!("{name_str} in paths can only be used in start position")
};
(label, None)
});
return PathResult::failed(
ident,
false,
finalize.is_some(),
module_had_parse_errors,
module,
|| {
let name_str = if name == kw::PathRoot {
"crate root".to_string()
} else {
format!("`{name}`")
};
let label = if segment_idx == 1 && path[0].ident.name == kw::PathRoot {
format!("global paths cannot start with {name_str}")
} else {
format!("{name_str} in paths can only be used in start position")
};
(label, None)
},
);
}

let binding = if let Some(module) = module {
Expand Down Expand Up @@ -1589,6 +1602,9 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {

let maybe_assoc = opt_ns != Some(MacroNS) && PathSource::Type.is_expected(res);
if let Some(next_module) = binding.module() {
if self.mods_with_parse_errors.contains(&next_module.def_id()) {
module_had_parse_errors = true;
}
module = Some(ModuleOrUniformRoot::Module(next_module));
record_segment_res(self, res);
} else if res == Res::ToolMod && !is_last && opt_ns.is_some() {
Expand All @@ -1614,6 +1630,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
ident,
is_last,
finalize.is_some(),
module_had_parse_errors,
module,
|| {
let label = format!(
Expand All @@ -1637,19 +1654,26 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
}
}

return PathResult::failed(ident, is_last, finalize.is_some(), module, || {
self.report_path_resolution_error(
path,
opt_ns,
parent_scope,
ribs,
ignore_binding,
ignore_import,
module,
segment_idx,
ident,
)
});
return PathResult::failed(
ident,
is_last,
finalize.is_some(),
module_had_parse_errors,
module,
|| {
self.report_path_resolution_error(
path,
opt_ns,
parent_scope,
ribs,
ignore_binding,
ignore_import,
module,
segment_idx,
ident,
)
},
);
}
}
}
Expand Down
8 changes: 7 additions & 1 deletion compiler/rustc_resolve/src/imports.rs
Original file line number Diff line number Diff line change
Expand Up @@ -670,9 +670,14 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {

fn throw_unresolved_import_error(
&mut self,
errors: Vec<(Import<'_>, UnresolvedImportError)>,
mut errors: Vec<(Import<'_>, UnresolvedImportError)>,
glob_error: bool,
) {
errors.retain(|(_import, err)| match err.module {
// Skip `use` errors for `use foo::Bar;` if `foo.rs` has unrecovered parse errors.
Some(def_id) if self.mods_with_parse_errors.contains(&def_id) => false,
_ => true,
});
if errors.is_empty() {
return;
}
Expand Down Expand Up @@ -898,6 +903,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
label,
suggestion,
module,
error_implied_by_parse_error: _,
} => {
if no_ambiguity {
assert!(import.imported_module.get().is_none());
Expand Down
7 changes: 7 additions & 0 deletions compiler/rustc_resolve/src/late.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4395,6 +4395,12 @@ impl<'a, 'ast, 'ra: 'ast, 'tcx> LateResolutionVisitor<'a, 'ast, 'ra, 'tcx> {
PathResult::Module(ModuleOrUniformRoot::Module(module)) if !module.is_normal() => {
PartialRes::new(module.res().unwrap())
}
// A part of this path references a `mod` that had a parse error. To avoid resolution
// errors for each reference to that module, we don't emit an error for them until the
// `mod` is fixed. this can have a significant cascade effect.
PathResult::Failed { error_implied_by_parse_error: true, .. } => {
PartialRes::new(Res::Err)
}
// In `a(::assoc_item)*` `a` cannot be a module. If `a` does resolve to a module we
// don't report an error right away, but try to fallback to a primitive type.
// So, we are still able to successfully resolve something like
Expand Down Expand Up @@ -4443,6 +4449,7 @@ impl<'a, 'ast, 'ra: 'ast, 'tcx> LateResolutionVisitor<'a, 'ast, 'ra, 'tcx> {
suggestion,
module,
segment_name,
error_implied_by_parse_error: _,
} => {
return Err(respan(span, ResolutionError::FailedToResolve {
segment: Some(segment_name),
Expand Down
Loading
Loading