Skip to content

Commit

Permalink
Give a better error message for duplicate built-in macros
Browse files Browse the repository at this point in the history
Previously, this would say no such macro existed, but this was
misleading, since the macro _did_ exist, it was just already seen.

- Say where the macro was previously defined
- Add long-form error message
  • Loading branch information
jyn514 committed Sep 1, 2020
1 parent 022e1fe commit be2947d
Show file tree
Hide file tree
Showing 8 changed files with 112 additions and 8 deletions.
1 change: 1 addition & 0 deletions compiler/rustc_error_codes/src/error_codes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -454,6 +454,7 @@ E0768: include_str!("./error_codes/E0768.md"),
E0769: include_str!("./error_codes/E0769.md"),
E0770: include_str!("./error_codes/E0770.md"),
E0771: include_str!("./error_codes/E0771.md"),
E0773: include_str!("./error_codes/E0773.md"),
;
// E0006, // merged with E0005
// E0008, // cannot bind by-move into a pattern guard
Expand Down
38 changes: 38 additions & 0 deletions compiler/rustc_error_codes/src/error_codes/E0773.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
A builtin-macro was defined more than once.

Erroneous code example:

```compile_fail,E0773
#![feature(decl_macro)]
#![feature(rustc_attrs)]
#[rustc_builtin_macro]
pub macro test($item:item) {
/* compiler built-in */
}
mod inner {
#[rustc_builtin_macro]
pub macro test($item:item) {
/* compiler built-in */
}
}
```

To fix the issue, remove the duplicate declaration:

```
#![feature(decl_macro)]
#![feature(rustc_attrs)]
#[rustc_builtin_macro]
pub macro test($item:item) {
/* compiler built-in */
}
```

In very rare edge cases, this may happen when loading `core` or `std` twice,
once with `check` metadata and once with `build` metadata.
For more information, see [#75176].

[#75176]: https://github.com/rust-lang/rust/pull/75176#issuecomment-683234468
8 changes: 7 additions & 1 deletion compiler/rustc_resolve/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -867,6 +867,12 @@ pub struct ExternPreludeEntry<'a> {
pub introduced_by_item: bool,
}

/// Used for better errors for E0773
enum BuiltinMacroState {
NotYetSeen(SyntaxExtension),
AlreadySeen(Span),
}

/// The main resolver class.
///
/// This is the visitor that walks the whole crate.
Expand Down Expand Up @@ -960,7 +966,7 @@ pub struct Resolver<'a> {

crate_loader: CrateLoader<'a>,
macro_names: FxHashSet<Ident>,
builtin_macros: FxHashMap<Symbol, SyntaxExtension>,
builtin_macros: FxHashMap<Symbol, BuiltinMacroState>,
registered_attrs: FxHashSet<Ident>,
registered_tools: FxHashSet<Ident>,
macro_use_prelude: FxHashMap<Symbol, &'a NameBinding<'a>>,
Expand Down
22 changes: 18 additions & 4 deletions compiler/rustc_resolve/src/macros.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,15 @@

use crate::imports::ImportResolver;
use crate::Namespace::*;
use crate::{AmbiguityError, AmbiguityErrorMisc, AmbiguityKind, Determinacy};
use crate::{AmbiguityError, AmbiguityErrorMisc, AmbiguityKind, BuiltinMacroState, Determinacy};
use crate::{CrateLint, ParentScope, ResolutionError, Resolver, Scope, ScopeSet, Weak};
use crate::{ModuleKind, ModuleOrUniformRoot, NameBinding, PathResult, Segment, ToNameBinding};
use rustc_ast::{self as ast, NodeId};
use rustc_ast_lowering::ResolverAstLowering;
use rustc_ast_pretty::pprust;
use rustc_attr::StabilityLevel;
use rustc_data_structures::fx::FxHashSet;
use rustc_errors::struct_span_err;
use rustc_expand::base::{Indeterminate, InvocationRes, ResolverExpand, SyntaxExtension};
use rustc_expand::compile_declarative_macro;
use rustc_expand::expand::{AstFragment, AstFragmentKind, Invocation, InvocationKind};
Expand Down Expand Up @@ -166,7 +167,7 @@ impl<'a> ResolverExpand for Resolver<'a> {
}

fn register_builtin_macro(&mut self, ident: Ident, ext: SyntaxExtension) {
if self.builtin_macros.insert(ident.name, ext).is_some() {
if self.builtin_macros.insert(ident.name, BuiltinMacroState::NotYetSeen(ext)).is_some() {
self.session
.span_err(ident.span, &format!("built-in macro `{}` was already defined", ident));
}
Expand Down Expand Up @@ -1076,10 +1077,23 @@ impl<'a> Resolver<'a> {

if result.is_builtin {
// The macro was marked with `#[rustc_builtin_macro]`.
if let Some(ext) = self.builtin_macros.remove(&item.ident.name) {
if let Some(builtin_macro) = self.builtin_macros.get_mut(&item.ident.name) {
// The macro is a built-in, replace its expander function
// while still taking everything else from the source code.
result.kind = ext.kind;
// If we already loaded this builtin macro, give a better error message than 'no such builtin macro'.
match mem::replace(builtin_macro, BuiltinMacroState::AlreadySeen(item.span)) {
BuiltinMacroState::NotYetSeen(ext) => result.kind = ext.kind,
BuiltinMacroState::AlreadySeen(span) => {
struct_span_err!(
self.session,
item.span,
E0773,
"attempted to define built-in macro more than once"
)
.span_note(span, "previously defined here")
.emit();
}
}
} else {
let msg = format!("cannot find a built-in macro with name `{}`", item.ident);
self.session.span_err(item.span, &msg);
Expand Down
17 changes: 17 additions & 0 deletions src/test/ui/macros/duplicate-builtin.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
// compile-flags:--crate-type lib
#![feature(decl_macro)]
#![feature(rustc_attrs)]

#[rustc_builtin_macro]
pub macro test($item:item) {
//~^ NOTE previously defined
/* compiler built-in */
}

mod inner {
#[rustc_builtin_macro]
pub macro test($item:item) {
//~^ ERROR attempted to define built-in macro more than once [E0773]
/* compiler built-in */
}
}
21 changes: 21 additions & 0 deletions src/test/ui/macros/duplicate-builtin.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
error[E0773]: attempted to define built-in macro more than once
--> $DIR/duplicate-builtin.rs:13:5
|
LL | / pub macro test($item:item) {
LL | |
LL | | /* compiler built-in */
LL | | }
| |_____^
|
note: previously defined here
--> $DIR/duplicate-builtin.rs:6:1
|
LL | / pub macro test($item:item) {
LL | |
LL | | /* compiler built-in */
LL | | }
| |_^

error: aborting due to previous error

For more information about this error, try `rustc --explain E0773`.
4 changes: 2 additions & 2 deletions src/test/ui/macros/unknown-builtin.rs
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
// error-pattern: cannot find a built-in macro with name `line`
// error-pattern: attempted to define built-in macro more than once

#![feature(rustc_attrs)]

#[rustc_builtin_macro]
macro_rules! unknown { () => () } //~ ERROR cannot find a built-in macro with name `unknown`

#[rustc_builtin_macro]
macro_rules! line { () => () }
macro_rules! line { () => () } //~ NOTE previously defined here

fn main() {
line!();
Expand Down
9 changes: 8 additions & 1 deletion src/test/ui/macros/unknown-builtin.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ error: cannot find a built-in macro with name `unknown`
LL | macro_rules! unknown { () => () }
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: cannot find a built-in macro with name `line`
error[E0773]: attempted to define built-in macro more than once
--> $SRC_DIR/core/src/macros/mod.rs:LL:COL
|
LL | / macro_rules! line {
Expand All @@ -13,6 +13,13 @@ LL | | /* compiler built-in */
LL | | };
LL | | }
| |_____^
|
note: previously defined here
--> $DIR/unknown-builtin.rs:9:1
|
LL | macro_rules! line { () => () }
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: aborting due to 2 previous errors

For more information about this error, try `rustc --explain E0773`.

0 comments on commit be2947d

Please sign in to comment.