Greatly simplify doctest parsing and information extraction by GuillaumeGomez · Pull Request #138104 · rust-lang/rust

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!("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

 }
+
+#[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

-    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

+                    && (!info.maybe_crate_attrs.is_empty() || !info.crate_attrs.is_empty())
                {
-                    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-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 Status: Awaiting review from the assignee but also interested parties. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. 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 Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. 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 Relevant to the rustdoc-frontend team, which will review and decide on the web UI/UX output. 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 Area: port run-make Makefiles to rmake.rs label Mar 7, 2025

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

fmease approved these changes Mar 27, 2025

View reviewed changes

fmease added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. 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

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the 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 Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Mar 28, 2025

Update coverage-run-rustdoc output

87d524b

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the 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

fmease mentioned this pull request Apr 10, 2025

Doc test containing code before main function fails to compile on beta #139651

Closed

apiraino mentioned this pull request Apr 22, 2025

Regression on doc-tests with statements and a main function #140162

Closed

This was referenced Apr 25, 2025

Fix detection of main function if there are expressions around it #140220

Merged

rustdoc: Fix doctest heuristic for main fn wrapping #140420

Merged

fmease mentioned this pull request Sep 25, 2025

rustdoc: Since 1.87 we permit shebangs in doctests that have a main fn #147014

Open

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

Uh oh!

Conversation

GuillaumeGomez commented Mar 6, 2025

Uh oh!

rustbot commented Mar 7, 2025

Uh oh!

GuillaumeGomez commented Mar 8, 2025

Uh oh!

fmease left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

GuillaumeGomez commented Mar 27, 2025

Uh oh!

bors commented Mar 27, 2025

Uh oh!

jhpratt commented Mar 28, 2025

Uh oh!

GuillaumeGomez commented Mar 28, 2025

Uh oh!

bors commented Mar 28, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

fmease left a comment •

edited

Loading