Skip to content

Commit 1fac479

Browse files
ChayimFriedman2ChayimFriedman2
committed
Fix matching of doc comments in MBEs
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).
1 parent 8f66da0 commit 1fac479

40 files changed

Lines changed: 976 additions & 428 deletions

File tree

‎crates/cfg/src/tests.rs‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ fn assert_parse_result(input: &str, expected: CfgExpr) {
2323
pred_ast.syntax(),
2424
DummyTestSpanMap,
2525
DUMMY,
26-
DocCommentDesugarMode::ProcMacro,
26+
DocCommentDesugarMode::Keep,
2727
);
2828
let cfg = CfgExpr::parse(&tt);
2929
assert_eq!(cfg, expected);
@@ -39,7 +39,7 @@ fn check_dnf(input: &str, expect: Expect) {
3939
pred_ast.syntax(),
4040
DummyTestSpanMap,
4141
DUMMY,
42-
DocCommentDesugarMode::ProcMacro,
42+
DocCommentDesugarMode::Keep,
4343
);
4444
let cfg = CfgExpr::parse(&tt);
4545
let actual = format!("#![cfg({})]", DnfExpr::new(&cfg));
@@ -57,7 +57,7 @@ fn check_why_inactive(input: &str, opts: &CfgOptions, expect: Expect) {
5757
pred_ast.syntax(),
5858
DummyTestSpanMap,
5959
DUMMY,
60-
DocCommentDesugarMode::ProcMacro,
60+
DocCommentDesugarMode::Keep,
6161
);
6262
let cfg = CfgExpr::parse(&tt);
6363
let dnf = DnfExpr::new(&cfg);
@@ -77,7 +77,7 @@ fn check_enable_hints(input: &str, opts: &CfgOptions, expected_hints: &[&str]) {
7777
pred_ast.syntax(),
7878
DummyTestSpanMap,
7979
DUMMY,
80-
DocCommentDesugarMode::ProcMacro,
80+
DocCommentDesugarMode::Keep,
8181
);
8282
let cfg = CfgExpr::parse(&tt);
8383
let dnf = DnfExpr::new(&cfg);

‎crates/hir-def/src/item_tree/attrs.rs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,8 @@ impl AttrsOrCfg {
8686
.unwrap_or_else(|| meta.syntax().clone()),
8787
span_map,
8888
span,
89-
DocCommentDesugarMode::ProcMacro,
89+
// FIXME: This won't be correct once we support args for macro_rules attributes.
90+
DocCommentDesugarMode::Keep,
9091
);
9192
let input = Some(Box::new(AttrInput::TokenTree(tt)));
9293
(span, input)

‎crates/hir-def/src/macro_expansion_tests/mbe/matching.rs‎

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -257,3 +257,84 @@ macro_rules! m {
257257
"#]],
258258
);
259259
}
260+
261+
#[test]
262+
fn doc_comment_is_ignored() {
263+
check(
264+
r#"
265+
macro_rules! m {
266+
(
267+
/// hello
268+
) => {};
269+
}
270+
271+
m!();
272+
"#,
273+
expect![[r#"
274+
macro_rules! m {
275+
(
276+
/// hello
277+
) => {};
278+
}
279+
280+
281+
"#]],
282+
);
283+
check(
284+
r#"
285+
macro_rules! m {
286+
() => {
287+
macro_rules! m2 {
288+
(/** hello */) => {}
289+
}
290+
};
291+
}
292+
293+
m!();
294+
m2!();
295+
"#,
296+
expect![[r#"
297+
macro_rules! m {
298+
() => {
299+
macro_rules! m2 {
300+
(/** hello */) => {}
301+
}
302+
};
303+
}
304+
305+
macro_rules !m2 {
306+
(/** hello */
307+
) = > {}
308+
}
309+
310+
"#]],
311+
);
312+
check(
313+
r#"
314+
macro_rules! m {
315+
($($t:tt)*) => {
316+
macro_rules! m2 {
317+
($($t)*) => {}
318+
}
319+
};
320+
}
321+
322+
m!(/** hello */);
323+
m2!();
324+
"#,
325+
expect![[r#"
326+
macro_rules! m {
327+
($($t:tt)*) => {
328+
macro_rules! m2 {
329+
($($t)*) => {}
330+
}
331+
};
332+
}
333+
334+
macro_rules !m2 {
335+
(#[doc = r" hello "]) = > {}
336+
}
337+
/* error: unexpected token in input */
338+
"#]],
339+
);
340+
}

‎crates/hir-def/src/macro_expansion_tests/mod.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -325,7 +325,7 @@ fn pretty_print_macro_expansion(
325325
(T!['{'], T!['}']) => "",
326326
(T![=], _) | (_, T![=]) => " ",
327327
(_, T!['{']) => " ",
328-
(T![;] | T!['{'] | T!['}'], _) => "\n",
328+
(T![;] | T!['{'] | T!['}'] | T![inner_doc_comment] | T![outer_doc_comment], _) => "\n",
329329
(_, T!['}']) => "\n",
330330
_ if (prev_kind.is_any_identifier()
331331
|| prev_kind == LIFETIME_IDENT

‎crates/hir-def/src/macro_expansion_tests/proc_macros.rs‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,8 @@ mod foo {
5050
5151
#[attr1]
5252
#[attr2] struct S;
53-
#[doc = " Foo"] mod foo {
53+
/// Foo
54+
mod foo {
5455
# ![foo]
5556
# ![doc = "123..."]
5657
# ![attr2]
@@ -293,8 +294,11 @@ struct S;
293294
#[doc = "doc attr"]
294295
struct S;
295296
296-
#[doc = " doc string \\n with newline"]
297-
#[doc = "\n MultiLines Doc\n MultiLines Doc\n"]
297+
/// doc string \n with newline
298+
/**
299+
MultiLines Doc
300+
MultiLines Doc
301+
*/
298302
#[doc = "doc attr"] struct S;"##]],
299303
);
300304
}

‎crates/hir-expand/src/attrs.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -437,7 +437,7 @@ impl AttrId {
437437
tt.syntax(),
438438
SpanMap::RealSpanMap(&span_map),
439439
span_map.span_for_range(tt.syntax().text_range()),
440-
DocCommentDesugarMode::ProcMacro,
440+
DocCommentDesugarMode::Keep,
441441
);
442442
let Some((_, _, derive_tts)) =
443443
parse_path_comma_token_tree(db, &tt).nth(derive_index as usize)

‎crates/hir-expand/src/builtin/derive_macro.rs‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -295,7 +295,7 @@ fn parse_adt_from_syntax(
295295
it.syntax(),
296296
tm,
297297
call_site,
298-
DocCommentDesugarMode::ProcMacro,
298+
DocCommentDesugarMode::Keep,
299299
)
300300
}
301301
None => {
@@ -309,7 +309,7 @@ fn parse_adt_from_syntax(
309309
it.syntax(),
310310
tm,
311311
call_site,
312-
DocCommentDesugarMode::ProcMacro,
312+
DocCommentDesugarMode::Keep,
313313
)
314314
}),
315315
ast::TypeOrConstParam::Const(_) => None,
@@ -322,7 +322,7 @@ fn parse_adt_from_syntax(
322322
ty.syntax(),
323323
tm,
324324
call_site,
325-
DocCommentDesugarMode::ProcMacro,
325+
DocCommentDesugarMode::Keep,
326326
)
327327
})
328328
.unwrap_or_else(|| {
@@ -343,7 +343,7 @@ fn parse_adt_from_syntax(
343343
it.syntax(),
344344
tm,
345345
call_site,
346-
DocCommentDesugarMode::ProcMacro,
346+
DocCommentDesugarMode::Keep,
347347
)
348348
})
349349
.collect()
@@ -380,7 +380,7 @@ fn parse_adt_from_syntax(
380380
it.syntax(),
381381
tm,
382382
call_site,
383-
DocCommentDesugarMode::ProcMacro,
383+
DocCommentDesugarMode::Keep,
384384
)
385385
})
386386
.collect();
@@ -664,7 +664,7 @@ fn coerce_shared_target(
664664
FxHashMap::default(),
665665
remove,
666666
span,
667-
DocCommentDesugarMode::ProcMacro,
667+
DocCommentDesugarMode::Keep,
668668
|_, _| (true, Vec::new()),
669669
);
670670

@@ -1464,7 +1464,7 @@ fn coerce_pointee_expand(
14641464
self_for_traits.syntax(),
14651465
&span_map,
14661466
span,
1467-
DocCommentDesugarMode::ProcMacro,
1467+
DocCommentDesugarMode::Keep,
14681468
);
14691469
let info = match parse_adt_from_syntax(&adt, &span_map, span) {
14701470
Ok(it) => it,

‎crates/hir-expand/src/builtin/fn_macro.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -842,7 +842,7 @@ fn include_expand(
842842
&editioned_file_id.parse(db).syntax_node(),
843843
crate::HirFileId::from(editioned_file_id).span_map(db),
844844
span,
845-
syntax_bridge::DocCommentDesugarMode::ProcMacro,
845+
syntax_bridge::DocCommentDesugarMode::Keep,
846846
))
847847
}
848848

‎crates/hir-expand/src/cfg_process.rs‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -297,18 +297,24 @@ pub(crate) fn attr_macro_input_to_token_tree(
297297
span_map: SpanMap<'_>,
298298
span: Span,
299299
is_derive: bool,
300+
is_declarative: bool,
300301
censor_item_tree_attr_ids: &[AttrId],
301302
krate: Crate,
302303
) -> (tt::TopSubtree, SyntaxFixupUndoInfo) {
303-
let fixups = fixup::fixup_syntax(span_map, node, span, DocCommentDesugarMode::ProcMacro);
304+
let doc_comment_mode = if is_declarative {
305+
DocCommentDesugarMode::DesugarMbeInput
306+
} else {
307+
DocCommentDesugarMode::Keep
308+
};
309+
let fixups = fixup::fixup_syntax(span_map, node, span, doc_comment_mode);
304310
(
305311
syntax_bridge::syntax_node_to_token_tree_modified(
306312
node,
307313
span_map,
308314
fixups.append,
309315
fixups.remove,
310316
span,
311-
DocCommentDesugarMode::ProcMacro,
317+
doc_comment_mode,
312318
macro_input_callback(db, is_derive, censor_item_tree_attr_ids, krate, span, span_map),
313319
),
314320
fixups.undo_info,

‎crates/hir-expand/src/declarative.rs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -136,7 +136,7 @@ impl AstId<ast::Macro> {
136136
map.span_for_range(
137137
macro_rules.macro_rules_token().unwrap().text_range(),
138138
),
139-
DocCommentDesugarMode::Mbe,
139+
DocCommentDesugarMode::Keep,
140140
);
141141

142142
mbe::DeclarativeMacro::parse_macro_rules(&tt, ctx_edition)
@@ -158,14 +158,14 @@ impl AstId<ast::Macro> {
158158
args.syntax(),
159159
map,
160160
span,
161-
DocCommentDesugarMode::Mbe,
161+
DocCommentDesugarMode::Keep,
162162
)
163163
});
164164
let body = syntax_bridge::syntax_node_to_token_tree(
165165
body.syntax(),
166166
map,
167167
span,
168-
DocCommentDesugarMode::Mbe,
168+
DocCommentDesugarMode::Keep,
169169
);
170170

171171
mbe::DeclarativeMacro::parse_macro2(args.as_ref(), &body, ctx_edition)

0 commit comments

Comments
 (0)