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

Don't ICE when getting an input file name's stem fails #128710

Merged
merged 3 commits into from
Aug 7, 2024

Conversation

ChrisDenton
Copy link
Member

Fixes #128681

The file stem is only used as a user-friendly prefix on intermediary files. While nice to have, it's not the end of the world if it fails so there's no real reason to emit an error here. We can continue with a fixed name as we do when an anonymous string is used.

@rustbot
Copy link
Collaborator

rustbot commented Aug 5, 2024

r? @fee1-dead

rustbot has assigned @fee1-dead.
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

@rustbot rustbot added A-run-make Area: port run-make Makefiles to rmake.rs 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 Aug 5, 2024
@rustbot
Copy link
Collaborator

rustbot commented Aug 5, 2024

This PR modifies tests/run-make/. If this PR is trying to port a Makefile
run-make test to use rmake.rs, please update the
run-make port tracking issue
so we can track our progress. You can either modify the tracking issue
directly, or you can comment on the tracking issue and link this PR.

cc @jieyouxu

@jieyouxu
Copy link
Member

jieyouxu commented Aug 5, 2024

r? @jieyouxu

@rustbot rustbot assigned jieyouxu and unassigned fee1-dead Aug 5, 2024
@jieyouxu
Copy link
Member

jieyouxu commented Aug 5, 2024

Looks reasonable to me from first glance, I'll double check what Input::file_stem is used for.

@ChrisDenton
Copy link
Member Author

Oh it'll also be used for the final artefact name if it's the only (or first?) input and it's not otherwise named. I think this is fine though as it's the same as piping input.

@jieyouxu
Copy link
Member

jieyouxu commented Aug 5, 2024

Oh it'll also be used for the final artefact name if it's the only (or first?) input and it's not otherwise named. I think this is fine though as it's the same as piping input.

I took a look and AFAICT this is fine -- we can't be naming the output file \\.\NUL in the first place (that would be an epic troll on the end user). Do you think we should add a check for the rust_out fallback output artifact name behavior? I'm fine with accepting this falling back to rust_out with or without this additional check.

@ChrisDenton
Copy link
Member Author

Tbh, I'm not sure. But I guess adding a check can't hurt too much. Worst case someone might have to update the test in the future.

@jieyouxu
Copy link
Member

jieyouxu commented Aug 6, 2024

Tbh, I'm not sure. But I guess adding a check can't hurt too much. Worst case someone might have to update the test in the future.

Yeah, I don't think we care about the exact behavior of this too much, but it seems better to at least have a test to know this behavior is expected somewhere.

Copy link
Member

@jieyouxu jieyouxu left a comment

Choose a reason for hiding this comment

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

Thanks for melting the ICE! 😎

@jieyouxu
Copy link
Member

jieyouxu commented Aug 6, 2024

Please r=me after PR CI is green

@ChrisDenton
Copy link
Member Author

Thanks for melting the ICE! 😎

🔥And a super important one too! 🔥

@bors r=jieyouxu rollup

@bors
Copy link
Contributor

bors commented Aug 6, 2024

📌 Commit 3fd645e has been approved by jieyouxu

It is now in the queue for this repository.

@bors 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 Aug 6, 2024
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Aug 6, 2024
Don't ICE when getting an input file name's stem fails

Fixes rust-lang#128681

The file stem is only used as a user-friendly prefix on intermediary files. While nice to have, it's not the end of the world if it fails so there's no real reason to emit an error here. We can continue with a fixed name as we do when an anonymous string is used.
bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 6, 2024
…iaskrgr

Rollup of 7 pull requests

Successful merges:

 - rust-lang#122792 (Stabilize `min_exhaustive_patterns`)
 - rust-lang#124944 (On trait bound mismatch, detect multiple crate versions in dep tree)
 - rust-lang#128273 (Improve `Ord` violation help)
 - rust-lang#128406 (implement BufReader::peek)
 - rust-lang#128539 (Forbid unused unsafe in vxworks-specific std modules)
 - rust-lang#128692 (Add a triagebot mention for `library/Cargo.lock`)
 - rust-lang#128710 (Don't ICE when getting an input file name's stem fails)

r? `@ghost`
`@rustbot` modify labels: rollup
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Aug 6, 2024
Don't ICE when getting an input file name's stem fails

Fixes rust-lang#128681

The file stem is only used as a user-friendly prefix on intermediary files. While nice to have, it's not the end of the world if it fails so there's no real reason to emit an error here. We can continue with a fixed name as we do when an anonymous string is used.
bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 6, 2024
…iaskrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#125048 (PinCoerceUnsized trait into core)
 - rust-lang#128273 (Improve `Ord` violation help)
 - rust-lang#128406 (implement BufReader::peek)
 - rust-lang#128539 (Forbid unused unsafe in vxworks-specific std modules)
 - rust-lang#128687 (interpret: refactor function call handling to be better-abstracted)
 - rust-lang#128692 (Add a triagebot mention for `library/Cargo.lock`)
 - rust-lang#128710 (Don't ICE when getting an input file name's stem fails)
 - rust-lang#128718 (Consider `cfg_attr` checked by `CheckAttrVisitor`)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 6, 2024
…iaskrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#125048 (PinCoerceUnsized trait into core)
 - rust-lang#128273 (Improve `Ord` violation help)
 - rust-lang#128406 (implement BufReader::peek)
 - rust-lang#128539 (Forbid unused unsafe in vxworks-specific std modules)
 - rust-lang#128687 (interpret: refactor function call handling to be better-abstracted)
 - rust-lang#128692 (Add a triagebot mention for `library/Cargo.lock`)
 - rust-lang#128710 (Don't ICE when getting an input file name's stem fails)
 - rust-lang#128718 (Consider `cfg_attr` checked by `CheckAttrVisitor`)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 7, 2024
…iaskrgr

Rollup of 9 pull requests

Successful merges:

 - rust-lang#124944 (On trait bound mismatch, detect multiple crate versions in dep tree)
 - rust-lang#125048 (PinCoerceUnsized trait into core)
 - rust-lang#128406 (implement BufReader::peek)
 - rust-lang#128539 (Forbid unused unsafe in vxworks-specific std modules)
 - rust-lang#128687 (interpret: refactor function call handling to be better-abstracted)
 - rust-lang#128692 (Add a triagebot mention for `library/Cargo.lock`)
 - rust-lang#128710 (Don't ICE when getting an input file name's stem fails)
 - rust-lang#128718 (Consider `cfg_attr` checked by `CheckAttrVisitor`)
 - rust-lang#128751 (std::thread: set_name implementation proposal for vxWorks.)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit f9325b7 into rust-lang:master Aug 7, 2024
6 checks passed
@rustbot rustbot added this to the 1.82.0 milestone Aug 7, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Aug 7, 2024
Rollup merge of rust-lang#128710 - ChrisDenton:null, r=jieyouxu

Don't ICE when getting an input file name's stem fails

Fixes rust-lang#128681

The file stem is only used as a user-friendly prefix on intermediary files. While nice to have, it's not the end of the world if it fails so there's no real reason to emit an error here. We can continue with a fixed name as we do when an anonymous string is used.
@ChrisDenton ChrisDenton deleted the null branch August 7, 2024 08:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-run-make Area: port run-make Makefiles to rmake.rs S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

ICE when using \\.\NUL as the input filename on Windows
5 participants