Fix imports of built-in macros (mostly)#61877

petrochenkov · 2019-06-15T21:52:32Z

Built-in macros now have DefIds from LOCAL_CRATE and their DefIndexes occupy a continuous fixed-size range immediately following GlobalMetaData entries.

This way they can be used in imports without causing ICEs due to CrateNum::BuiltinMacros.
I tried a few different approaches mentioned in #61687, but this one seems to be the least ugly and painful.
EDIT: One more alternative is to follow the example of Res::PrimTy(hir::PrimTy) and introduce Res::BuiltinMacro(usize) so that built-in macros don't need a DefId at all. I didn't try it yet, but perhaps I should.

Caveats:

Same built-in macro imported through different crates has different DefIds.
This can be observable in corner cases (e.g. glob vs glob conflicts, see the test import-builtin-macros-bug.rs ).
I tried to convert (THAT_CRATE, DefIndex) into (LOCAL_CRATE, DefIndex) for built-in macros immediately during decoding, but apparently there are assumptions that entries from other crates cannot have DefId from the current crate after decoding, so it caused ICEs.
Individual imports of built-in macros are not stability checked (e.g. use concat_idents; does not produce a feature error) so the whole feature is gated instead.
The primary motivation for the fix is reexporting built-in macros from the standard library and we can use unstable features in the standard library.
The proper solution to this (and multiple other) problems is moving stability/deprecation checking into resolve.

Closes #61687
cc #61875
r? @eddyb

Centril · 2019-06-15T22:39:20Z

src/libsyntax/feature_gate.rs

    // #[repr(transparent)] on unions.
    (active, transparent_unions, "1.37.0", Some(60405), None),

+    // `use builtin_macro`.


Suggested change

// `use builtin_macro`.

// Allows `use builtin_macro;`.

Centril · 2019-06-15T22:48:08Z

src/test/ui/rust-2018/uniform-paths/import-builtin-macros-stability.rs

@@ -0,0 +1,10 @@
+// FIXME: Individual imports of built-in macros are not stability checked right now,
+// so the whole feature is gated instead.


I don't understand what this comment means. Can you illustrate the difference?

use concat_idents as unstable; below should produce a "concat_idents is unstable" error, but it doesn't.

Oh I see; I read .stderr sloppily... thanks!

Ahaha, concat_idents and friends are actually stable themselves, they just report feature errors as a part of their expansion.
I'll fix this, but #61898 is a pre-requisite.

petrochenkov · 2019-06-16T10:12:36Z

I'm going to try the Res::BuiltinMacro approach, marking as waiting-on-author.

eddyb · 2019-06-16T11:39:36Z

Something I really wanted years ago was to move the "definitions" of builtin types into libcore.
That is, to have #[intrinsic = "u8"] type u8; or #[intrinsic = "u8"] enum u8 {} or w/e somewhere in libcore, and have that be the canonical definition, to get rid of some of the hardcoding.
(I guess nowadays you could also abuse extern "intrinsic" { type u8; })

So I'm wondering, can we put #[intrinsic = "concat_idents"] macro concat_idents; in libcore?
Not a reexport of some magical item, but a definition with implementation-specified semantics.
Even if we haven't done it for builtin types yet, maybe we can do it for macros to start with?

petrochenkov · 2019-06-16T12:00:34Z

So I'm wondering, can we put #[intrinsic = "concat_idents"] macro concat_idents; in libcore?

That's an interesting idea.
Attaching all this data associated with built-in macros (like stability) to an actual node in source code would make many things simpler.
I'll try it.

petrochenkov · 2019-06-16T12:10:23Z

For built-in types it was tried in #32274.
(The downside is some potential inconveniences for libcore itself and other code using no_core or no_imlicit_prelude, but that depends on how exactly this is implemented.)

For built-in macros it may work generally better because they are not so fundamentally tied to the compiler, the compiler just provides expander functions for them, otherwise they are usual macros.

petrochenkov · 2019-06-16T19:20:54Z

Built-in macros introduced through libcore are blocked on #61898.

nikomatsakis · 2019-06-18T12:35:23Z

@eddyb

Nit: I feel like these should be lang items -- I'm not sure why we have so many categories of "special thing known to the compiler" but I think we should centralize on one of them.

eddyb · 2019-06-18T16:11:10Z

@nikomatsakis IMO we should distinguish between "this is exposing a compiler primitive to the library" and "this is exposing a library entity to the compiler", only the latter of which needs to be unique.

syntax: Factor out common fields from `SyntaxExtension` variants And some other related cleanups. Continuation of rust-lang#61606. This will also help to unblock rust-lang#61877.

bors · 2019-06-19T09:58:51Z

☔ The latest upstream changes (presumably #61945) made this pull request unmergeable. Please resolve the merge conflicts.

matklad · 2019-06-19T12:23:36Z

Something I really wanted years ago was to move the "definitions" of builtin types into libcore.

Yess, I have been thinking exactly about this when adding primitives names resolution to rust-analyzer. It also helps with goto definition UX in IDEs, because you have the actual source to go to.

It probably shouldn't be literally libcore though, because no_core is a thing. Rather we need a separate builtins crate which you can't opt out of.

mark-i-m · 2019-06-19T13:52:55Z

It probably shouldn't be literally libcore though, because no_core is a thing. Rather we need a separate builtins crate which you can't opt out of.

libreallycore

eddyb · 2019-06-19T14:41:01Z

It probably shouldn't be literally libcore though, because no_core is a thing. Rather we need a separate builtins crate which you can't opt out of.

Nope, #![no_core] exists literally for libcore alone. It's really hard to have any other setup, because a bunch of lang items would have to be in it for anything to work, not just the builtin types/macros.

EDIT: worst case, this will change and we can reexport those items in libcore backwards-compatibly.

petrochenkov · 2019-06-22T00:47:19Z

Now waiting on #62042

petrochenkov · 2019-06-23T20:10:37Z

#62086 confirmed that the idea with defining built-in macros through libcore is viable, so I'll close this PR.

petrochenkov added 3 commits June 15, 2019 20:34

syntax: Remove DummyResolver

0506568

resolve: Fix imports of built-in macros

7f98cf7

CrateNum::BuiltinMacros -> CrateNum::LegacyProcMacros

8fd02fd

rust-highfive assigned eddyb Jun 15, 2019

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 15, 2019

This was referenced Jun 15, 2019

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

Closed

Tracking issue for feature(builtin_macro_imports) #61875

Closed

Centril reviewed Jun 15, 2019

View reviewed changes

petrochenkov added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 16, 2019

petrochenkov added S-blocked Status: Blocked on something else such as an RFC or other implementation work. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jun 16, 2019

petrochenkov mentioned this pull request Jun 16, 2019

syntax: Factor out common fields from SyntaxExtension variants #61898

Merged

Centril mentioned this pull request Jun 17, 2019

rustdoc: resolve intra-doc links to builtin macros #61916

Closed

petrochenkov mentioned this pull request Jun 22, 2019

Support stability and deprecation checking for all macros #62042

Merged

petrochenkov mentioned this pull request Jun 22, 2019

Built-in derive macros Send and Sync exist #62050

Closed

petrochenkov mentioned this pull request Jun 23, 2019

Define built-in macros through libcore #62086

Merged

petrochenkov closed this Jun 23, 2019

petrochenkov deleted the usbumac3 branch July 1, 2019 22:56

		@@ -0,0 +1,10 @@
		// FIXME: Individual imports of built-in macros are not stability checked right now,
		// so the whole feature is gated instead.

Uh oh!

Conversation

petrochenkov commented Jun 15, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Centril Jun 15, 2019

Choose a reason for hiding this comment

Uh oh!

Centril Jun 15, 2019

Choose a reason for hiding this comment

Uh oh!

petrochenkov Jun 16, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Centril Jun 16, 2019

Choose a reason for hiding this comment

Uh oh!

petrochenkov Jun 18, 2019

Choose a reason for hiding this comment

Uh oh!

petrochenkov commented Jun 16, 2019

Uh oh!

eddyb commented Jun 16, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

petrochenkov commented Jun 16, 2019

Uh oh!

petrochenkov commented Jun 16, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

petrochenkov commented Jun 16, 2019

Uh oh!

nikomatsakis commented Jun 18, 2019

Uh oh!

eddyb commented Jun 18, 2019

Uh oh!

bors commented Jun 19, 2019

Uh oh!

matklad commented Jun 19, 2019

Uh oh!

mark-i-m commented Jun 19, 2019

Uh oh!

eddyb commented Jun 19, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

petrochenkov commented Jun 22, 2019

Uh oh!

petrochenkov commented Jun 23, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

petrochenkov commented Jun 15, 2019 •

edited

Loading

petrochenkov Jun 16, 2019 •

edited

Loading

eddyb commented Jun 16, 2019 •

edited

Loading

petrochenkov commented Jun 16, 2019 •

edited

Loading

eddyb commented Jun 19, 2019 •

edited

Loading