[rustdoc] Do not consider nested functions as main function even if named `main` in doctests #132105

GuillaumeGomez · 2024-10-24T13:07:01Z

If a nested function is called main, it is not considered as the entry point of the program. Therefore, doctests should not consider such functions as such either.

r? @notriddle

notriddle · 2024-10-24T17:05:42Z

src/librustdoc/doctest/make.rs

@@ -298,12 +303,14 @@ fn parse_source(
        match item.kind {
            ast::ItemKind::Fn(ref fn_item) if !info.has_main_fn => {
                if item.ident.name == sym::main {
-                    info.has_main_fn = true;
+                    info.has_main_fn = is_top_level;


Suggested change

info.has_main_fn = is_top_level;

info.has_main_fn = info.has_main_fn || is_top_level;

Also, please add this test case

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

Oof, good catch!

…in` in doctests

notriddle · 2024-10-24T23:00:11Z

@bors r+

bors · 2024-10-24T23:00:14Z

📌 Commit aab2b67 has been approved by notriddle

It is now in the queue for this repository.

bors · 2024-10-25T03:52:38Z

⌛ Testing commit aab2b67 with merge 9372bec...

…r=notriddle [rustdoc] Do not consider nested functions as main function even if named `main` in doctests Fixes rust-lang#131893. If a nested function is called `main`, it is not considered as the entry point of the program. Therefore, doctests should not consider such functions as such either. r? `@notriddle`

bors · 2024-10-25T04:45:47Z

⌛ Testing commit aab2b67 with merge 044686e...

jieyouxu · 2024-10-25T04:46:17Z

Something about the queue is really messed up, I'll do a resync
EDIT: #t-infra zulip thread: https://rust-lang.zulipchat.com/#narrow/channel/242791-t-infra/topic/bors.20merging.20merged.20PRs
@bors treeclosed=1000

jieyouxu · 2024-10-25T05:02:35Z

??? Does editing the treeclosed comment reopen the tree
@bors treeclosed=1000

jieyouxu · 2024-10-25T05:04:52Z

It's as if this PR is still being tested, so to be safe:
@bors r- retry

jieyouxu · 2024-10-25T05:05:02Z

@bors r-

jieyouxu · 2024-10-25T05:05:58Z

@bors r=@notriddle

bors · 2024-10-25T05:06:00Z

📌 Commit aab2b67 has been approved by notriddle

It is now in the queue for this repository.

bors · 2024-10-25T05:06:01Z

🌲 The tree is currently closed for pull requests below priority 1000. This pull request will be tested once the tree is reopened.

jieyouxu · 2024-10-25T05:09:58Z

Seems to have finished resyncing, so
@bors treeclosed-

bors · 2024-10-25T05:11:08Z

⌛ Testing commit aab2b67 with merge 017ae1b...

bors · 2024-10-25T07:41:24Z

☀️ Test successful - checks-actions
Approved by: notriddle
Pushing 017ae1b to master...

GuillaumeGomez · 2024-10-25T08:09:20Z

Should we backport it?

rust-timer · 2024-10-25T09:38:52Z

Finished benchmarking commit (017ae1b): comparison URL.

Overall result: ✅ improvements - no action needed

@rustbot label: -perf-regression

Instruction count

This is the most reliable metric that we have; it was used to determine the overall result at the top of this comment. However, even this metric can sometimes exhibit noise.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-1.0%	[-1.5%, -0.4%]	2
All ❌✅ (primary)	-	-	0

Max RSS (memory usage)

Results (primary 2.4%, secondary 3.8%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	2.4%	[2.4%, 2.4%]	1
Regressions ❌ (secondary)	3.8%	[3.8%, 3.8%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	2.4%	[2.4%, 2.4%]	1

Cycles

This benchmark run did not return any relevant results for this metric.

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 783.653s -> 781.857s (-0.23%)
Artifact size: 333.71 MiB -> 333.71 MiB (0.00%)

notriddle · 2024-10-26T17:02:30Z

Should we backport it?

I wouldn't. There's a very easy workaround, it went a long time without anyone hitting the bug, and it causes a failed compile instead of something worse (like a miscompile or a spurious test pass).

rustbot assigned notriddle Oct 24, 2024

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 Oct 24, 2024

This comment has been minimized.

Sign in to view

GuillaumeGomez force-pushed the doctest-nested-main branch from 006d5d1 to d51fe6e Compare October 24, 2024 13:21

This comment has been minimized.

Sign in to view

GuillaumeGomez force-pushed the doctest-nested-main branch from b559eba to 942b449 Compare October 24, 2024 14:33

notriddle reviewed Oct 24, 2024

View reviewed changes

GuillaumeGomez added 2 commits October 24, 2024 21:35

Do not consider nested functions as main function even if named `ma…

0e0a84a

…in` in doctests

Add regression test for rust-lang#131893

aab2b67

GuillaumeGomez force-pushed the doctest-nested-main branch from 942b449 to aab2b67 Compare October 24, 2024 19:35

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-review Status: Awaiting review from the assignee but also interested parties. labels Oct 24, 2024

This comment was marked as outdated.

Sign in to view

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 Oct 25, 2024

This comment was marked as outdated.

Sign in to view

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 Oct 25, 2024

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 Oct 25, 2024

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 Oct 25, 2024

bors added the merged-by-bors This PR was explicitly merged by bors. label Oct 25, 2024

bors merged commit 017ae1b into rust-lang:master Oct 25, 2024
7 checks passed

rustbot added this to the 1.84.0 milestone Oct 25, 2024

GuillaumeGomez deleted the doctest-nested-main branch October 25, 2024 08:08

	info.has_main_fn = is_top_level;
	info.has_main_fn = info.has_main_fn \|\| is_top_level;

[rustdoc] Do not consider nested functions as main function even if named main in doctests #132105

[rustdoc] Do not consider nested functions as main function even if named main in doctests #132105

Uh oh!

Conversation

GuillaumeGomez commented Oct 24, 2024

Uh oh!

This comment has been minimized.

This comment has been minimized.

notriddle Oct 24, 2024

Choose a reason for hiding this comment

Uh oh!

GuillaumeGomez Oct 24, 2024

Choose a reason for hiding this comment

Uh oh!

notriddle commented Oct 24, 2024

Uh oh!

bors commented Oct 24, 2024

Uh oh!

bors commented Oct 25, 2024

Uh oh!

bors commented Oct 25, 2024

Uh oh!

jieyouxu commented Oct 25, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

jieyouxu commented Oct 25, 2024

Uh oh!

jieyouxu commented Oct 25, 2024

Uh oh!

jieyouxu commented Oct 25, 2024

Uh oh!

jieyouxu commented Oct 25, 2024

Uh oh!

bors commented Oct 25, 2024

Uh oh!

bors commented Oct 25, 2024

Uh oh!

jieyouxu commented Oct 25, 2024

Uh oh!

bors commented Oct 25, 2024

Uh oh!

bors commented Oct 25, 2024

Uh oh!

Uh oh!

GuillaumeGomez commented Oct 25, 2024

Uh oh!

rust-timer commented Oct 25, 2024

Overall result: ✅ improvements - no action needed

Instruction count

Max RSS (memory usage)

Cycles

Binary size

Uh oh!

notriddle commented Oct 26, 2024

Uh oh!

Uh oh!

[rustdoc] Do not consider nested functions as main function even if named `main` in doctests #132105

[rustdoc] Do not consider nested functions as main function even if named `main` in doctests #132105

jieyouxu commented Oct 25, 2024 •

edited

Loading