Improve proc-macro def ids

[jseyfried]

Support cstore.relative_def_path(id) and cstore.def_key(id) with proc-macro def ids.
Fixes #38207.
r? @nikomatsakis

[jseyfried]

I manually verified via logs that cstore.relative_def_path(id) and cstore.def_key(id) work correctly with proc-macro def ids, but I'm not sure how best to add a regression test -- having trouble reproducing the underlying bug.

[jseyfried]

cc @nrc

[nikomatsakis]

@jseyfried I can take a stab at making a regr test involving incremental compilation

[nikomatsakis]

@jseyfried This seems pretty clean though -- confined to metadata, as you promised. :)

[nikomatsakis]

Well, I failed to produce a regr test and am now on vacation for a week. =) I'm inclined to r+ -- though I suspect that creating an incremental test that uses a proc macro would suffice.

[nikomatsakis]

r? @michaelwoerister

[michaelwoerister]

Unfortunately I also failed in producing a small test case.

Why is it that these DefIds need special handling?

[nikomatsakis]

@michaelwoerister normally we create an Entry structure for each element of the module tree, and then we reconstruct a DefId by retracing the DefPath through these entries. However, for procedural macros, we are not creating the same structure in the metadata -- we're making a synthetic one. At least this is what I understand, @jseyfried can confirm.

That said, I find this mildly confusing too. That is, in my mind's eye, we could still encode the procedural macro crate's items, since the path to the procedural macro that the user should be using is basically the path to the item that is annotated with #[proc_macro].

That is, the way I think of it, the procedural macro is conceptually executed by interpreting the MIR attached to the procedural macro crate -- or, put another way, we attach some executable data to the #[proc_macro] fn.

I guess this view is inaccurate?

[michaelwoerister]

Sounds about right to me. I don't think that there can be dynamically created procedural macros (although the macro/plugin registrar function infrastructure would probably allow that).

[michaelwoerister]

@bors r+

I think we should refactor how DefPaths are handled in metadata but I don't want to block this PR on that.

[bors]

📌 Commit 5200a11 has been approved by michaelwoerister

[bors]

⌛ Testing commit 5200a11 with merge a274617...

[bors]

☀️ Test successful - auto-linux-32-nopt-t, auto-linux-32-opt, auto-linux-64-cargotest, auto-linux-64-cross-freebsd, auto-linux-64-debug-opt, auto-linux-64-nopt-t, auto-linux-64-opt, auto-linux-64-opt-rustbuild, auto-linux-64-x-android-t, auto-linux-cross-opt, auto-linux-musl-64-opt, auto-mac-32-opt, auto-mac-64-opt, auto-mac-64-opt-rustbuild, auto-mac-cross-ios-opt, auto-win-gnu-32-opt, auto-win-gnu-32-opt-rustbuild, auto-win-gnu-64-opt, auto-win-msvc-32-opt, auto-win-msvc-64-cargotest, auto-win-msvc-64-opt, auto-win-msvc-64-opt-rustbuild
Approved by: michaelwoerister
Pushing a274617 to master...

[jseyfried]

@nikomatsakis
Yeah, I think that view is conceptually accurate. Ideally, I think a proc macro def id should be either the def id of the underlying #[proc_macro] function or should correspond to a specially encoded Entry with with a new kind EntryKind::ProcMacro.

However, refactoring the current registrar-based system implemented in #35957 to either of these options will probably be quite a bit of work (it might be a good idea to combine this with a transition from dynamic linking to IPC -- cc @eddyb).

[Arnavion]

@jseyfried The repro in #38237 hits the assert you added to maybe_entry. Can you take a look?