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

GuillaumeGomez · 2025-04-23T20:11:04Z

Once this is merged, we can backport and I'll send a follow-up to emit a warning in case a main function is about to be "wrapped" (and therefore not run).

r? @fmease

try-job: x86_64-mingw-1

rustbot · 2025-04-23T20:11:07Z

fmease is on vacation.

Please choose another assignee.

rustbot · 2025-04-23T20:11:09Z

r? @notriddle

rustbot has assigned @notriddle.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

fmease · 2025-04-23T20:11:44Z

I'll review tonight!

GuillaumeGomez · 2025-04-23T20:12:37Z

(If you're in vacations, it's fine not too but I really appreciate!)

jyn514 · 2025-04-23T20:28:58Z

src/librustdoc/doctest/make.rs

+                            reset_error_count(&psess);
+                            return Err(());
+                        }
+                        has_non_module_items = true;
                    }
                    StmtKind::MacCall(ref mac_call) if !info.has_main_fn => {


i think you need to also assume that macros can expand to expressions, not just items. in fact i think you need to do this for any statement other than an item.

I voluntarily ignored it for the simple fact that trying to match on a macro call is pretty much useless since we have no clue how it will be expanded. The fact that we match on fn main is already a pretty bad idea. If I start saying "everything that doesn't match the first condition means it's an expression", then this code would fail to compile:

include!("auxiliary/empty.rs"); fn main() {}

Because "auxiliary/empty.rs" would be considered an expression, meaning the whole code would be wrapped, and the compiler would be unhappy:

error: non-statement macro in statement position: include | 3 | include!("auxiliary/empty.rs"); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

So sadly, I think the best course of action here is to keep this ugly hack...

While Jyn is obviously right regarding macro expansion, older versions of rustdoc (e.g., 1.86, 1.75) don't handle those top-level macros calls correctly either, I mean ones which expand to exprs. At least from what I've gathered. I tested:

//! ``` //! macro_rules! mk { () => { if true { return; } } } //! mk!(); //! fn main() {} //! ```

1.88, 1.86, 1.75: macro expansion ignores keyword `if` and any tokens following // the usage of `mk!` is likely invalid in item context.

So it's fine to keep treating macro calls as item macro calls. All of this is a heuristic anyway and I'm so glad GuillaumeGomez threw out the janky partially string-based heuristics in #138104 which also used to reparse the input string multiple times.

As it's the case with all heuristics, there will always be false positives and negatives.

Guillaume, could you add a short comment above the MacroCall arm mentioning that we assume that the macro calls expand to item(s) even though they could expand to stmts and exprs, too?

fmease

r=me with nits addressed. sorry for the broken promise and the delay

fmease · 2025-04-25T13:29:26Z

src/librustdoc/doctest/make.rs

@@ -462,6 +470,11 @@ fn parse_source(source: &str, crate_name: &Option<&str>) -> Result<ParseSourceIn
                    push_to_s(&mut info.crates, source, span, &mut prev_span_hi);
                }
            }
+            if has_non_module_items {
+                // FIXME: if `info.has_main_fn` is `true`, emit a warning here to mention that


👍 Could you open an issue for that unless we already have one for that?

Here it is: #140310

fmease · 2025-04-25T13:35:28Z

src/librustdoc/doctest/make.rs

@@ -437,7 +441,11 @@ fn parse_source(source: &str, crate_name: &Option<&str>) -> Result<ParseSourceIn
                            }
                        }
                    }
-                    _ => {}
+                    // We do nothing in this case. Not marking it as `non_module_items` either.
+                    StmtKind::Empty => {}


No, we should set has_non_module_items = true when encountering StmtKind::Empty since it represents lone ;s which are not valid items.

Consider:

//! ``` //! fn main() {}; //! ```

or

//! ``` //! ;fn main() {} //! ```

which lead to expected item, found `;` on master.

However as a matter of fact, <=1.86 did not handle this 'correctly' either.

EDIT: This comment is completely wrong.

Weeeeell... Let me raise to you:

struct s{}; fn main() {}

:D

There is even tests/rustdoc-ui/doctest/failed-doctest-extra-semicolon-on-item.rs to check it. So sorry but I'll leave it as is for backward compatibility. :')

Wait, ignore my previous comment, it's not allowed to have a rogue semi-colon in code like that. I'll change it then.

fmease · 2025-04-25T13:52:34Z

src/librustdoc/doctest/make.rs

+                            reset_error_count(&psess);
+                            return Err(());
+                        }
+                        has_non_module_items = true;
                    }
                    StmtKind::MacCall(ref mac_call) if !info.has_main_fn => {


While Jyn is obviously right regarding macro expansion, older versions of rustdoc (e.g., 1.86, 1.75) don't handle those top-level macros calls correctly either, I mean ones which expand to exprs. At least from what I've gathered. I tested:

//! ``` //! macro_rules! mk { () => { if true { return; } } } //! mk!(); //! fn main() {} //! ```

1.88, 1.86, 1.75: macro expansion ignores keyword `if` and any tokens following // the usage of `mk!` is likely invalid in item context.

So it's fine to keep treating macro calls as item macro calls. All of this is a heuristic anyway and I'm so glad GuillaumeGomez threw out the janky partially string-based heuristics in #138104 which also used to reparse the input string multiple times.

As it's the case with all heuristics, there will always be false positives and negatives.

fmease · 2025-04-25T13:58:07Z

src/librustdoc/doctest/make.rs

+                            reset_error_count(&psess);
+                            return Err(());
+                        }
+                        has_non_module_items = true;
                    }
                    StmtKind::MacCall(ref mac_call) if !info.has_main_fn => {


Guillaume, could you add a short comment above the MacroCall arm mentioning that we assume that the macro calls expand to item(s) even though they could expand to stmts and exprs, too?

src/librustdoc/doctest/make.rs

GuillaumeGomez · 2025-04-25T19:37:16Z

r=me with nits addressed. sorry for the broken promise and the delay

No worries, you're supposed to be in vacation. If I had known, I wouldn't have set you as reviewer. But in any case, thanks a lot! Gonna update then r+.

… inside a function

GuillaumeGomez · 2025-04-25T21:11:22Z

@bors r=fmease rollup

bors · 2025-04-25T21:11:25Z

📌 Commit 3ef98a5 has been approved by fmease

It is now in the queue for this repository.

…ng, r=fmease Fix detection of main function if there are expressions around it Fixes rust-lang#140162. Fixes rust-lang#139651. Once this is merged, we can backport and I'll send a follow-up to emit a warning in case a `main` function is about to be "wrapped" (and therefore not run). r? `@fmease`

…iaskrgr Rollup of 9 pull requests Successful merges: - rust-lang#139865 (Stabilize proc_macro::Span::{start,end,line,column}.) - rust-lang#140086 (If creating a temporary directory fails with permission denied then retry with backoff) - rust-lang#140216 (Document that "extern blocks must be unsafe" in Rust 2024) - rust-lang#140220 (Fix detection of main function if there are expressions around it) - rust-lang#140253 (Add XtensaAsmPrinter) - rust-lang#140272 (Improve error message for `||` (or) in let chains) - rust-lang#140305 (Track per-obligation recursion depth only if there is inference in the new solver) - rust-lang#140306 (handle specialization in the new trait solver) - rust-lang#140308 (stall generator witness obligations: add regression test) r? `@ghost` `@rustbot` modify labels: rollup

…ng, r=fmease Fix detection of main function if there are expressions around it Fixes rust-lang#140162. Fixes rust-lang#139651. Once this is merged, we can backport and I'll send a follow-up to emit a warning in case a `main` function is about to be "wrapped" (and therefore not run). r? `@fmease` try-job: x86_64-gnu-aux

…ng, r=fmease Fix detection of main function if there are expressions around it Fixes rust-lang#140162. Fixes rust-lang#139651. Once this is merged, we can backport and I'll send a follow-up to emit a warning in case a `main` function is about to be "wrapped" (and therefore not run). r? ``@fmease`` try-job: x86_64-gnu-aux

Rollup of 7 pull requests Successful merges: - rust-lang#137439 (Stabilise `std::ffi::c_str`) - rust-lang#138737 (uefi: Update r-efi) - rust-lang#139646 (check types of const param defaults) - rust-lang#140220 (Fix detection of main function if there are expressions around it) - rust-lang#140291 (Correctly display stdout and stderr in case a doctest is failing) - rust-lang#140297 (Update example to use CStr::to_string_lossy) - rust-lang#140330 (Clarified bootstrap optimization "true" argument) r? `@ghost` `@rustbot` modify labels: rollup

…ng, r=fmease Fix detection of main function if there are expressions around it Fixes rust-lang#140162. Fixes rust-lang#139651. Once this is merged, we can backport and I'll send a follow-up to emit a warning in case a `main` function is about to be "wrapped" (and therefore not run). r? ```@fmease``` try-job: x86_64-gnu-aux

Rollup of 8 pull requests Successful merges: - rust-lang#137439 (Stabilise `std::ffi::c_str`) - rust-lang#139031 (Use char::is_whitespace directly in str::trim*) - rust-lang#139090 (fix docs for `Peekable::next_if{_eq}`) - rust-lang#140220 (Fix detection of main function if there are expressions around it) - rust-lang#140297 (Update example to use CStr::to_string_lossy) - rust-lang#140330 (Clarified bootstrap optimization "true" argument) - rust-lang#140339 (session: Cleanup `CanonicalizedPath::new`) - rust-lang#140348 (Update lint-docs to default to Rust 2024) r? `@ghost` `@rustbot` modify labels: rollup

tgross35 · 2025-04-27T07:52:37Z

This seems like the most likely cause of #140356 (comment)

@bors r-

tgross35 · 2025-04-27T07:52:53Z

@bors try

bors · 2025-04-27T07:54:04Z

⌛ Trying commit aa69e3a with merge eee34b5...

…, r=<try> Fix detection of main function if there are expressions around it Fixes rust-lang#140162. Fixes rust-lang#139651. Once this is merged, we can backport and I'll send a follow-up to emit a warning in case a `main` function is about to be "wrapped" (and therefore not run). r? `@fmease` try-job: x86_64-mingw-1

bors · 2025-04-27T10:33:57Z

☀️ Try build successful - checks-actions
Build commit: eee34b5 (eee34b57cac5916bdbde87e06eb6149b92dfc405)

tgross35 · 2025-04-27T16:56:49Z

Huh, spurious then?

@bors r=fmease rollup=maybe

bors · 2025-04-27T16:56:52Z

📌 Commit aa69e3a has been approved by fmease

It is now in the queue for this repository.

…llaumeGomez Rollup of 7 pull requests Successful merges: - rust-lang#140056 (Fix a wrong error message in 2024 edition) - rust-lang#140220 (Fix detection of main function if there are expressions around it) - rust-lang#140249 (Remove `weak` alias terminology) - rust-lang#140316 (Introduce `BoxMarker` to improve pretty-printing correctness) - rust-lang#140347 (ci: clean more disk space in codebuild) - rust-lang#140349 (ci: use aws codebuild for the `dist-x86_64-linux` job) - rust-lang#140379 (rustc-dev-guide subtree update) r? `@ghost` `@rustbot` modify labels: rollup

Rollup merge of rust-lang#140220 - GuillaumeGomez:doctest-main-wrapping, r=fmease Fix detection of main function if there are expressions around it Fixes rust-lang#140162. Fixes rust-lang#139651. Once this is merged, we can backport and I'll send a follow-up to emit a warning in case a `main` function is about to be "wrapped" (and therefore not run). r? `@fmease` try-job: x86_64-mingw-1

matthiaskrgr · 2025-04-28T21:36:32Z

maybe we should have cratered this 🤔

fmease · 2025-04-29T00:08:57Z

maybe we should have cratered this

Not really, no. We employ crater if we intentionally change a behavior whereas the negative fallout you're seeing (the failed cargotest test and now issue #140412) is due to unintentional behavior changes.

As the reviewer I gladly take the blame here. The former issue I completely missed, that's totally on me. It's great that the cargotest smoke test caught it.

Regarding the latter (#140412), I have to note that I didn't have time to re-review the PR after the latest push (I only skimmed the comments). However, that's still my fault since I should've blocked the PR on a proper re-review given the delicate nature of the doctest impl and the previous failure.

matthiaskrgr · 2025-04-29T04:41:56Z

🤔
what I mean is, we already knew we are likely to cause negative fallout because it slipped through the cracks once, so why not make sure there is nothing more we missed?

GuillaumeGomez added 2 commits April 23, 2025 22:09

Fix detection of main function if there are expressions around it

3536324

Add regression ui test for rust-lang#140162 and for rust-lang#139651

81438c0

rustbot assigned notriddle Apr 23, 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 Apr 23, 2025

GuillaumeGomez mentioned this pull request Apr 23, 2025

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

Closed

fmease assigned fmease and unassigned notriddle Apr 23, 2025

GuillaumeGomez mentioned this pull request Apr 23, 2025

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

Closed

jyn514 reviewed Apr 23, 2025

View reviewed changes

fmease approved these changes Apr 25, 2025

View reviewed changes

fmease reviewed Apr 25, 2025

View reviewed changes

src/librustdoc/doctest/make.rs Outdated Show resolved Hide resolved

GuillaumeGomez added 2 commits April 25, 2025 22:01

Improve code

3ededc1

If there is a ; alone, we consider that the doctest needs to be put…

3ef98a5

… inside a function

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 Apr 25, 2025

matthiaskrgr mentioned this pull request Apr 25, 2025

Rollup of 9 pull requests #140315

Closed

jhpratt mentioned this pull request Apr 26, 2025

Rollup of 14 pull requests #140321

Closed

tgross35 mentioned this pull request Apr 27, 2025

Rollup of 8 pull requests #140354

Closed

tgross35 mentioned this pull request Apr 27, 2025

Rollup of 7 pull requests #140355

Closed

tgross35 mentioned this pull request Apr 27, 2025

Rollup of 8 pull requests #140356

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 Apr 27, 2025

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 Apr 27, 2025

GuillaumeGomez mentioned this pull request Apr 28, 2025

Rollup of 7 pull requests #140388

Merged

bors merged commit a782b54 into rust-lang:master Apr 28, 2025
7 checks passed

rustbot added this to the 1.88.0 milestone Apr 28, 2025

ehuss mentioned this pull request Apr 28, 2025

Regression in doctest compile_fail detection with global_asm! #140412

Open

fmease mentioned this pull request Apr 29, 2025

rustdoc: Fix doctest heuristic for main fn wrapping #140420

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

GuillaumeGomez commented Apr 23, 2025 •

edited by tgross35

Loading

rustbot commented Apr 23, 2025

rustbot commented Apr 23, 2025

fmease commented Apr 23, 2025

GuillaumeGomez commented Apr 23, 2025

jyn514 Apr 23, 2025 •

edited

Loading

GuillaumeGomez Apr 24, 2025

fmease Apr 25, 2025 •

edited

Loading

fmease Apr 25, 2025 •

edited

Loading

GuillaumeGomez Apr 25, 2025

fmease left a comment

fmease Apr 25, 2025

GuillaumeGomez Apr 25, 2025

fmease Apr 25, 2025

fmease Apr 25, 2025

GuillaumeGomez Apr 25, 2025 •

edited

Loading

GuillaumeGomez Apr 25, 2025

fmease Apr 25, 2025 •

edited

Loading

fmease Apr 25, 2025 •

edited

Loading

GuillaumeGomez commented Apr 25, 2025

GuillaumeGomez commented Apr 25, 2025

bors commented Apr 25, 2025

tgross35 commented Apr 27, 2025

tgross35 commented Apr 27, 2025

bors commented Apr 27, 2025

bors commented Apr 27, 2025

tgross35 commented Apr 27, 2025

bors commented Apr 27, 2025

matthiaskrgr commented Apr 28, 2025

fmease commented Apr 29, 2025 •

edited

Loading

matthiaskrgr commented Apr 29, 2025

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

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

Conversation

GuillaumeGomez commented Apr 23, 2025 • edited by tgross35 Loading

rustbot commented Apr 23, 2025

rustbot commented Apr 23, 2025

fmease commented Apr 23, 2025

GuillaumeGomez commented Apr 23, 2025

jyn514 Apr 23, 2025 • edited Loading

Choose a reason for hiding this comment

GuillaumeGomez Apr 24, 2025

Choose a reason for hiding this comment

fmease Apr 25, 2025 • edited Loading

Choose a reason for hiding this comment

fmease Apr 25, 2025 • edited Loading

Choose a reason for hiding this comment

GuillaumeGomez Apr 25, 2025

Choose a reason for hiding this comment

fmease left a comment

Choose a reason for hiding this comment

fmease Apr 25, 2025

Choose a reason for hiding this comment

GuillaumeGomez Apr 25, 2025

Choose a reason for hiding this comment

fmease Apr 25, 2025

Choose a reason for hiding this comment

fmease Apr 25, 2025

Choose a reason for hiding this comment

GuillaumeGomez Apr 25, 2025 • edited Loading

Choose a reason for hiding this comment

GuillaumeGomez Apr 25, 2025

Choose a reason for hiding this comment

fmease Apr 25, 2025 • edited Loading

Choose a reason for hiding this comment

fmease Apr 25, 2025 • edited Loading

Choose a reason for hiding this comment

GuillaumeGomez commented Apr 25, 2025

GuillaumeGomez commented Apr 25, 2025

bors commented Apr 25, 2025

tgross35 commented Apr 27, 2025

tgross35 commented Apr 27, 2025

bors commented Apr 27, 2025

bors commented Apr 27, 2025

tgross35 commented Apr 27, 2025

bors commented Apr 27, 2025

matthiaskrgr commented Apr 28, 2025

fmease commented Apr 29, 2025 • edited Loading

matthiaskrgr commented Apr 29, 2025

GuillaumeGomez commented Apr 23, 2025 •

edited by tgross35

Loading

jyn514 Apr 23, 2025 •

edited

Loading

fmease Apr 25, 2025 •

edited

Loading

fmease Apr 25, 2025 •

edited

Loading

GuillaumeGomez Apr 25, 2025 •

edited

Loading

fmease Apr 25, 2025 •

edited

Loading

fmease Apr 25, 2025 •

edited

Loading

fmease commented Apr 29, 2025 •

edited

Loading