Skip to content

Commit

Permalink
Rollup merge of rust-lang#133937 - estebank:silence-resolve-errors-fr…
Browse files Browse the repository at this point in the history
…om-mod-with-parse-errors, r=davidtwco

Keep track of parse errors in `mod`s and don't emit resolve errors for paths involving them

When we expand a `mod foo;` and parse `foo.rs`, we now track whether that file had an unrecovered parse error that reached the end of the file. If so, we keep that information around in the HIR and mark its `DefId` in the `Resolver`. When resolving a path like `foo::bar`, we do not emit any errors for "`bar` not found in `foo`", as we know that the parse error might have caused `bar` to not be parsed and accounted for.

When this happens in an existing project, every path referencing `foo` would be an irrelevant compile error. Instead, we now skip emitting anything until `foo.rs` is fixed. Tellingly enough, we didn't have any test for errors caused by expansion of `mod`s with parse errors.

Fix rust-lang#97734.
  • Loading branch information
matthiaskrgr authored Dec 12, 2024
2 parents 0c0b1de + 27420c6 commit 3c266aa
Show file tree
Hide file tree
Showing 28 changed files with 178 additions and 85 deletions.
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

0 comments on commit 3c266aa

Please sign in to comment.