rust-lang / rust-playground

The Rust Playground
https://play.rust-lang.org/
Apache License 2.0
1.26k stars 206 forks source link

`main` function definition not on its own line is not detected #624

Open MonliH opened 4 years ago

MonliH commented 4 years ago

After entering the following code:

use std; fn main(){ println!("Hello, world!"); }

and clicking the "Build" button, I get the following message:

No main function was detected, so your code was compiled
but not run. If you’d like to execute your code, please
add a main function.

No code in the main function is run.



After a bit more testing, I've found that if the main function's signature is not on its own line, the playground fails to detect the function:

// Also doesn't detect main
const foo: i32 = 10; fn main(){}

// Also doesn't detect main
use std; fn main(){
}

// Does detect main
use std;
fn main() {}
sollyucko commented 4 years ago

Detection code, AFAICT: https://github.com/integer32llc/rust-playground/blob/master/ui/frontend/selectors/index.ts#L13

shepmaster commented 4 years ago

I don’t really consider this to be a problem. Why would you choose to write nonstandard Rust? You can run rustfmt to fix it. If you don’t, you can use the drop down and choose run manually.

MonliH commented 4 years ago

I came across this because I was playing around with code golfing in rust :smile:. I needed to store the code somewhere so other people could run it and see my solution, so that's why.

I know its sort of a niche use case, although fixing small things like this makes for a more polished project. That being said, I can see why this wouldn't be high priority. I might be able to patch this as well.

sollyucko commented 4 years ago

I think removing the ^ at the start that requires the RegExp to match at the start of the line should probably work. The starting \s* (matches optional whitespace) and the ending m (^ and $ can match the start/end of the line instead of the whole string) can optionally be removed.

shepmaster commented 4 years ago

removing the ^ at the start

// fn main() {}
MonliH commented 4 years ago

Hmm, yeah. Seems like there would be a a few more edge cases you need to deal with, if you remove that:

/* fn main() {} */
const FOO: &'static str = r#"fn main() {}
"#;

Maybe look for a main function on the server side instead? i.e. is there a way to detect if a main function exists after compiling it?

shepmaster commented 4 years ago

after compiling it?

The compilation command and even the file name itself changes based on how it's being compiled.

sollyucko commented 4 years ago

I guess you could build it, then analyze the binary to detect the presence of a main function (Run) or a test runner (Test), then determine what to do with it.

Also, multi-line comments can still trick the selector:

/*
fn main() {}
*/

Playground URL: https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=5c587547e3f69172f6d75909badb8f78

shepmaster commented 4 years ago

To be clear, any and every regex solution is basically "best effort", thus why I don't consider this a priority. If the code does the right thing for the usual and expected cases (i.e. properly formatted code), that's what's important. If it also does the right thing for less-common cases (e.g. const fn main()) without breaking usual cases and without introducing extremely slow performance, that's ok.

shepmaster commented 4 years ago

And this is one reason why the dropdown exists — for when we get it wrong

image

MonliH commented 4 years ago

Just to note: that wasn't immediately obvious to me when I ran into this. I didn't even notice that playground changed the mode based on whether it detected a main function or not.

Maybe you could (somehow) make this slightly more clear?

shepmaster commented 4 years ago

Maybe you could (somehow) make this slightly more clear?

How do you suggest?

The text changes from run to build and there’s output added that says no main was detected.

MonliH commented 4 years ago

Not sure how much it will help, but maybe add something to that message, along the lines of:

If you think this was a mistake, change the mode to "Run" and try again.

This could add a bit of noise, though.

c-git commented 2 years ago

Was trying out the examples in the Begin Rust book and one of the examples has a comment in the brackets of the main and I also ran into this issue.

fn main(/* comment */) {
   // snip
}

I think I regex to work properly in this case would that be a useful PR?

PS. Adding to the "no main found message" that there is an option to choose run instead of build would be good. I can try to draft a updated message if that would be helpful.

c-git commented 2 years ago

I just noticed that this is tagged with help wanted. If desirable I think I can also update the regex to work with the case that this issue was originally opened for.

shepmaster commented 2 years ago

update the regex to work with the case that this issue was originally opened for

that'd be wonderful!

I can try to draft a updated message if that would be helpful.

sure!

c-git commented 1 year ago

With regard to the updated message I think we should probably add the following to the end

"or if you would like to try to run anyway please using the three dots next to the BUILD button to select RUN from the drop down."

So the full message would read:

No main function was detected, so your code was compiled but not run. If you’d like to execute your code, please [add a main function]() or if you would like to try to run anyway please use the three dots next to the BUILD button to select RUN from the drop down.

c-git commented 1 year ago

I've created the PR to address the issue that was originally reported and the one I encountered while following the Begin Rust Book. I think I could have also prevented

/*
fn main( ){
    println!("Hello, world!");
}
*/

from being detected as having a main but I think it would have caused a non-negligible runtime cost because it would have to check from each /* until it got to main. Didn't spend too much time thinking it all the way through because the cost didn't seem worthwhile.

shepmaster commented 1 year ago

I think we should probably add the following to the end

This seems reasonable, but making it actionable is probably even a better step:

No main function was detected, so your code was compiled but not run. If you’d like to execute your code, please add a main function or override automatic detection and run the code anyway.

I'm torn on how best to describe the additional menu next to the build/run button, or even if we should!

c-git commented 1 year ago

Will clicking the link cause the "button type" to change like the drop down would? If it does then I don't think describing the button adds much value as they would have gotten what they want and might notice that the button can change and that accomplishes the same goal. But even if they don't notice they would have still gotten their code to run and I think that would suffice. I only tried to describe how to use the drop down because I didn't think of a link. I think the link is "easier" to use than try to follow descriptions which can be misinterpreted (or just not clear).