Skip to content

Commit 6d45da4

Browse files
estebankestebank
committed
Keep track of parse errors in mods 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. 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.
1 parent 77c7410 commit 6d45da4

File tree

28 files changed

+150
-112
lines changed

28 files changed

+150
-112
lines changed

‎compiler/rustc_ast/src/ast.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -2860,7 +2860,7 @@ pub enum ModKind {
28602860
/// or with definition outlined to a separate file `mod foo;` and already loaded from it.
28612861
/// The inner span is from the first token past `{` to the last token until `}`,
28622862
/// or from the first to the last token in the loaded file.
2863-
Loaded(ThinVec<P<Item>>, Inline, ModSpans),
2863+
Loaded(ThinVec<P<Item>>, Inline, ModSpans, bool),
28642864
/// Module with definition outlined to a separate file `mod foo;` but not yet loaded from it.
28652865
Unloaded,
28662866
}

‎compiler/rustc_ast/src/mut_visit.rs

+6-1
Original file line numberDiff line numberDiff line change
@@ -1211,7 +1211,12 @@ impl WalkItemKind for ItemKind {
12111211
ItemKind::Mod(safety, mod_kind) => {
12121212
visit_safety(vis, safety);
12131213
match mod_kind {
1214-
ModKind::Loaded(items, _inline, ModSpans { inner_span, inject_use_span }) => {
1214+
ModKind::Loaded(
1215+
items,
1216+
_inline,
1217+
ModSpans { inner_span, inject_use_span },
1218+
_,
1219+
) => {
12151220
items.flat_map_in_place(|item| vis.flat_map_item(item));
12161221
vis.visit_span(inner_span);
12171222
vis.visit_span(inject_use_span);

‎compiler/rustc_ast/src/visit.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -380,7 +380,7 @@ impl WalkItemKind for ItemKind {
380380
try_visit!(visitor.visit_fn(kind, span, id));
381381
}
382382
ItemKind::Mod(_unsafety, mod_kind) => match mod_kind {
383-
ModKind::Loaded(items, _inline, _inner_span) => {
383+
ModKind::Loaded(items, _inline, _inner_span, _) => {
384384
walk_list!(visitor, visit_item, items);
385385
}
386386
ModKind::Unloaded => {}

‎compiler/rustc_ast_lowering/src/item.rs

+5-3
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@ impl<'a, 'hir> ItemLowerer<'a, 'hir> {
9090
fn lower_crate(&mut self, c: &Crate) {
9191
debug_assert_eq!(self.resolver.node_id_to_def_id[&CRATE_NODE_ID], CRATE_DEF_ID);
9292
self.with_lctx(CRATE_NODE_ID, |lctx| {
93-
let module = lctx.lower_mod(&c.items, &c.spans);
93+
let module = lctx.lower_mod(&c.items, &c.spans, false);
9494
lctx.lower_attrs(hir::CRATE_HIR_ID, &c.attrs);
9595
hir::OwnerNode::Crate(module)
9696
})
@@ -118,13 +118,15 @@ impl<'hir> LoweringContext<'_, 'hir> {
118118
&mut self,
119119
items: &[P<Item>],
120120
spans: &ModSpans,
121+
had_parse_errors: bool,
121122
) -> &'hir hir::Mod<'hir> {
122123
self.arena.alloc(hir::Mod {
123124
spans: hir::ModSpans {
124125
inner_span: self.lower_span(spans.inner_span),
125126
inject_use_span: self.lower_span(spans.inject_use_span),
126127
},
127128
item_ids: self.arena.alloc_from_iter(items.iter().flat_map(|x| self.lower_item_ref(x))),
129+
had_parse_errors,
128130
})
129131
}
130132

@@ -235,8 +237,8 @@ impl<'hir> LoweringContext<'_, 'hir> {
235237
})
236238
}
237239
ItemKind::Mod(_, mod_kind) => match mod_kind {
238-
ModKind::Loaded(items, _, spans) => {
239-
hir::ItemKind::Mod(self.lower_mod(items, spans))
240+
ModKind::Loaded(items, _, spans, had_parse_errors) => {
241+
hir::ItemKind::Mod(self.lower_mod(items, spans, *had_parse_errors))
240242
}
241243
ModKind::Unloaded => panic!("`mod` items should have been loaded by now"),
242244
},

‎compiler/rustc_ast_passes/src/ast_validation.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -1029,7 +1029,7 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
10291029
self.dcx().emit_err(errors::UnsafeItem { span, kind: "module" });
10301030
}
10311031
// Ensure that `path` attributes on modules are recorded as used (cf. issue #35584).
1032-
if !matches!(mod_kind, ModKind::Loaded(_, Inline::Yes, _))
1032+
if !matches!(mod_kind, ModKind::Loaded(_, Inline::Yes, _, _))
10331033
&& !attr::contains_name(&item.attrs, sym::path)
10341034
{
10351035
self.check_mod_file_item_asciionly(item.ident);

‎compiler/rustc_builtin_macros/src/test_harness.rs

+4-2
Original file line numberDiff line numberDiff line change
@@ -141,8 +141,10 @@ impl<'a> MutVisitor for TestHarnessGenerator<'a> {
141141

142142
// We don't want to recurse into anything other than mods, since
143143
// mods or tests inside of functions will break things
144-
if let ast::ItemKind::Mod(_, ModKind::Loaded(.., ast::ModSpans { inner_span: span, .. })) =
145-
item.kind
144+
if let ast::ItemKind::Mod(
145+
_,
146+
ModKind::Loaded(.., ast::ModSpans { inner_span: span, .. }, _),
147+
) = item.kind
146148
{
147149
let prev_tests = mem::take(&mut self.tests);
148150
walk_item_kind(

‎compiler/rustc_expand/src/expand.rs

+19-13
Original file line numberDiff line numberDiff line change
@@ -723,7 +723,7 @@ impl<'a, 'b> MacroExpander<'a, 'b> {
723723
item_inner.kind,
724724
ItemKind::Mod(
725725
_,
726-
ModKind::Unloaded | ModKind::Loaded(_, Inline::No, _),
726+
ModKind::Unloaded | ModKind::Loaded(_, Inline::No, _, _),
727727
)
728728
) =>
729729
{
@@ -889,7 +889,7 @@ impl<'a, 'b> MacroExpander<'a, 'b> {
889889
fn visit_item(&mut self, item: &'ast ast::Item) {
890890
match &item.kind {
891891
ItemKind::Mod(_, mod_kind)
892-
if !matches!(mod_kind, ModKind::Loaded(_, Inline::Yes, _)) =>
892+
if !matches!(mod_kind, ModKind::Loaded(_, Inline::Yes, _, _)) =>
893893
{
894894
feature_err(
895895
self.sess,
@@ -1195,7 +1195,7 @@ impl InvocationCollectorNode for P<ast::Item> {
11951195

11961196
let ecx = &mut collector.cx;
11971197
let (file_path, dir_path, dir_ownership) = match mod_kind {
1198-
ModKind::Loaded(_, inline, _) => {
1198+
ModKind::Loaded(_, inline, _, _) => {
11991199
// Inline `mod foo { ... }`, but we still need to push directories.
12001200
let (dir_path, dir_ownership) = mod_dir_path(
12011201
ecx.sess,
@@ -1217,15 +1217,21 @@ impl InvocationCollectorNode for P<ast::Item> {
12171217
ModKind::Unloaded => {
12181218
// We have an outline `mod foo;` so we need to parse the file.
12191219
let old_attrs_len = attrs.len();
1220-
let ParsedExternalMod { items, spans, file_path, dir_path, dir_ownership } =
1221-
parse_external_mod(
1222-
ecx.sess,
1223-
ident,
1224-
span,
1225-
&ecx.current_expansion.module,
1226-
ecx.current_expansion.dir_ownership,
1227-
&mut attrs,
1228-
);
1220+
let ParsedExternalMod {
1221+
items,
1222+
spans,
1223+
file_path,
1224+
dir_path,
1225+
dir_ownership,
1226+
had_parse_error,
1227+
} = parse_external_mod(
1228+
ecx.sess,
1229+
ident,
1230+
span,
1231+
&ecx.current_expansion.module,
1232+
ecx.current_expansion.dir_ownership,
1233+
&mut attrs,
1234+
);
12291235

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

1242-
*mod_kind = ModKind::Loaded(items, Inline::No, spans);
1248+
*mod_kind = ModKind::Loaded(items, Inline::No, spans, had_parse_error);
12431249
node.attrs = attrs;
12441250
if node.attrs.len() > old_attrs_len {
12451251
// If we loaded an out-of-line module and added some inner attributes,

‎compiler/rustc_expand/src/module.rs

+4-1
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ pub(crate) struct ParsedExternalMod {
3737
pub file_path: PathBuf,
3838
pub dir_path: PathBuf,
3939
pub dir_ownership: DirOwnership,
40+
pub had_parse_error: bool,
4041
}
4142

4243
pub enum ModError<'a> {
@@ -74,14 +75,16 @@ pub(crate) fn parse_external_mod(
7475
attrs.extend(inner_attrs);
7576
(items, inner_span, mp.file_path)
7677
};
78+
let had_parse_error = result.is_err();
79+
7780
// (1) ...instead, we return a dummy module.
7881
let (items, spans, file_path) =
7982
result.map_err(|err| err.report(sess, span)).unwrap_or_default();
8083

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

84-
ParsedExternalMod { items, spans, file_path, dir_path, dir_ownership }
87+
ParsedExternalMod { items, spans, file_path, dir_path, dir_ownership, had_parse_error }
8588
}
8689

8790
pub(crate) fn mod_dir_path(

‎compiler/rustc_hir/src/hir.rs

+1
Original file line numberDiff line numberDiff line change
@@ -3075,6 +3075,7 @@ pub enum ClosureBinder {
30753075
pub struct Mod<'hir> {
30763076
pub spans: ModSpans,
30773077
pub item_ids: &'hir [ItemId],
3078+
pub had_parse_errors: bool,
30783079
}
30793080

30803081
#[derive(Copy, Clone, Debug, HashStable_Generic)]

‎compiler/rustc_lint/src/builtin.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -3037,7 +3037,7 @@ impl EarlyLintPass for SpecialModuleName {
30373037
for item in &krate.items {
30383038
if let ast::ItemKind::Mod(
30393039
_,
3040-
ast::ModKind::Unloaded | ast::ModKind::Loaded(_, ast::Inline::No, _),
3040+
ast::ModKind::Unloaded | ast::ModKind::Loaded(_, ast::Inline::No, _, _),
30413041
) = item.kind
30423042
{
30433043
if item.attrs.iter().any(|a| a.has_name(sym::path)) {

‎compiler/rustc_parse/src/parser/item.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ impl<'a> Parser<'a> {
4545
let (inner_attrs, items, inner_span) =
4646
self.parse_mod(&token::CloseDelim(Delimiter::Brace))?;
4747
attrs.extend(inner_attrs);
48-
ModKind::Loaded(items, Inline::Yes, inner_span)
48+
ModKind::Loaded(items, Inline::Yes, inner_span, false)
4949
};
5050
Ok((id, ItemKind::Mod(safety, mod_kind)))
5151
}

‎compiler/rustc_resolve/src/build_reduced_graph.rs

+5-1
Original file line numberDiff line numberDiff line change
@@ -770,7 +770,7 @@ impl<'a, 'ra, 'tcx> BuildReducedGraphVisitor<'a, 'ra, 'tcx> {
770770
);
771771
}
772772

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

784+
if let ast::ModKind::Loaded(_, _, _, true) = mod_kind {
785+
self.r.mod_with_parse_errors.insert(def_id);
786+
}
787+
784788
// Descend into the module.
785789
self.parent_scope.module = module;
786790
}

‎compiler/rustc_resolve/src/diagnostics.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -3056,7 +3056,7 @@ impl<'tcx> visit::Visitor<'tcx> for UsePlacementFinder {
30563056

30573057
fn visit_item(&mut self, item: &'tcx ast::Item) {
30583058
if self.target_module == item.id {
3059-
if let ItemKind::Mod(_, ModKind::Loaded(items, _inline, mod_spans)) = &item.kind {
3059+
if let ItemKind::Mod(_, ModKind::Loaded(items, _inline, mod_spans, _)) = &item.kind {
30603060
let inject = mod_spans.inject_use_span;
30613061
if is_span_suitable_for_use_injection(inject) {
30623062
self.first_legal_span = Some(inject);

‎compiler/rustc_resolve/src/ident.rs

+53-29
Original file line numberDiff line numberDiff line change
@@ -1428,6 +1428,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
14281428
ignore_import: Option<Import<'ra>>,
14291429
) -> PathResult<'ra> {
14301430
let mut module = None;
1431+
let mut module_had_parse_errors = false;
14311432
let mut allow_super = true;
14321433
let mut second_binding = None;
14331434

@@ -1471,9 +1472,14 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
14711472
continue;
14721473
}
14731474
}
1474-
return PathResult::failed(ident, false, finalize.is_some(), module, || {
1475-
("there are too many leading `super` keywords".to_string(), None)
1476-
});
1475+
return PathResult::failed(
1476+
ident,
1477+
false,
1478+
finalize.is_some(),
1479+
module_had_parse_errors,
1480+
module,
1481+
|| ("there are too many leading `super` keywords".to_string(), None),
1482+
);
14771483
}
14781484
if segment_idx == 0 {
14791485
if name == kw::SelfLower {
@@ -1511,19 +1517,26 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
15111517

15121518
// Report special messages for path segment keywords in wrong positions.
15131519
if ident.is_path_segment_keyword() && segment_idx != 0 {
1514-
return PathResult::failed(ident, false, finalize.is_some(), module, || {
1515-
let name_str = if name == kw::PathRoot {
1516-
"crate root".to_string()
1517-
} else {
1518-
format!("`{name}`")
1519-
};
1520-
let label = if segment_idx == 1 && path[0].ident.name == kw::PathRoot {
1521-
format!("global paths cannot start with {name_str}")
1522-
} else {
1523-
format!("{name_str} in paths can only be used in start position")
1524-
};
1525-
(label, None)
1526-
});
1520+
return PathResult::failed(
1521+
ident,
1522+
false,
1523+
finalize.is_some(),
1524+
module_had_parse_errors,
1525+
module,
1526+
|| {
1527+
let name_str = if name == kw::PathRoot {
1528+
"crate root".to_string()
1529+
} else {
1530+
format!("`{name}`")
1531+
};
1532+
let label = if segment_idx == 1 && path[0].ident.name == kw::PathRoot {
1533+
format!("global paths cannot start with {name_str}")
1534+
} else {
1535+
format!("{name_str} in paths can only be used in start position")
1536+
};
1537+
(label, None)
1538+
},
1539+
);
15271540
}
15281541

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

15901603
let maybe_assoc = opt_ns != Some(MacroNS) && PathSource::Type.is_expected(res);
15911604
if let Some(next_module) = binding.module() {
1605+
if self.mod_with_parse_errors.contains(&next_module.def_id()) {
1606+
module_had_parse_errors = true;
1607+
}
15921608
module = Some(ModuleOrUniformRoot::Module(next_module));
15931609
record_segment_res(self, res);
15941610
} else if res == Res::ToolMod && !is_last && opt_ns.is_some() {
@@ -1614,6 +1630,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
16141630
ident,
16151631
is_last,
16161632
finalize.is_some(),
1633+
module_had_parse_errors,
16171634
module,
16181635
|| {
16191636
let label = format!(
@@ -1637,19 +1654,26 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
16371654
}
16381655
}
16391656

1640-
return PathResult::failed(ident, is_last, finalize.is_some(), module, || {
1641-
self.report_path_resolution_error(
1642-
path,
1643-
opt_ns,
1644-
parent_scope,
1645-
ribs,
1646-
ignore_binding,
1647-
ignore_import,
1648-
module,
1649-
segment_idx,
1650-
ident,
1651-
)
1652-
});
1657+
return PathResult::failed(
1658+
ident,
1659+
is_last,
1660+
finalize.is_some(),
1661+
module_had_parse_errors,
1662+
module,
1663+
|| {
1664+
self.report_path_resolution_error(
1665+
path,
1666+
opt_ns,
1667+
parent_scope,
1668+
ribs,
1669+
ignore_binding,
1670+
ignore_import,
1671+
module,
1672+
segment_idx,
1673+
ident,
1674+
)
1675+
},
1676+
);
16531677
}
16541678
}
16551679
}

‎compiler/rustc_resolve/src/imports.rs

+1
Original file line numberDiff line numberDiff line change
@@ -898,6 +898,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
898898
label,
899899
suggestion,
900900
module,
901+
error_implied_by_parse_error: _,
901902
} => {
902903
if no_ambiguity {
903904
assert!(import.imported_module.get().is_none());

‎compiler/rustc_resolve/src/late.rs

+7
Original file line numberDiff line numberDiff line change
@@ -4376,6 +4376,12 @@ impl<'a, 'ast, 'ra: 'ast, 'tcx> LateResolutionVisitor<'a, 'ast, 'ra, 'tcx> {
43764376
PathResult::Module(ModuleOrUniformRoot::Module(module)) if !module.is_normal() => {
43774377
PartialRes::new(module.res().unwrap())
43784378
}
4379+
// A part of this path references a `mod` that had a parse error. To avoid resolution
4380+
// errors for each reference to that module, we don't emit an error for them until the
4381+
// `mod` is fixed. this can have a significant cascade effect.
4382+
PathResult::Failed { error_implied_by_parse_error: true, .. } => {
4383+
PartialRes::new(Res::Err)
4384+
}
43794385
// In `a(::assoc_item)*` `a` cannot be a module. If `a` does resolve to a module we
43804386
// don't report an error right away, but try to fallback to a primitive type.
43814387
// So, we are still able to successfully resolve something like
@@ -4424,6 +4430,7 @@ impl<'a, 'ast, 'ra: 'ast, 'tcx> LateResolutionVisitor<'a, 'ast, 'ra, 'tcx> {
44244430
suggestion,
44254431
module,
44264432
segment_name,
4433+
error_implied_by_parse_error: _,
44274434
} => {
44284435
return Err(respan(span, ResolutionError::FailedToResolve {
44294436
segment: Some(segment_name),

0 commit comments

Comments
 (0)