fix: Fix matching of doc comments in MBEs - #23314
Draft
ChayimFriedman2 wants to merge 2 commits into
Draft
Conversation
ChayimFriedman2
marked this pull request as draft
September 7, 2026 21:57
ChayimFriedman2
force-pushed
the
skip-macro-doc-comments-v2
branch
from
September 7, 2026 21:59
aba3f22 to
1fac479
Compare
…he tt crate And not in the representation of the proc macro bridge. We still do not use the contiguous representation the rest of r-a uses, and prefer a tree with `Rc`s (this changed from `Arc` because we don't need thread safety for the bridge) for cheap cloning, but the types are the same. The reason for that is that conversion to the bridge's types is a lossy conversion, in rustc as well: the internal token tree representation contains details that are lost in this conversion. This is a prerequisite to fix (don't close GH) rust-lang#23088 - the only way to fix it properly is to have an additional kind of token tree for doc comments, like rustc does, as evidenced by the fact that if a proc macro would create this macro (that has a doc comment in its matcher), the doc comment would *not* be ignored for matching, but if the macro's matcher would have been passed to the proc macro and it wouldn't have touch it (meaning, not even listing the `TokenTree`s then giving them back), the comment would still be ignored. It is also required for supporting invisible groups properly for the same reason (there're actually several kinds of invisible delimiters, and the proc macro bridge lossily converts them all into one). While we're at, I've also tried to untangle the mess of Cargo features of the proc macro server, and reduce the code it contains: There are now *two* somewhat-orthogonal axes: the `in-rust-tree` feature, and the `in-ra` vs. `in-proc-macro-srv` feature. The former only decides whether we should import rustc crates from crates.io or from the sysroot. The latter decides what code to enable - the proc macro server doesn't need all the code in tt and proc-macro-api. I also moved the proc macro server's `TokenStream` from proc-macro-srv into proc-macro-api and inverted their dependency, as was required for this work. You cannot enable `in-proc-macro-srv` and disable `in-rust-tree` (in practice this just disables all proc macro server code since such combination can arise when compiling r-a, also proc-macro-api supports this mode for tests), and you are not supposed to enable both `in-ra` and `in-proc-macro-srv`, except for tests. Enabling `in-rust-tree` and disabling `in-proc-macro-srv` is possible, though, and done when compiling r-a in-tree. In the future, this mode may use sysroot crates not just for the proc macro server, like was done in the past.
They should be ignored, but only if they stay in the original representation as `tt::DocComment`. Which first means we need to *author* `tt::DocComment`, then handle the cases where it's *not* preserved: in MBE input, and when proc macros see it (but lazily; not when passed to proc macros, only when they inspect it - i.e. only when it's converted to the proc-macro-bridge's lossy representation. Invisible delimiters of MBE, one of the most frequent origins for bugs we have, also work in the same way (both originate from how rustc does not separate between its AST and macro input), so it's a precedent more important than just this edge case. I don't think we have a way to test the interactions with proc macros (this requires a real proc macro server with the bridge), but I tested all gory details manually with a built proc macro server. This requires a tiny adaptation from the RustRover folks (we should probably notify them): the serialization of `FlatTree` has changed slightly (even in the JSON format) for `version >= DOC_COMMENT_LEAF`. They can send (and receive) an empty `doc_comments` list if they do not send non-desugared doc comment token trees (which they do not, currently).
ChayimFriedman2
force-pushed
the
skip-macro-doc-comments-v2
branch
from
September 8, 2026 03:47
1fac479 to
f1ce6f2
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
They should be ignored, but only if they stay in the original representation as
tt::DocComment. Which first means we need to authortt::DocComment, then handle the cases where it's not preserved: in MBE input, and when proc macros see it (but lazily; not when passed to proc macros, only when they inspect it - i.e. only when it's converted to the proc-macro-bridge's lossy representation.Invisible delimiters of MBE, one of the most frequent origins for bugs we have, also work in the same way (both originate from how rustc does not separate between its AST and macro input), so it's a precedent more important than just this edge case.
I don't think we have a way to test the interactions with proc macros (this requires a real proc macro server with the bridge), but I tested all gory details manually with a built proc macro server.
This requires a tiny adaptation from the RustRover folks (we should probably notify them): the serialization of
FlatTreehas changed slightly (even in the JSON format) forversion >= DOC_COMMENT_LEAF. They can send (and receive) an emptydoc_commentslist if they do not send non-desugared doc comment token trees (which they do not, currently).Specifically, in those versions the schema went from:
Into:
Fixes #23088.
This sits on the top of #23152 which is required for this.