Greatly simplify doctest parsing and information extraction #138104

GuillaumeGomez · 2025-03-06T15:48:27Z

The original process was pretty terrible, as it tried to extract information such as attributes by performing matches over tokens like #!, which doesn't work very well considering you can have # ! [, which is valid.

Also, it now does it in one pass: if the parser is happy, then we try to extract information, otherwise we return early.

r? @fmease

rustbot · 2025-03-07T20:57:26Z

This PR modifies run-make tests.

cc @jieyouxu

GuillaumeGomez · 2025-03-08T00:13:55Z

CI finally passed! \o/

fmease

Thanks! Very cool. I have some comments but after that we should be good to go. Review comments starting with (preexisting) don't need to be addressed in this PR or by you for that matter

fmease · 2025-03-26T23:35:01Z

src/librustdoc/doctest/make.rs

+        debug!("crate_attrs:\n{crate_attrs}{maybe_crate_attrs}");
+        debug!("crates:\n{crates}");
+        debug!("after:\n{everything_else}");
+
        // If the AST returned an error, we don't want this doctest to be merged with the


Outdated sentence: On parse errors, we already return early above yielding invalid which has has !can_be_merged and failed_ast.

fmease · 2025-03-26T23:53:02Z

src/librustdoc/doctest/tests.rs

@@ -401,3 +402,26 @@ fn check_split_args() {
    compare("a\n\t \rb", &["a", "b"]);
    compare("a\n\t1 \rb", &["a", "1", "b"]);
 }
+
+#[test]
+fn comment_in_attrs() {


Wait, what has this test to do with comments? There are no comments, only an inner doc comment – which also isn't "in [an] attr"

fmease · 2025-03-26T23:57:04Z

src/librustdoc/doctest/tests.rs

+
+#[test]
+fn comment_in_attrs() {
+    // if input already has a fn main, it should insert a space before it


I'm confused what does this comment have to do with the input string? There is no fn main in it and there's also no space getting inserted before anything (I can only see an extra empty line in the generated main fn)

I'll rewrite the comment to explain better the problem.

Hum also the comment is wrong. XD

fmease · 2025-03-27T00:35:27Z

src/librustdoc/doctest/make.rs

+        // We need to shift by 1 because we added `{` at the beginning of the source.we provided
+        // to the parser.


Suggested change

// We need to shift by 1 because we added `{` at the beginning of the source.we provided

// to the parser.

// We need to shift by 1 because we added `{` at the beginning of the

// source we provided to the parser.

fmease · 2025-03-27T00:35:55Z

src/librustdoc/doctest/make.rs

+        prev_span_hi: &mut Option<usize>,
+    ) {
+        let extra_len = DOCTEST_CODE_WRAPPER.len();
+        // We need to shift by 1 because we added `{` at the beginning of the source.we provided


Outdated comment? We added more than { and we don't shift by 1

fmease · 2025-03-27T01:55:45Z

src/librustdoc/doctest/make.rs

-    AstError,
-    Ok,
-}
-
 fn cancel_error_count(psess: &ParseSess) {


(preexisting) This would ideally also be called reset_error_count canceling counting and resetting counting are different things >.<

fmease · 2025-03-27T02:03:33Z

src/librustdoc/doctest/make.rs

                {
-                    PartitionState::Crates
+                    // We add potential backlines/comments if there are some in items generated


backlines? line breaks?

Reading my comment destroyed my brain. What the hell happened when I wrote it? O.o

Ok got it, I definitely need to improve this...

fmease · 2025-03-27T02:10:30Z

src/librustdoc/doctest/make.rs

+                        while let Some(token) = iter.next() {
+                            if let TokenTree::Token(token, _) = token
+                                && let TokenKind::Ident(name, _) = token.kind
+                                && name == kw::Fn
+                                && let Some(TokenTree::Token(fn_token, _)) = iter.peek()
+                                && let TokenKind::Ident(fn_name, _) = fn_token.kind
+                                && fn_name == sym::main
+                                && let Some(TokenTree::Delimited(_, _, Delimiter::Parenthesis, _)) = {
+                                    iter.next();
+                                    iter.peek()
+                                }
+                            {
+                                info.has_main_fn = true;
+                                break;
+                            }
                        }


I appreciate the effort to try and get rod of the string-based fallback logic. It's a bit overkill maybe but I guess it's fine.

Tho note that this check will run a lot more often than the other check. E.g., it will check all macro calls given

//! ```` //! first!(...); //! second!(...); //! fn main() {} //! ```

because we go in source order. The old string-based fallback check only kicked in when it couldn't find fn main anywhere else. I guess in practice it doesn't really matter and doing two passes would be even more overkill

fmease · 2025-03-27T02:13:39Z

src/librustdoc/doctest/make.rs

+        s: &mut String,
+        source: &str,
+        span: rustc_span::Span,
+        prev_span_hi: &mut Option<usize>,


Suggested change

prev_span_hi: &mut Option<usize>,

prev_span_hi: &mut usize,

this could just be an usize right? starting off at 0 instead of None in parse_source

Fair enough! I do .unwrap_or(0) with it so it's definitely useless to keep it in an Option...

fmease · 2025-03-27T02:18:32Z

src/librustdoc/doctest/make.rs

+
+    fn push_to_s(
+        s: &mut String,
+        source: &str,


Wait, if you just passed wrapped_source we wouldn't need to fix the any of the byte offsets, right? Could we do that instead please

It would force to clone wrapped_source since it's passed by value to new_parser_from_source_str. So I'd rather not.

Ah, missed that! Fair fair

outside of wrapping function

GuillaumeGomez · 2025-03-27T16:08:31Z

Ah interesting, rebasing allowed me to uncover a new bug. New regression test incoming. :D

…en first non-crate items and the crate items

GuillaumeGomez · 2025-03-27T20:30:43Z

Applied suggestions. Since it was approved, let's go! :D

@bors r=fmease rollup

bors · 2025-03-27T20:30:46Z

📌 Commit 5d27440 has been approved by fmease

It is now in the queue for this repository.

…rsing, r=fmease Greatly simplify doctest parsing and information extraction The original process was pretty terrible, as it tried to extract information such as attributes by performing matches over tokens like `#!`, which doesn't work very well considering you can have `# ! [`, which is valid. Also, it now does it in one pass: if the parser is happy, then we try to extract information, otherwise we return early. r? `@fmease`

Rollup of 5 pull requests Successful merges: - rust-lang#138104 (Greatly simplify doctest parsing and information extraction) - rust-lang#139010 (Improve `xcrun` error handling) - rust-lang#139021 (std: get rid of pre-Vista fallback code) - rust-lang#139026 (Use `abs_diff` where applicable) - rust-lang#139030 (saethlin goes on vacation) r? `@ghost` `@rustbot` modify labels: rollup

jhpratt · 2025-03-28T01:40:52Z

@bors r-

#139033 (comment)

GuillaumeGomez · 2025-03-28T09:36:47Z

@bors r=fmease rollup

bors · 2025-03-28T09:36:50Z

📌 Commit 87d524b has been approved by fmease

It is now in the queue for this repository.

…iaskrgr Rollup of 7 pull requests Successful merges: - rust-lang#137889 (update outdated doc with new example) - rust-lang#138104 (Greatly simplify doctest parsing and information extraction) - rust-lang#138678 (rustc_resolve: fix instability in lib.rmeta contents) - rust-lang#138986 (feat(config): Add ChangeId enum for suppressing warnings) - rust-lang#139038 (Update target maintainers for thumb targets to reflect new REWG Arm team name) - rust-lang#139045 (bootstrap: update `test_find` test) - rust-lang#139047 (Remove ScopeDepth) Failed merges: - rust-lang#139044 (bootstrap: Avoid cloning `change-id` list) r? `@ghost` `@rustbot` modify labels: rollup

Rollup merge of rust-lang#138104 - GuillaumeGomez:simplify-doctest-parsing, r=fmease Greatly simplify doctest parsing and information extraction The original process was pretty terrible, as it tried to extract information such as attributes by performing matches over tokens like `#!`, which doesn't work very well considering you can have `# ! [`, which is valid. Also, it now does it in one pass: if the parser is happy, then we try to extract information, otherwise we return early. r? `@fmease`

rustbot assigned fmease Mar 6, 2025

rustbot added S-waiting-on-review T-rustdoc labels Mar 6, 2025

GuillaumeGomez force-pushed the simplify-doctest-parsing branch from 70b66bc to 4d9a209 Compare March 6, 2025 17:51

fmease added S-waiting-on-author and removed S-waiting-on-review labels Mar 6, 2025

GuillaumeGomez force-pushed the simplify-doctest-parsing branch from 543cc23 to 7e403e8 Compare March 7, 2025 15:17

rustbot added the T-rustdoc-frontend label Mar 7, 2025

GuillaumeGomez force-pushed the simplify-doctest-parsing branch 2 times, most recently from fe7ed38 to 5b53cea Compare March 7, 2025 20:57

rustbot added the A-run-make label Mar 7, 2025

GuillaumeGomez force-pushed the simplify-doctest-parsing branch from 5b53cea to 0aa9752 Compare March 7, 2025 22:48

GuillaumeGomez added S-waiting-on-review A-doctests and removed S-waiting-on-author T-rustdoc-frontend labels Mar 8, 2025

fmease approved these changes Mar 27, 2025

View reviewed changes

fmease added S-waiting-on-author and removed S-waiting-on-review labels Mar 27, 2025

GuillaumeGomez added 8 commits March 27, 2025 11:18

Greatly simplify doctest parsing and information extraction

1f1c630

Update rustdoc-ui tests

01fb7c3

Correctly handle fn main in macro

6f7e8d4

Correctly handle line comments in attributes and generate extern crates

123ea25

outside of wrapping function

Improve comment and test for generated doctest with code comments

be13105

Improve code comment

f333ad8

Add expect to the list of non-crate attributes for doctest generation

8327bb6

Improve code

2004dac

GuillaumeGomez force-pushed the simplify-doctest-parsing branch from 0aa9752 to 796b8d6 Compare March 27, 2025 14:24

rustbot added the T-rustdoc-frontend label Mar 27, 2025

This comment has been minimized.

Sign in to view

GuillaumeGomez added 2 commits March 27, 2025 16:23

Add new regression test for doctest

1494da4

Remove recursion in check_item

49f1e9c

Only take outer attributes into account when generating content betwe…

5d27440

…en first non-crate items and the crate items

GuillaumeGomez force-pushed the simplify-doctest-parsing branch from 796b8d6 to 5d27440 Compare March 27, 2025 16:43

bors added S-waiting-on-bors and removed S-waiting-on-author labels Mar 27, 2025

jhpratt mentioned this pull request Mar 27, 2025

Rollup of 5 pull requests #139033

Closed

bors added S-waiting-on-author and removed S-waiting-on-bors labels Mar 28, 2025

Update coverage-run-rustdoc output

87d524b

bors added S-waiting-on-bors and removed S-waiting-on-author labels Mar 28, 2025

matthiaskrgr mentioned this pull request Mar 28, 2025

Rollup of 7 pull requests #139054

Merged

bors merged commit ad87732 into rust-lang:master Mar 28, 2025
6 checks passed

rustbot added this to the 1.87.0 milestone Mar 28, 2025

GuillaumeGomez deleted the simplify-doctest-parsing branch March 30, 2025 18:19

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Greatly simplify doctest parsing and information extraction #138104

Greatly simplify doctest parsing and information extraction #138104

GuillaumeGomez commented Mar 6, 2025

rustbot commented Mar 7, 2025

GuillaumeGomez commented Mar 8, 2025

fmease left a comment •

edited

Loading

fmease Mar 26, 2025

fmease Mar 26, 2025

fmease Mar 26, 2025

GuillaumeGomez Mar 27, 2025

GuillaumeGomez Mar 27, 2025

fmease Mar 27, 2025

fmease Mar 27, 2025

fmease Mar 27, 2025

fmease Mar 27, 2025

GuillaumeGomez Mar 27, 2025

GuillaumeGomez Mar 27, 2025

fmease Mar 27, 2025

fmease Mar 27, 2025

GuillaumeGomez Mar 27, 2025

fmease Mar 27, 2025

GuillaumeGomez Mar 27, 2025

fmease Mar 27, 2025

This comment has been minimized.

GuillaumeGomez commented Mar 27, 2025

GuillaumeGomez commented Mar 27, 2025

bors commented Mar 27, 2025

jhpratt commented Mar 28, 2025

GuillaumeGomez commented Mar 28, 2025

bors commented Mar 28, 2025

		// We need to shift by 1 because we added `{` at the beginning of the source.we provided
		// to the parser.

Greatly simplify doctest parsing and information extraction #138104

Greatly simplify doctest parsing and information extraction #138104

Conversation

GuillaumeGomez commented Mar 6, 2025

rustbot commented Mar 7, 2025

GuillaumeGomez commented Mar 8, 2025

fmease left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

This comment has been minimized.

GuillaumeGomez commented Mar 27, 2025

GuillaumeGomez commented Mar 27, 2025

bors commented Mar 27, 2025

jhpratt commented Mar 28, 2025

GuillaumeGomez commented Mar 28, 2025

bors commented Mar 28, 2025

fmease left a comment •

edited

Loading