Don't emit an error about failing to produce a file with a specific name if user never gave an explicit name

[pacak]

Fixes #122509

You can ask rustc to produce some intermediate results with --emit foo, this operation comes in two flavors: --emit asm and --emit asm=foo.s. First one produces one or more .s files without any name guarantees, second one renames it into foo.s. Second version only works when compiler produces a single file - for asm files this means using a single compilation unit for example.

In case compilation produced more than a single file rustc runs following check to emit some warnings:

            if crate_output.outputs.contains_key(&output_type) {
                // 2) Multiple codegen units, with `--emit foo=some_name`. We have
                //    no good solution for this case, so warn the user.
                sess.dcx().emit_warn(errors::IgnoringEmitPath { extension });
            } else if crate_output.single_output_file.is_some() {
                // 3) Multiple codegen units, with `-o some_name`. We have
                //    no good solution for this case, so warn the user.
                sess.dcx().emit_warn(errors::IgnoringOutput { extension });
            } else {
                // 4) Multiple codegen units, but no explicit name. We
                //    just leave the `foo.0.x` files in place.
                // (We don't have to do any work in this case.)
            }

Comment in the final else branch implies that if user didn't ask for a specific name - there's no need to emit warnings. However because of the internal representation of crate_output.outputs - this doesn't work as expected: if user asked to produce an asm file without giving it an implicit name it will contain Some(None).

To fix the problem new code actually checks if user gave an explicit name. I think this was an original intentional behavior, at least comments imply that.

[rustbot]

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @michaelwoerister (or someone else) some time within the next two weeks.

Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (S-waiting-on-review and S-waiting-on-author) stays updated, invoking these commands when appropriate:

• @rustbot author: the review is finished, PR author should check the comments and take action accordingly
• @rustbot review: the author is ready for a review, this PR will be queued again in the reviewer's queue

[michaelwoerister]

Thanks for the PR, @pacak!

Would you mind adding a regression test?
https://rustc-dev-guide.rust-lang.org/tests/adding.html

Let me know if you need any help with that.

[pacak]

Absolutely. What is the most reliable way to ensure that the code will be compiled with at least two codegen units?

[michaelwoerister]

Passing -Ccodegen-units=2 and having at least two mods should do the trick.

[michaelwoerister]

Compile flags like that can be set in the test file: https://rustc-dev-guide.rust-lang.org/tests/headers.html

[pacak]

I think that's it. It supposed to produce an error before (I didn't check though) and it is not producing an error now.

[pacak]

tidy error: file tests/ui/issue-122509.rs must begin with a descriptive name, consider {reason}-issue-122509.rs

Looking at other tests that do start with just issue - "all tests are equal, but some are more equal..."

[pacak]

tidy error: following path contains more than 859 entries, you should move the test to some relevant subdirectory (current: 860): /checkout/tests/ui

There's no relevant subdirectory for invalid warnings though as far as I can see. I tried moving it into issues - it also complains about having too many items...

[pacak]

Adding a directory to tests/ui doesn't work either.

I give up.

[pacak]

So, please avoid putting a new test there and try to find a more relevant place.

Okay, now there's a sub directory for warnings.

[michaelwoerister]

I'm sorry those tidy errors seemed to have sent you on a wild goose chase. There indeed doesn't seem to be a relevant pre-existing subdirectory for that test.

The test and the fix both look good!

@bors r+ rollup

[bors]

📌 Commit b84326e has been approved by michaelwoerister

It is now in the queue for this repository.

[pacak]

I'm sorry those tidy errors seemed to have sent you on a wild goose chase.

Not your fault. And now I know where to look and why is it happening :)