Stream: t-compiler/help

Topic: Can I visit the AST before DefIds are generated?


Joshua Nelson (Jun 20 2020 at 15:32, on Zulip):

context: https://github.com/rust-lang/rust/pull/73532#issuecomment-646991250

I'm looking around librustc_expand/expand.rs but it looks like everything here is an AstFragment, not part of the AST proper.

Joshua Nelson (Jun 20 2020 at 15:33, on Zulip):

Whoops, I missed https://doc.rust-lang.org/nightly/nightly-rustc/rustc_expand/expand/enum.AstFragment.html#method.mut_visit_with

Joshua Nelson (Jun 20 2020 at 16:10, on Zulip):

@ecstatic-morse so here's the idea I had:

  1. Expand macros
  2. Before generating the DefId for any of the AST, run ReplaceBodyWithLoop
  3. Generate DefIds

This broke immediately because PlaceHolderExpander expects all ids to be DUMMY_NODE_ID. So I changed it to use DUMMY_NODE_ID and now lower_node_id_generic is panicking because it wants ids not to be dummies. Do you have any idea what's going on? I'm very confused.

// without dummy ids
$ rustc +stage1 -Z unpretty=everybody_loops priv-in-pub.rs
thread 'rustc' panicked at 'assertion failed: `(left == right)`
  left: `NodeId(28)`,
 right: `NodeId(4294967040)`', src/librustc_expand/placeholders.rs:326:17
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
// with dummy ids
$ rustc +stage1 -Z unpretty=everybody_loops priv-in-pub.rs
thread 'rustc' panicked at 'assertion failed: `(left != right)`
  left: `NodeId(4294967040)`,
 right: `NodeId(4294967040)`', src/librustc_ast_lowering/lib.rs:599:9
ecstatic-morse (Jun 20 2020 at 16:22, on Zulip):

No. I think you have the right idea; we need to do the AST transformation for rustdoc before assigning DefIds, but I don't work on the frontend at all

marmeladema (Jun 20 2020 at 16:37, on Zulip):

So iirc lower_node_id_generic is assigning new HirId to NodeId so it needs real node ids, not dummy ones

Joshua Nelson (Jun 20 2020 at 16:40, on Zulip):

Sure, but why does PlaceHolderExpander need dummies? Or I guess I'm not sure why it's being run at all on this code.

Joshua Nelson (Jun 20 2020 at 16:42, on Zulip):

let me push to a branch real quick

marmeladema (Jun 20 2020 at 16:43, on Zulip):

I've never actually had to look at librustc_expand so I probably won't be able to give good advises

Joshua Nelson (Jun 20 2020 at 16:47, on Zulip):

WIP: https://github.com/jyn514/rust/tree/replace-bodies/earlier-pass

marmeladema (Jun 20 2020 at 16:48, on Zulip):

I know that recently @Aaron Hill and @Vadim Petrochenkov worked on macros and defids

Joshua Nelson (Jun 20 2020 at 16:52, on Zulip):

This is the relevant code, but I'm not sure what it's doing: https://github.com/jyn514/rust/blob/4933cb2090986866a041a9c11a4233ad0ec13bb3/src/librustc_expand/expand.rs#L516

Joshua Nelson (Jun 20 2020 at 17:05, on Zulip):

maybe I could move this into build_reduced_graph instead, just before it calls collect_definitions?

Joshua Nelson (Jun 20 2020 at 17:21, on Zulip):

nope, same error that it expected a dummy span

Joshua Nelson (Jun 20 2020 at 17:29, on Zulip):

I'm getting annoyed enough I might make a PR switching from HirId + DUMMY_NODE_ID to Option<HirId>

marmeladema (Jun 20 2020 at 18:22, on Zulip):

I don't understand how that would help

Vadim Petrochenkov (Jun 20 2020 at 20:00, on Zulip):

I don't think you need to run everybody-loop before DefIds are generated, it's run at the right time.

Vadim Petrochenkov (Jun 20 2020 at 20:00, on Zulip):

It just needs to keep nodes having DefIds.

Vadim Petrochenkov (Jun 20 2020 at 20:02, on Zulip):

(That's what I can say without digging too much into the problem, and I don't really want to dig too much into this.)

Joshua Nelson (Jun 20 2020 at 23:03, on Zulip):

This ended up not running everybody_loops at all, see https://github.com/rust-lang/rust/pull/73566

Last update: Sep 28 2020 at 15:45UTC