Assign def ids and build the module graph during expansion #36601
Conversation
There was a problem hiding this comment.
macro_escape is ancient! (5bf385b)
Can be removed now.
There was a problem hiding this comment.
Ugh, I haven't noticed it's span_warn and not span_err, even the variable is named err...
|
r = me, once the deps land. |
|
☔ The latest upstream changes (presumably #36551) made this pull request unmergeable. Please resolve the merge conflicts. |
jseyfried
force-pushed
the
build_reduced_graph_in_expansion
branch
7 times, most recently
from
08f131e to
105cc1e
Compare
There was a problem hiding this comment.
After this PR, we load extern crates (in BuildReducedGraphVisitor) before checking for non-ascii idents (in the gated feature checking pass).
|
@nrc I added three more commits -- r? |
jseyfried
force-pushed
the
build_reduced_graph_in_expansion
branch
5 times, most recently
from
1dff614 to
0594531
Compare
There was a problem hiding this comment.
I was going to say it would be better to have a struct than a tuple at this point (esp with a bool with comment). However, given that this field only ever seems to have one instantiation, it might just be over-abstracted. Could it be changed to a flag with a method rather than an optional closure?
There was a problem hiding this comment.
I could refactor the closure into a user-defined trait object (e.g. Option<&'a mut MacroInvocationVisitor>, where MacroInvocationVisitor is a trait with method fn visit_invoc(id: NodeId, def_index: DefIndex, const_integer: bool)).
The invocation visiting code needs to be in resolve, so we need a trait object of some sort.
There was a problem hiding this comment.
I think a closure is as good as a trait object, but probably better to take a struct rather than a 3-tuple.
There was a problem hiding this comment.
Good point -- amended accordingly.
src/librustc_resolve/macros.rs
Outdated
There was a problem hiding this comment.
Could you add a comment explaining what const_integer means please?
jseyfried
force-pushed
the
build_reduced_graph_in_expansion
branch
2 times, most recently
from
a21377c to
3aa46c3
Compare
|
☔ The latest upstream changes (presumably #36678) made this pull request unmergeable. Please resolve the merge conflicts. |
and expand the `__test_reexports` in the correct scope.
jseyfried
force-pushed
the
build_reduced_graph_in_expansion
branch
from
3aa46c3 to
dfa69be
Compare
|
@bors: r+ |
|
📌 Commit dfa69be has been approved by |
Assign def ids and build the module graph during expansion r? @nrc
4 participants
Groundwork for macro modularization (cc #35896).
r? @nrc