tests: add basic test coverage for stable `--crate-type` flag #134906

jieyouxu · 2024-12-29T22:01:51Z

I experimented locally with making the compiler panic if multiple crate types are passed to a single --crate-type flag, and it turns out not only does the compiler successfully bootstrap, only two ui tests failed:

failures:
    [ui] tests/ui/sepcomp/sepcomp-lib.rs
    [ui] tests/ui/sepcomp/sepcomp-lib-lto.rs

test result: FAILED. 4 passed; 2 failed; 18181 ignored; 0 measured; 0 filtered out; finished in 364.55ms

These are not specific tests for the --crate-type flag, only their auxiliary happens to use a --crate-type with two crate types:

//@ compile-flags: -C codegen-units=3 --crate-type=rlib,dylib -g

I was not able to find a specific test in run-make test suite either.

So this PR tries to add some basic test coverage for the stable --crate-type flag's interface (including to check that --crate-type accepts multiple crate types), since I imagine it might be slightly awkward if we accidentally regressed this.

r? compiler

jieyouxu · 2024-12-29T22:03:00Z

tests/ui/cli/crate-type-flag.rs

+//!
+//! This test does not try to check if the output artifacts are valid.
+
+// FIXME(#132309): add a proper `supports-crate-type` directive.


This is a known could-be-improved compiletest directive handling aspect #132309 to not have all the random ignore-*s, didn't want to fix that in this PR.

jieyouxu · 2024-12-29T22:05:57Z

tests/ui/cli/crate-type-flag.rs

+
+//@ revisions: multivalue multivalue_combined
+
+//@[multivalue] compile-flags: --crate-type=lib,rlib,staticlib


I never knew this was a valid form, just stumbed across this while reviewing #134720 and was wondering why the crate-type parser accepted a list_list.

jieyouxu · 2024-12-29T22:07:14Z

tests/ui/cli/crate-type-flag.rs

+//@ revisions: underscore_proc_macro
+
+//@[underscore_proc_macro] compile-flags: --crate-type=proc_macro
+//@[underscore_proc_macro] error-pattern: "unknown crate type: `proc_macro`"


I intentionally used error-pattern because the stderr snapshots can be easy to accidentally bless away, but this forces the stderr to contain the unknown crate type error message ~~(fuzzily matched)~~.

You could even use //@ dont-check-compiler-stderr for the 2 tests, so that the stderr snapshots are not saved, and are impossible to bless away ^^.

I believe the stderr snapshots are still compared when I only have error-pattern. This is (1) useful to detect if the exact error message changed, but can be easily reblessed (2) let's you preview what the error message looks like (more specifically say if we add a suggestion) e.g. #134720.

If you find this confusing, then that's #134888 😆

jieyouxu · 2024-12-29T22:09:12Z

tests/ui/cli/crate-type-flag.rs

This test is under tests/ui/cli (new subdirectory) because I couldn't find a better home and didn't want to throw it directly under tests/ui/. I imagine other stable cli flags likely are also under-tested...

Please use the preexisting tests/ui/invalid-compile-flags/¹ instead. See discussion in #133277.

Footnotes

ls tests/ui/invalid-compile-flags/**/*.rs | wc -l = 11. ↩

I imagine other stable cli flags likely are also under-tested...

Undoubtedly yes. Well, apart from tests/ui/invalid-compile-flags/ I know of tests/ui/extern-flag/ for --extern which contains 16 UI tests.

Ah thanks for the pointers. I stared at ls -d tests/ui/*/ without noticing that directory...

Please use the preexisting tests/ui/invalid-compile-flags/1 instead. See discussion in #133277.

FTR, that directory would be a bit misleading for this test. This test has both "positive coverage", i.e. does a basic smoke test of the simple 1-crate-type cases and multi-crate-type cases, and it also has "negative coverage", i.e. that some invalid crate-type formulations are rejected with an error.

Indeed:

tests/ui/invalid-compile-flags/branch-protection-missing-pac-ret.rs tests/ui/invalid-compile-flags/codegen-option-without-group.rs tests/ui/invalid-compile-flags/debug-option-without-group.rs tests/ui/invalid-compile-flags/function-return/requires-x86-or-x86_64.rs tests/ui/invalid-compile-flags/function-return/thunk-extern-requires-non-large-code-model.rs tests/ui/invalid-compile-flags/invalid-llvm-passes.rs tests/ui/invalid-compile-flags/print-without-arg.rs tests/ui/invalid-compile-flags/print.rs tests/ui/invalid-compile-flags/reg-struct-return/requires-x86.rs tests/ui/invalid-compile-flags/regparm/regparm-valid-values.rs tests/ui/invalid-compile-flags/regparm/requires-x86.rs

All of these seems to be exercising invalid compile-flags (i.e. are all negative coverage).

EDIT: started a discussion in https://rust-lang.zulipchat.com/#narrow/channel/131828-t-compiler/topic/Slightly.20reorganizing.20compile.20flags.20test.20coverage, but I'm going to stuff this test under the existing directory in this PR.

Anyway, moved the test under invalid-compile-flags/ since I didn't want to make a new directory.

jieyouxu · 2024-12-29T22:09:37Z

@bors rollup=iffy (exercises all the crate-types)

lqd · 2024-12-29T22:48:47Z

tests/ui/cli/crate-type-flag.rs

+//@ revisions: underscore_proc_macro
+
+//@[underscore_proc_macro] compile-flags: --crate-type=proc_macro
+//@[underscore_proc_macro] error-pattern: "unknown crate type: `proc_macro`"


You could even use //@ dont-check-compiler-stderr for the 2 tests, so that the stderr snapshots are not saved, and are impossible to bless away ^^.

tests/ui/cli/crate-type-flag.rs

lqd · 2024-12-29T23:02:09Z

with the nits above (+ fmease's suggestion of the appropriate test directory),

lqd · 2024-12-30T10:48:13Z

👍

r=lqd,fmease when green

jieyouxu · 2024-12-30T11:11:42Z

tests/ui/invalid-compile-flags/crate-type-flag.rs

+//@[unknown] compile-flags: --crate-type=🤡
+//@[unknown] error-pattern: "unknown crate type: `🤡`"
+


Remark: changed this to use an actual Unicode emoji, to check that we can handle non-ASCII inputs as well.

jieyouxu · 2024-12-30T13:57:41Z

@bors r=lqd,fmease

bors · 2024-12-30T13:57:43Z

📌 Commit 92e8e84 has been approved by lqd,fmease

It is now in the queue for this repository.

bors · 2024-12-30T15:44:07Z

⌛ Testing commit 92e8e84 with merge 93722f7...

bors · 2024-12-30T18:23:30Z

☀️ Test successful - checks-actions
Approved by: lqd,fmease
Pushing 93722f7 to master...

rust-timer · 2024-12-30T19:39:08Z

Finished benchmarking commit (93722f7): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

Results (primary -2.6%, secondary 1.8%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	1.8%	[1.2%, 2.3%]	2
Improvements ✅ (primary)	-2.6%	[-2.6%, -2.6%]	1
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-2.6%	[-2.6%, -2.6%]	1

Cycles

Results (secondary 2.4%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	2.4%	[2.3%, 2.6%]	2
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-	-	0

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 761.456s -> 759.393s (-0.27%)
Artifact size: 325.54 MiB -> 325.55 MiB (0.01%)

rustbot assigned cjgillot Dec 29, 2024

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 Dec 29, 2024

jieyouxu changed the title ~~tests: add basic test coverage for cli flag --crate-type~~ tests: add basic test coverage for stable --crate-type flag Dec 29, 2024

jieyouxu force-pushed the multi-crate-type branch from 16d3c41 to 3572361 Compare December 29, 2024 22:04

jieyouxu commented Dec 29, 2024

View reviewed changes

jieyouxu added A-testsuite Area: The testsuite used to check the correctness of rustc A-CLI Area: Command-line interface (CLI) to the compiler labels Dec 29, 2024

lqd reviewed Dec 29, 2024

View reviewed changes

jieyouxu 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 Dec 29, 2024

jieyouxu force-pushed the multi-crate-type branch 2 times, most recently from 4a5a2d3 to 912f6e3 Compare December 30, 2024 10:41

tests: add basic test coverage for cli flag --crate-type

92e8e84

jieyouxu force-pushed the multi-crate-type branch from 912f6e3 to 92e8e84 Compare December 30, 2024 11:10

jieyouxu commented Dec 30, 2024

View reviewed changes

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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Dec 30, 2024

bors added the merged-by-bors This PR was explicitly merged by bors. label Dec 30, 2024

bors merged commit 93722f7 into rust-lang:master Dec 30, 2024
7 checks passed

rustbot added this to the 1.85.0 milestone Dec 30, 2024

jieyouxu deleted the multi-crate-type branch December 30, 2024 21:58

jieyouxu mentioned this pull request Feb 14, 2025

Add --print=supported-crate-types rust-lang/compiler-team#836

Closed

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

tests: add basic test coverage for stable `--crate-type` flag #134906

tests: add basic test coverage for stable `--crate-type` flag #134906

jieyouxu commented Dec 29, 2024

jieyouxu Dec 29, 2024 •

edited

Loading

jieyouxu Dec 29, 2024

jieyouxu Dec 29, 2024 •

edited

Loading

lqd Dec 29, 2024 •

edited

Loading

jieyouxu Dec 29, 2024 •

edited

Loading

jieyouxu Dec 29, 2024

jieyouxu Dec 29, 2024 •

edited

Loading

fmease Dec 30, 2024

fmease Dec 30, 2024 •

edited

Loading

jieyouxu Dec 30, 2024 •

edited

Loading

jieyouxu Dec 30, 2024 •

edited

Loading

jieyouxu Dec 30, 2024

jieyouxu commented Dec 29, 2024

lqd Dec 29, 2024 •

edited

Loading

lqd commented Dec 29, 2024 •

edited

Loading

lqd commented Dec 30, 2024

jieyouxu Dec 30, 2024 •

edited

Loading

jieyouxu commented Dec 30, 2024

bors commented Dec 30, 2024

bors commented Dec 30, 2024

bors commented Dec 30, 2024

rust-timer commented Dec 30, 2024


		//@ revisions: multivalue multivalue_combined

		//@[multivalue] compile-flags: --crate-type=lib,rlib,staticlib

		//@[unknown] compile-flags: --crate-type=🤡
		//@[unknown] error-pattern: "unknown crate type: `🤡`"

tests: add basic test coverage for stable --crate-type flag #134906

tests: add basic test coverage for stable --crate-type flag #134906

Conversation

jieyouxu commented Dec 29, 2024

jieyouxu Dec 29, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jieyouxu Dec 29, 2024 • edited Loading

Choose a reason for hiding this comment

lqd Dec 29, 2024 • edited Loading

Choose a reason for hiding this comment

jieyouxu Dec 29, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jieyouxu Dec 29, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Footnotes

fmease Dec 30, 2024 • edited Loading

Choose a reason for hiding this comment

jieyouxu Dec 30, 2024 • edited Loading

Choose a reason for hiding this comment

jieyouxu Dec 30, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jieyouxu commented Dec 29, 2024

lqd Dec 29, 2024 • edited Loading

Choose a reason for hiding this comment

lqd commented Dec 29, 2024 • edited Loading

lqd commented Dec 30, 2024

jieyouxu Dec 30, 2024 • edited Loading

Choose a reason for hiding this comment

jieyouxu commented Dec 30, 2024

bors commented Dec 30, 2024

bors commented Dec 30, 2024

bors commented Dec 30, 2024

rust-timer commented Dec 30, 2024

Overall result: no relevant changes - no action needed

tests: add basic test coverage for stable `--crate-type` flag #134906

tests: add basic test coverage for stable `--crate-type` flag #134906

jieyouxu Dec 29, 2024 •

edited

Loading

jieyouxu Dec 29, 2024 •

edited

Loading

lqd Dec 29, 2024 •

edited

Loading

jieyouxu Dec 29, 2024 •

edited

Loading

jieyouxu Dec 29, 2024 •

edited

Loading

fmease Dec 30, 2024 •

edited

Loading

jieyouxu Dec 30, 2024 •

edited

Loading

jieyouxu Dec 30, 2024 •

edited

Loading

lqd Dec 29, 2024 •

edited

Loading

lqd commented Dec 29, 2024 •

edited

Loading

jieyouxu Dec 30, 2024 •

edited

Loading