Skip to content

Commit

Permalink
On outer attr not found, avoid knock-down errors
Browse files Browse the repository at this point in the history
When we encounter code like

```rust
#![non_existent]

#[derive(Clone)]
struct S;

fn main() {
    S.clone();
}
```

we avoid emitting errors about the `derive` (which isn't early resolved
as normal, but *has* a later resolution) and stop the compiler from
advancing to later stages, to avoid knock down errors from the `derive`
not being evaluated. Recovering these would be possible, but would
produce incorrect errors in the cases where had the outer attribute been
present it would have modified the behavior/evaluation of the `derive`
attributes.

Fix #118455.
  • Loading branch information
estebank committed Dec 1, 2023
1 parent 25c75bc commit 9e404df
Show file tree
Hide file tree
Showing 7 changed files with 49 additions and 19 deletions.
1 change: 1 addition & 0 deletions compiler/rustc_resolve/src/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1039,6 +1039,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
if let Ok((Some(ext), _)) = this.resolve_macro_path(
derive,
Some(MacroKind::Derive),
false,
parent_scope,
false,
false,
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_resolve/src/ident.rs
Original file line number Diff line number Diff line change
Expand Up @@ -443,6 +443,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
match this.resolve_macro_path(
derive,
Some(MacroKind::Derive),
false,
parent_scope,
true,
force,
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_resolve/src/late.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3869,7 +3869,7 @@ impl<'a: 'ast, 'b, 'ast, 'tcx> LateResolutionVisitor<'a, 'b, 'ast, 'tcx> {
let path_seg = |seg: &Segment| PathSegment::from_ident(seg.ident);
let path = Path { segments: path.iter().map(path_seg).collect(), span, tokens: None };
if let Ok((_, res)) =
self.r.resolve_macro_path(&path, None, &self.parent_scope, false, false)
self.r.resolve_macro_path(&path, None, false, &self.parent_scope, false, false)
{
return Ok(Some(PartialRes::new(res)));
}
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_resolve/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1047,7 +1047,7 @@ pub struct Resolver<'a, 'tcx> {
proc_macro_stubs: FxHashSet<LocalDefId>,
/// Traces collected during macro resolution and validated when it's complete.
single_segment_macro_resolutions:
Vec<(Ident, MacroKind, ParentScope<'a>, Option<NameBinding<'a>>)>,
Vec<(Ident, MacroKind, bool, ParentScope<'a>, Option<NameBinding<'a>>)>,
multi_segment_macro_resolutions:
Vec<(Vec<Segment>, Span, MacroKind, ParentScope<'a>, Option<Res>)>,
builtin_attrs: Vec<(Ident, ParentScope<'a>)>,
Expand Down
46 changes: 40 additions & 6 deletions compiler/rustc_resolve/src/macros.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ use rustc_ast_pretty::pprust;
use rustc_attr::StabilityLevel;
use rustc_data_structures::intern::Interned;
use rustc_data_structures::sync::Lrc;
use rustc_errors::{struct_span_err, Applicability};
use rustc_errors::{struct_span_err, Applicability, FatalError};
use rustc_expand::base::{Annotatable, DeriveResolutions, Indeterminate, ResolverExpand};
use rustc_expand::base::{SyntaxExtension, SyntaxExtensionKind};
use rustc_expand::compile_declarative_macro;
Expand Down Expand Up @@ -373,6 +373,7 @@ impl<'a, 'tcx> ResolverExpand for Resolver<'a, 'tcx> {
match self.resolve_macro_path(
path,
Some(MacroKind::Derive),
false,
&parent_scope,
true,
force,
Expand Down Expand Up @@ -504,8 +505,14 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
force: bool,
soft_custom_inner_attributes_gate: bool,
) -> Result<(Lrc<SyntaxExtension>, Res), Indeterminate> {
let (ext, res) = match self.resolve_macro_path(path, Some(kind), parent_scope, true, force)
{
let (ext, res) = match self.resolve_macro_path(
path,
Some(kind),
inner_attr,
parent_scope,
true,
force,
) {
Ok((Some(ext), res)) => (ext, res),
Ok((None, res)) => (self.dummy_ext(kind), res),
Err(Determinacy::Determined) => (self.dummy_ext(kind), Res::Err),
Expand Down Expand Up @@ -627,6 +634,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
&mut self,
path: &ast::Path,
kind: Option<MacroKind>,
inner_attr: bool,
parent_scope: &ParentScope<'a>,
trace: bool,
force: bool,
Expand Down Expand Up @@ -685,6 +693,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
self.single_segment_macro_resolutions.push((
path[0].ident,
kind,
inner_attr,
*parent_scope,
binding.ok(),
));
Expand Down Expand Up @@ -794,7 +803,9 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
}

let macro_resolutions = mem::take(&mut self.single_segment_macro_resolutions);
for (ident, kind, parent_scope, initial_binding) in macro_resolutions {
let mut has_reported_inner_attr_error = false;
let mut raise_fatal_error = false;
for (ident, kind, inner_attr, parent_scope, initial_binding) in macro_resolutions {
match self.early_resolve_ident_in_lexical_scope(
ident,
ScopeSet::Macro(kind),
Expand All @@ -810,7 +821,17 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
});
let res = binding.res();
let seg = Segment::from_ident(ident);
check_consistency(self, &[seg], ident.span, kind, initial_res, res);
if has_reported_inner_attr_error
&& let Res::Def(DefKind::Macro(MacroKind::Attr), _) = res
&& let None = initial_res
{
// Do not emit an indeterminate resolution and later errors when an outer
// attribute wasn't found, as this can be knock down effects. #118455
raise_fatal_error = true;
} else {
let res = binding.res();
check_consistency(self, &[seg], ident.span, kind, initial_res, res);
};
if res == Res::NonMacroAttr(NonMacroAttrKind::DeriveHelperCompat) {
let node_id = self
.invocation_parents
Expand All @@ -825,7 +846,10 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
);
}
}
Err(..) => {
Err(_) => {
if inner_attr {
has_reported_inner_attr_error = true;
}
let expected = kind.descr_expected();

let mut err = self.tcx.sess.create_err(CannotFindIdentInThisScope {
Expand All @@ -851,6 +875,16 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
None,
);
}

if raise_fatal_error {
// When we encounter an inner attribute failure, and subsequent successful macro
// resolutions following early resolution failures. This is so when an outer attribute
// isn't found, and we encounter `derive` attributes, we won't raise errors caused by
// any code that relies on those derives having been evaluated. We don't attempt to
// recover because the behavior of those derives could have been modified by the outer
// attribute, causing *other* errors, so it is safest to just stop early instead.
FatalError.raise();
}
}

fn check_stability_and_deprecation(
Expand Down
6 changes: 4 additions & 2 deletions tests/ui/proc-macro/derive-helper-legacy-spurious.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,10 @@
#[macro_use]
extern crate test_macros;

#[derive(Empty)] //~ ERROR cannot determine resolution for the attribute macro `derive`
#[derive(Empty, Clone)] // no error emitted here
#[empty_helper] //~ ERROR cannot find attribute `empty_helper` in this scope
struct Foo {}

fn main() {}
fn main() {
let _ = Foo.clone(); // no error emitted here
}
10 changes: 1 addition & 9 deletions tests/ui/proc-macro/derive-helper-legacy-spurious.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -4,19 +4,11 @@ error: cannot find attribute `dummy` in this scope
LL | #![dummy]
| ^^^^^

error: cannot determine resolution for the attribute macro `derive`
--> $DIR/derive-helper-legacy-spurious.rs:8:3
|
LL | #[derive(Empty)]
| ^^^^^^
|
= note: import resolution is stuck, try simplifying macro imports

error: cannot find attribute `empty_helper` in this scope
--> $DIR/derive-helper-legacy-spurious.rs:9:3
|
LL | #[empty_helper]
| ^^^^^^^^^^^^

error: aborting due to 3 previous errors
error: aborting due to 2 previous errors

0 comments on commit 9e404df

Please sign in to comment.