Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Convert run-pass tests with trivial main to build-pass. #119484

Closed
wants to merge 1 commit into from

Conversation

cjgillot
Copy link
Contributor

@cjgillot cjgillot commented Jan 1, 2024

If main is empty, having a run-pass test does not bring anything more than a build-pass one.

@rustbot
Copy link
Collaborator

rustbot commented Jan 1, 2024

r? @petrochenkov

(rustbot has picked a reviewer for you, use r? to override)

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jan 1, 2024
@rust-log-analyzer

This comment has been minimized.

@tmiasko
Copy link
Contributor

tmiasko commented Jan 1, 2024

Multiple entry points (under #[cfg(...)], --test, etc):

  • tests/ui/abi/abi-sysv64-register-usage.rs
  • tests/ui/env-null-vars.rs
  • tests/ui/issues/issue-21634.rs
  • tests/ui/rfcs/rfc-1014-stdout-existential-crisis/rfc-1014-2.rs
  • tests/ui/simple_global_asm.rs
  • tests/ui/test-attrs/test-type.rs
  • tests/ui/test-attrs/test-vs-cfg-test.rs
  • tests/ui/threads-sendsync/thread-local-extern-static.rs
  • tests/ui/weird-exit-code.rs

Potentially testing if built executable works:

  • tests/ui/abi/relocation_model_pic.rs
  • tests/ui/cfg/crt-static-off-works.rs
  • tests/ui/cfg/crt-static-on-works.rs

// run-pass
#![allow(dead_code)]
#![allow(unused_mut)]
#![allow(unused_imports)]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume you used some script or regex to remove the attributes.
Could you tweak it to remove lines entirely, instead of leaving empty lines?


#![feature(coroutines)]

fn _run(bar: &mut i32) {
|| { //~ WARN unused coroutine that must be used
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe it makes sense to add #[warn(...)] attributes to keep the warnings when they are significant.

@@ -1,4 +1,4 @@
// run-pass
// build-pass
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

False positive, the empty main is under cfg.

@petrochenkov
Copy link
Contributor

petrochenkov commented Jan 1, 2024

Many of these can probably be check-pass, but build-pass is already an improvement.

Also need to double check any tests involving potential life before and after main - global variables with constructors/destructors, #[link_section], #[used], maybe something else + also conditional main like in #119484 (comment).
+ also custom codegen options that may result in the produced binary crashing even with empty main.

@petrochenkov petrochenkov 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 Jan 1, 2024
// run-pass
// build-pass
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this main is not very trivial

@@ -1,4 +1,4 @@
// run-pass
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This tests dynamic linking. The original issue #12133 also looks like it encountered issue at runtime.

I'm concerned that these changes are applied too broadly and some other subtle things that happen before/after main are being missed.

@cjgillot cjgillot closed this Jan 1, 2024
@cjgillot
Copy link
Contributor Author

cjgillot commented Jan 1, 2024

I should really avoid filing PR 30mins before new year. Thankfully that happens only once a year.

@cjgillot cjgillot deleted the build-pass branch January 5, 2024 23:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants