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

[regression] empty shebang parsing #73250

Closed
Mark-Simulacrum opened this issue Jun 11, 2020 · 6 comments · Fixed by #73596
Closed

[regression] empty shebang parsing #73250

Mark-Simulacrum opened this issue Jun 11, 2020 · 6 comments · Fixed by #73596
Labels
P-medium Medium priority regression-from-stable-to-beta Performance or correctness regression from stable to beta. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Milestone

Comments

@Mark-Simulacrum
Copy link
Member

We fail to parse this (admittedly questionable) code as of beta:

https://play.rust-lang.org/?version=nightly&mode=debug&edition=2018&gist=4fd6b726ceaddf0c06e67761d2276277

#!

use std::any;

fn main() {}

This broke one repository (not crate) in the beta crater run: https://crater-reports.s3.amazonaws.com/beta-1.45-1/beta-2020-06-03/gh/grng3r.rust_todo/log.txt

@Mark-Simulacrum Mark-Simulacrum added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. regression-from-stable-to-beta Performance or correctness regression from stable to beta. labels Jun 11, 2020
@Mark-Simulacrum Mark-Simulacrum added this to the 1.45 milestone Jun 11, 2020
@rustbot rustbot added the I-prioritize Issue: Indicates that prioritization has been requested for this issue. label Jun 11, 2020
@Mark-Simulacrum
Copy link
Member Author

cc @petrochenkov

I personally suspect that we may want to ask T-lang if we're good accepting this as expected breakage. Alternatively I guess we can adjust the shebang detection to not require a non-empty shebang.

@petrochenkov petrochenkov self-assigned this Jun 11, 2020
@lcnr lcnr added P-medium Medium priority and removed I-prioritize Issue: Indicates that prioritization has been requested for this issue. labels Jun 12, 2020
@lcnr
Copy link
Contributor

lcnr commented Jun 12, 2020

@petrochenkov
Copy link
Contributor

For reference, here's the code responsible for parsing shebang in Linux kernel - https://github.com/torvalds/linux/blob/master/fs/binfmt_script.c

(Fully whitespace (including empty) #! line isn't considered a shebang.)

@Mark-Simulacrum
Copy link
Member Author

I am inclined to close as won't fix; let's cc @grng3r as the author of that repository as well.

I'm going to go ahead and nominate this so that we can get a 👍 (or not) at the T-compiler meeting confirming won't fix status.

@petrochenkov
Copy link
Contributor

I made a fix in #73596, which also has a benefit of simplifying the rules slightly.

@petrochenkov petrochenkov removed their assignment Jun 21, 2020
@bors bors closed this as completed in dda8a7f Jun 27, 2020
@grng3r
Copy link

grng3r commented Jun 29, 2020 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P-medium Medium priority regression-from-stable-to-beta Performance or correctness regression from stable to beta. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants