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

Imports/reexports of built-in macros are not supported #61687

Closed
petrochenkov opened this issue Jun 9, 2019 · 4 comments · Fixed by #62086
Closed

Imports/reexports of built-in macros are not supported #61687

petrochenkov opened this issue Jun 9, 2019 · 4 comments · Fixed by #62086
Assignees
Labels
A-macros Area: All kinds of macros (custom derive, macro_rules!, proc macros, ..) A-resolve Area: Path resolution C-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-lang Relevant to the language team, which will review and decide on the PR/issue.

Comments

@petrochenkov
Copy link
Contributor

use column; // error: cannot import a built-in macro

fn main() {}

The reason behind this error is that if you import something with a DefId, that DefId should be supported by all stages of compilation (metadata emitting specifically), but built-in macros don't have such a DefId.

The crate ID in their DefId has special value CrateNum::BuiltinMacros that is supported only inside rustc_resolve and causes ICEs every time it leaves it.

@jonas-schievink jonas-schievink added A-macros Area: All kinds of macros (custom derive, macro_rules!, proc macros, ..) A-resolve Area: Path resolution C-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jun 9, 2019
@petrochenkov
Copy link
Contributor Author

Possible fix 1.

Use LOCAL_CRATE as a CrateNum in built-in macros' DefIds.

Built-in macros have fixed def-indices, so we should be able to recognize a built-in macro when loading metadata from other crate and thus know how to expand it.

Using LOCAL_CRATE will mean that each crate reexporting built-in macros will reexport it's own different version (with a separate DefId).
This is not correct and observable through import conflicts (imports of the same macro will conflict while they shouldn't).
This can perhaps be fixed by changing DefIds (resetting crate num to LOCAL_CRATE) on the fly during metadata loading (?). Not sure.

@petrochenkov
Copy link
Contributor Author

Possible fix 2.

Introduce a special CrateNum for things defined by the language rather than any crate and support it through the whole compilation (aka CrateNum::BuiltinMacros done right).

Built-in types could potentially use it as well if it becomes necessary for them to have DefIds.

@petrochenkov
Copy link
Contributor Author

The primary motivation for fixing this is reexporting built-in macros from the standard library.

This way those macros will have a reliable way to call them (e.g. ::std::column!()) from other macros (the __rust_unstable_column!() problem) even without having stable def-site hygiene in the language.

@Centril Centril added the T-lang Relevant to the language team, which will review and decide on the PR/issue. label Jun 9, 2019
@petrochenkov
Copy link
Contributor Author

petrochenkov commented Jun 15, 2019

Fixed in #62086

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 Area: All kinds of macros (custom derive, macro_rules!, proc macros, ..) A-resolve Area: Path resolution C-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-lang Relevant to the language team, which will review and decide on the PR/issue.
Projects
None yet
3 participants