Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Built-in macros, derives and attributes have inconsistent interactions with macros of the same name. #52269

Closed
alercah opened this issue Jul 11, 2018 · 10 comments
Assignees
Labels
A-macros-1.2 Area: Declarative macros 1.2

Comments

@alercah
Copy link
Contributor

alercah commented Jul 11, 2018

Macros currently occupy a single namespace, regardless of whether they are used for attributes, derives, or function-like macros. This was a design decision for procedural macros, on the basis that it is easier to split them into separate namespaces later, but it leads to some confusing interactions with built-in attributes (like cfg or derive), macros (like compile_error or env) or derives (like Eq or Clone). The behaviour should be tightened up so that it is intuitive and straightforward to explain.


Currently, the rules appear to be as follows:

  • Macros can be defined that have the same name as a built-in attribute or macro. When used, the builtin takes precedence and the macro is ignored. If the macro is used in a different context, it is found and works normally (e.g. a function-like macro named derive can be called, or an attribute named compile_error can be used). Raw identifiers can be used to get around the built-in.
  • Macros can be defined that have the same name as a built-in derive, except with proc_macro_derive. If a macro is defined with the same name as a built-in derive, it takes precedence. Defining a macro with proc_macro_derive that conflicts with the built-in gives an error at the definition.

If use_extern_macro is enabled, then you can freely rename imports. This lets you get around the proc_macro_derive rule:

#![feature(use_extern_macro)]

use failure::Fail as Clone;
use std::fmt::{Display, Formatter};

#[derive(Debug, Clone)]
struct Foo {
}

impl Display for Foo {
    fn fmt(&self, fmt : &mut Formatter) -> Result<(), std::fmt::Error> {
        write!(fmt, "{:?}", self)
    }
}

fn main() {
    let _ = Foo{}.cause();
}

Note that this behaviour applies only to built-in macros. Macros defined in libstd or libcore (for no_std) are imported in the prelude and can be overridden as one would expect for prelude names.


I do not think we can get away with treating all builtins as if they are normal names defined in the prelude that can be overridden. While it might be possible to treat some of them that way, the cfg attribute is an excellent example of one that cannot be safely modified, or else the following program would be problematic, because macro imports apply throughout the entire source file, not just after the import:

#[cfg(target_os = "linux")]
use some_crate::some_macro as cfg;

At the same time, I don't see why built-in derives should be special. The traits they implement are not, after all; even the ones like Copy which are magical. And it is possible, though inadvisable, to shadow them independently: trait Copy {} won't prevent #[derive(Copy)] from working, for instance.

This implies to me that we should have the following in the macro namespace:

  1. Built-ins which are truly and deeply magical, and cannot safely be overridden. Their names are treated like keywords in the macro namespace, in that the same non-raw identifier cannot be used at all.
  2. Intrinsics which are implemented by the compiler, but defined and named normally, like existing lang items and intrinsic functions.

This would imply the following behaviour changes:

  1. proc_macro_derive can declare derives with the same name as an intrinsic derive; these will simply shadow the intrinsic.
  2. Built-in function-like macros and attributes are always referred to when their names are used in the macro namespace, even in the wrong context, unless a raw identifier is used.
  3. Macros of any kind cannot be declared with the same name as a built-in macro or attribute unless a raw identifier is used.
  4. Going forward, we could possibly look at converting some built-in attributes and macros into intrinsics so that their names are freed up.

This leaves an unresolved question around renaming imports (e.g. use foo::bar as cfg; where foo::bar is names both a function and macro; is this an error? warning? or it just silently hides the macro as r#cfg?) but would clean up the bulk of the issues with the situation.

@petrochenkov petrochenkov self-assigned this Jul 11, 2018
@petrochenkov
Copy link
Contributor

petrochenkov commented Jul 11, 2018

I'm going to implement roughly the same scheme as described here, because it's currently used for non-macro namespaces (I described it previously in the tool attributes RFC - rust-lang/rfcs#2103 (comment)).

  • User-defined names are, well, user-defined and modularized.
  • Macros defined in the standard library (including perhaps procedural ones in the future, moved to the library from the compiler!) are introduced through standard library prelude, with lower priority than user-defined names during in-scope resolution (#[macro_use] extern crate ... and preludes are the same mechanism).
  • Builtin macros (including derives) and attributes are introduced through language prelude, with even lower priority, in the same way as primitive types like u8 or bool.

If something currently diverges from this scheme, e.g. builtin something shadows user-defined something, then it's a bug and it should be reported (preferably in a separate focused issue with reproduction).

The only tricky case here is attributes that need to be inspected before name resolution, like #[feature(...)] or, as you've mentioned, #[cfg(...)].
Perhaps a few attributes will need to be reserved and not resolved, or perhaps they'll be interpreted as tokens first, but then resolved normally, I think it'll become cleared during implementation.

@djrenren
Copy link
Contributor

Currently attributes exist in another namespace so if we implement overriding of attributes it'll be a breaking change.

Right now you can have both a macro_rules! test and a #[test] attribute. Allowing overriding (which seems like the right call to me) would break any code that has both because they'd be in the same namespace. We even do this in the compiler: https://github.com/rust-lang/rust/blob/master/src/libsyntax_pos/analyze_filemap.rs

@petrochenkov
Copy link
Contributor

petrochenkov commented Jul 24, 2018

My plan was to implement the principled scheme first, then run it through crater and look how much code breaks on stable, what cases of breakage are common, and then make adjustments.

@djrenren
Copy link
Contributor

djrenren commented Jul 24, 2018

Sounds like a good plan. I've run into this issue while working on custom test frameworks so mind if I take a crack at it?

@petrochenkov
Copy link
Contributor

@djrenren
Yes, it would be good if more people got familiar with this code.
However, I'm currently working on #52512 and #44690 which are closely related to this issue and are prerequisites in some sense, and hope to start working on this issue this weekend.

There's one piece of work that's relatively independent though.
There's a number of locations that accept attributes, but cannot be inputs to macros, for example generic parameters fn f<#[my_attr] T>() {} (the full list can be found in #52367 (comment) or in ast.rs).
We need to go through all these attributes, resolve them and report an error if resolution does not refer to an "inert" attribute (builtin, custom or tool attribute).

@djrenren
Copy link
Contributor

Sweet I'll take a look at it.

Mark-Simulacrum added a commit to Mark-Simulacrum/rust that referenced this issue Aug 1, 2018
resolve: Implement prelude search for macro paths, implement tool attributes

When identifier is macro path is resolved in scopes (i.e. the first path segment - `foo` in `foo::mac!()` or `foo!()`), scopes are searched in the same order as for non-macro paths - items in modules, extern prelude, tool prelude (see later), standard library prelude, language prelude, but with some extra shadowing restrictions (names from globs and macro expansions cannot shadow names from outer scopes). See the comment in `fn resolve_lexical_macro_path_segment` for more details.

"Tool prelude" currently contains two "tool modules" `rustfmt` and `clippy`, and is searched immediately after extern prelude.
This makes the [possible long-term solution](https://github.com/rust-lang/rfcs/blob/master/text/2103-tool-attributes.md#long-term-solution) for tool attributes exactly equivalent to the existing extern prelude scheme, except that `--extern=my_crate` making crate names available in scope is replaced with something like `--tool=my_tool` making tool names available in scope.

The `tool_attributes` feature is still unstable and `#![feature(tool_attributes)]` now implicitly enables `#![feature(use_extern_macros)]`. `use_extern_macros` is a prerequisite for `tool_attributes`, so their stabilization will happen in the same order.
If `use_extern_macros` is not enabled, then tool attributes are treated as custom attributes (this is temporary, anyway).

Fixes rust-lang#52576
Fixes rust-lang#52512
Fixes rust-lang#51277
cc rust-lang#52269
bors added a commit that referenced this issue Aug 2, 2018
resolve: Implement prelude search for macro paths, implement tool attributes

When identifier is macro path is resolved in scopes (i.e. the first path segment - `foo` in `foo::mac!()` or `foo!()`), scopes are searched in the same order as for non-macro paths - items in modules, extern prelude, tool prelude (see later), standard library prelude, language prelude, but with some extra shadowing restrictions (names from globs and macro expansions cannot shadow names from outer scopes). See the comment in `fn resolve_lexical_macro_path_segment` for more details.

"Tool prelude" currently contains two "tool modules" `rustfmt` and `clippy`, and is searched immediately after extern prelude.
This makes the [possible long-term solution](https://github.com/rust-lang/rfcs/blob/master/text/2103-tool-attributes.md#long-term-solution) for tool attributes exactly equivalent to the existing extern prelude scheme, except that `--extern=my_crate` making crate names available in scope is replaced with something like `--tool=my_tool` making tool names available in scope.

The `tool_attributes` feature is still unstable and `#![feature(tool_attributes)]` now implicitly enables `#![feature(use_extern_macros)]`. `use_extern_macros` is a prerequisite for `tool_attributes`, so their stabilization will happen in the same order.
If `use_extern_macros` is not enabled, then tool attributes are treated as custom attributes (this is temporary, anyway).

Fixes #52576
Fixes #52512
Fixes #51277
cc #52269
@aturon aturon modified the milestones: Rust 2018 RC, Edition RC 2 Sep 5, 2018
@aturon
Copy link
Member

aturon commented Sep 5, 2018

Bumping to RC2 milestone.

@alexcrichton
Copy link
Member

@petrochenkov to check on on this issue, I think #53913 is evaluating the impact of one strategy for solving this, right? Otherwise I believe the built-in attributes cannot currently be shadowed, even if a macro of such a name is imported? (in that it looks like nightly accepts this:)

#![feature(decl_macro)]

macro cfg() {
    
}

#[cfg(all())]
fn main() {}

@petrochenkov
Copy link
Contributor

@alexcrichton
Yes, that's right.

bors added a commit that referenced this issue Sep 11, 2018
resolve: Future proof resolutions for potentially built-in attributes

This is not full "pass all attributes through name resolution", but a more conservative solution.
If built-in attribute is ambiguous with any other macro in scope, then an error is reported.

What complications arise with the full solution - #53913 (comment).

cc #50911 (comment)
cc #52269
Closes #53531
@petrochenkov
Copy link
Contributor

petrochenkov commented Sep 19, 2018

Status: future proofed in #53913.

There's still a single macro namespace, but it has two sub-namespaces now to avoid backward-incompatible conflicts. #54069 has more detailed description of how the sub-namespace scheme works.

When used, the builtin takes precedence and the macro is ignored.

After #53913 if potentially built-in attribute is ambiguous with any other macro name in scope (taking sub-namespacing into account), then error is reported.
Some very special names (cfg, cfg_attr and derive) are completely reserved from being defined in macro namespace (including use foo::bar as cfg;).

It's possible to relax this restriction to achieve the end goal - user-defined attribute macros shadowing built-in attributes (or rather newly introduced built-in attributes not breaking user-defined macros), but it's not entirely trivial, see #53913 (comment) for more detailed explanation.

Defining a macro with proc_macro_derive that conflicts with the built-in gives an error at the definition.

This artificial error is still in place, but it can be easily removed now since it doesn't serve its purpose of preventing user-defined derives from shadowing built-in derives.

Future plans:

  • Possibly relax the ambiguity errors and reservations for potentially built-in attributes.
  • Remove the "cannot override a built-in #[derive] mode" error in proc_macro_registrar.rs.

I think we can close this generic issue now and perhaps create new more focused issues for "future plans" (if those even need tracking).

Centril added a commit to Centril/rust that referenced this issue Jul 26, 2019
Define built-in macros through libcore

This PR defines built-in macros through libcore using a scheme similar to lang items (attribute `#[rustc_builtin_macro]`).
All the macro properties (stability, visibility, etc.) are taken from the source code in libcore, with exception of the expander function transforming input tokens/AST into output tokens/AST, which is still provided by the compiler.

The macros are made available to user code through the standard library prelude (`{core,std}::prelude::v1`), so they are still always in scope.
As a result **built-in macros now have stable absolute addresses in the library**, like `core::prelude::v1::line!()`, this is an insta-stable change.

Right now `prelude::v1` is the only publicly available absolute address for these macros, but eventually they can be moved into more appropriate locations with library team approval (e.g. `Clone` derive -> `core::clone::Clone`).

Now when built-in macros have canonical definitions they can be imported or reexported without issues (rust-lang#61687).

Other changes:
- You can now define a derive macro with a name matching one of the built-in derives (rust-lang#52269). This was an artificial restriction that could be worked around with import renaming anyway.

Known regressions:
- Empty library crate with a crate-level `#![test]` attribute no longer compiles without `--test`. Previously it didn't compile *with* `--test` or with the bin crate type.

Fixes rust-lang#61687
Fixes rust-lang#61804
r? @eddyb
bors added a commit that referenced this issue Jul 26, 2019
Define built-in macros through libcore

This PR defines built-in macros through libcore using a scheme similar to lang items (attribute `#[rustc_builtin_macro]`).
All the macro properties (stability, visibility, etc.) are taken from the source code in libcore, with exception of the expander function transforming input tokens/AST into output tokens/AST, which is still provided by the compiler.

The macros are made available to user code through the standard library prelude (`{core,std}::prelude::v1`), so they are still always in scope.
As a result **built-in macros now have stable absolute addresses in the library**, like `core::prelude::v1::line!()`, this is an insta-stable change.

Right now `prelude::v1` is the only publicly available absolute address for these macros, but eventually they can be moved into more appropriate locations with library team approval (e.g. `Clone` derive -> `core::clone::Clone`).

Now when built-in macros have canonical definitions they can be imported or reexported without issues (#61687).

Other changes:
- You can now define a derive macro with a name matching one of the built-in derives (#52269). This was an artificial restriction that could be worked around with import renaming anyway.

Known regressions:
- Empty library crate with a crate-level `#![test]` attribute no longer compiles without `--test`. Previously it didn't compile *with* `--test` or with the bin crate type.

Fixes #61687
Fixes #61804
r? @eddyb
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-macros-1.2 Area: Declarative macros 1.2
Projects
None yet
Development

No branches or pull requests

6 participants