Skip to content

Commit

Permalink
Keep track of parse errors in mods and don't emit resolve errors fo…
Browse files Browse the repository at this point in the history
…r 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. 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 `mod` expansion.

Fix rust-lang#97734.
  • Loading branch information
estebank committed Dec 7, 2024
1 parent 8eb65ad commit d0955c5
Show file tree
Hide file tree
Showing 28 changed files with 132 additions and 94 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 @@ -2872,7 +2872,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, bool),
/// 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 @@ -1211,7 +1211,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
8 changes: 5 additions & 3 deletions compiler/rustc_ast_lowering/src/item.rs
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ impl<'a, 'hir> ItemLowerer<'a, 'hir> {
fn lower_crate(&mut self, c: &Crate) {
debug_assert_eq!(self.resolver.node_id_to_def_id[&CRATE_NODE_ID], CRATE_DEF_ID);
self.with_lctx(CRATE_NODE_ID, |lctx| {
let module = lctx.lower_mod(&c.items, &c.spans);
let module = lctx.lower_mod(&c.items, &c.spans, false);
lctx.lower_attrs(hir::CRATE_HIR_ID, &c.attrs);
hir::OwnerNode::Crate(module)
})
Expand Down Expand Up @@ -118,13 +118,15 @@ impl<'hir> LoweringContext<'_, 'hir> {
&mut self,
items: &[P<Item>],
spans: &ModSpans,
had_parse_errors: bool,
) -> &'hir hir::Mod<'hir> {
self.arena.alloc(hir::Mod {
spans: hir::ModSpans {
inner_span: self.lower_span(spans.inner_span),
inject_use_span: self.lower_span(spans.inject_use_span),
},
item_ids: self.arena.alloc_from_iter(items.iter().flat_map(|x| self.lower_item_ref(x))),
had_parse_errors,
})
}

Expand Down Expand Up @@ -235,8 +237,8 @@ impl<'hir> LoweringContext<'_, 'hir> {
})
}
ItemKind::Mod(_, mod_kind) => match mod_kind {
ModKind::Loaded(items, _, spans) => {
hir::ItemKind::Mod(self.lower_mod(items, spans))
ModKind::Loaded(items, _, spans, had_parse_errors) => {
hir::ItemKind::Mod(self.lower_mod(items, spans, *had_parse_errors))
}
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
5 changes: 4 additions & 1 deletion 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: bool,
}

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

// (1) ...instead, we return a dummy module.
let (items, spans, file_path) =
result.map_err(|err| err.report(sess, span)).unwrap_or_default();

// 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
1 change: 1 addition & 0 deletions compiler/rustc_hir/src/hir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3075,6 +3075,7 @@ pub enum ClosureBinder {
pub struct Mod<'hir> {
pub spans: ModSpans,
pub item_ids: &'hir [ItemId],
pub had_parse_errors: bool,
}

#[derive(Copy, Clone, Debug, HashStable_Generic)]
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 @@ -3037,7 +3037,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, false)
};
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(_, _, _, true) = mod_kind {
self.r.mod_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.mod_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
1 change: 1 addition & 0 deletions compiler/rustc_resolve/src/imports.rs
Original file line number Diff line number Diff line change
Expand Up @@ -898,6 +898,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 @@ -4376,6 +4376,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 @@ -4424,6 +4430,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 d0955c5

Please sign in to comment.