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

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 rust-lang#118455.
  • Loading branch information
estebank committed Dec 4, 2023
1 parent 0e2dac8 commit c0ebfe9
Show file tree
Hide file tree
Showing 7 changed files with 72 additions and 34 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),
ast::AttrStyle::Outer,
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),
ast::AttrStyle::Outer,
parent_scope,
true,
force,
Expand Down
11 changes: 8 additions & 3 deletions compiler/rustc_resolve/src/late.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3868,9 +3868,14 @@ impl<'a: 'ast, 'b, 'ast, 'tcx> LateResolutionVisitor<'a, 'b, 'ast, 'tcx> {
if qself.is_none() {
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)
{
if let Ok((_, res)) = self.r.resolve_macro_path(
&path,
None,
AttrStyle::Outer,
&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 @@ -1046,7 +1046,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, ast::AttrStyle, 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
75 changes: 56 additions & 19 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 @@ -268,15 +268,19 @@ impl<'a, 'tcx> ResolverExpand for Resolver<'a, 'tcx> {
}
};

let (path, kind, inner_attr, derives) = match invoc.kind {
let (path, kind, attr_kind, derives) = match invoc.kind {
InvocationKind::Attr { ref attr, ref derives, .. } => (
&attr.get_normal_item().path,
MacroKind::Attr,
attr.style == ast::AttrStyle::Inner,
attr.style,
self.arenas.alloc_ast_paths(derives),
),
InvocationKind::Bang { ref mac, .. } => (&mac.path, MacroKind::Bang, false, &[][..]),
InvocationKind::Derive { ref path, .. } => (path, MacroKind::Derive, false, &[][..]),
InvocationKind::Bang { ref mac, .. } => {
(&mac.path, MacroKind::Bang, ast::AttrStyle::Outer, &[][..])
}
InvocationKind::Derive { ref path, .. } => {
(path, MacroKind::Derive, ast::AttrStyle::Outer, &[][..])
}
};

// Derives are not included when `invocations` are collected, so we have to add them here.
Expand All @@ -287,7 +291,7 @@ impl<'a, 'tcx> ResolverExpand for Resolver<'a, 'tcx> {
path,
kind,
supports_macro_expansion,
inner_attr,
attr_kind,
parent_scope,
node_id,
force,
Expand Down Expand Up @@ -373,6 +377,7 @@ impl<'a, 'tcx> ResolverExpand for Resolver<'a, 'tcx> {
match self.resolve_macro_path(
path,
Some(MacroKind::Derive),
ast::AttrStyle::Outer,
&parent_scope,
true,
force,
Expand Down Expand Up @@ -498,19 +503,19 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
path: &ast::Path,
kind: MacroKind,
supports_macro_expansion: SupportsMacroExpansion,
inner_attr: bool,
attr_kind: ast::AttrStyle,
parent_scope: &ParentScope<'a>,
node_id: NodeId,
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)
{
Ok((Some(ext), res)) => (ext, res),
Ok((None, res)) => (self.dummy_ext(kind), res),
Err(Determinacy::Determined) => (self.dummy_ext(kind), Res::Err),
Err(Determinacy::Undetermined) => return Err(Indeterminate),
};
let (ext, res) =
match self.resolve_macro_path(path, Some(kind), attr_kind, 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),
Err(Determinacy::Undetermined) => return Err(Indeterminate),
};

// Report errors for the resolved macro.
for segment in &path.segments {
Expand Down Expand Up @@ -549,7 +554,9 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
match supports_macro_expansion {
SupportsMacroExpansion::No => Some(("a", "non-macro attribute")),
SupportsMacroExpansion::Yes { supports_inner_attrs } => {
if inner_attr && !supports_inner_attrs {
if let ast::AttrStyle::Inner = attr_kind
&& !supports_inner_attrs
{
Some(("a", "non-macro inner attribute"))
} else {
None
Expand Down Expand Up @@ -588,7 +595,10 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
}

// We are trying to avoid reporting this error if other related errors were reported.
if res != Res::Err && inner_attr && !self.tcx.features().custom_inner_attributes {
if res != Res::Err
&& let ast::AttrStyle::Inner = attr_kind
&& !self.tcx.features().custom_inner_attributes
{
let msg = match res {
Res::Def(..) => "inner macro attributes are unstable",
Res::NonMacroAttr(..) => "custom inner attributes are unstable",
Expand Down Expand Up @@ -627,6 +637,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
&mut self,
path: &ast::Path,
kind: Option<MacroKind>,
attr_kind: ast::AttrStyle,
parent_scope: &ParentScope<'a>,
trace: bool,
force: bool,
Expand Down Expand Up @@ -685,6 +696,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
self.single_segment_macro_resolutions.push((
path[0].ident,
kind,
attr_kind,
*parent_scope,
binding.ok(),
));
Expand Down Expand Up @@ -794,7 +806,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, attr_kind, parent_scope, initial_binding) in macro_resolutions {
match self.early_resolve_ident_in_lexical_scope(
ident,
ScopeSet::Macro(kind),
Expand All @@ -810,7 +824,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 +849,10 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
);
}
}
Err(..) => {
Err(_) => {
if let ast::AttrStyle::Inner = attr_kind {
has_reported_inner_attr_error = true;
}
let expected = kind.descr_expected();

let mut err = self.tcx.sess.create_err(CannotFindIdentInThisScope {
Expand All @@ -851,6 +878,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 c0ebfe9

Please sign in to comment.