Skip to content

Commit 423e7b8

Browse files
ChrisDentonChrisDenton
authored
Rollup merge of #139618 - petrochenkov:virsugg, r=jieyouxu
compiletest: Make `SUGGESTION` annotations viral If one of them is expected in a test file, then others should be annotated as well, in the same way as with `HELP`s and `NOTE`s. This doesn't require much of an additional annotation burden, but simplifies the rules. r? ```@jieyouxu```
2 parents cc9420f + 06dd9e2 commit 423e7b8

20 files changed

+109
-98
lines changed

Diff for: ‎src/doc/rustc-dev-guide/src/tests/directives.md

+1
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,7 @@ for more details.
101101
| `normalize-stdout` | Normalize actual stdout with a rule `"<raw>" -> "<normalized>"` before comparing against snapshot | `ui`, `incremental` | `"<RAW>" -> "<NORMALIZED>"`, `<RAW>`/`<NORMALIZED>` is regex capture and replace syntax |
102102
| `dont-check-compiler-stderr` | Don't check actual compiler stderr vs stderr snapshot | `ui` | N/A |
103103
| `dont-check-compiler-stdout` | Don't check actual compiler stdout vs stdout snapshot | `ui` | N/A |
104+
| `dont-require-annotations` | Don't require line annotations for the given diagnostic kind (`//~ KIND`) to be exhaustive | `ui`, `incremental` | `ERROR`, `WARN`, `NOTE`, `HELP`, `SUGGESTION` |
104105
| `run-rustfix` | Apply all suggestions via `rustfix`, snapshot fixed output, and check fixed output builds | `ui` | N/A |
105106
| `rustfix-only-machine-applicable` | `run-rustfix` but only machine-applicable suggestions | `ui` | N/A |
106107
| `exec-env` | Env var to set when executing a test | `ui`, `crashes` | `<KEY>=<VALUE>` |

Diff for: ‎src/doc/rustc-dev-guide/src/tests/ui.md

+38-17
Original file line numberDiff line numberDiff line change
@@ -303,8 +303,7 @@ It should be preferred to using `error-pattern`, which is imprecise and non-exha
303303
### `error-pattern`
304304

305305
The `error-pattern` [directive](directives.md) can be used for runtime messages, which don't
306-
have a specific span, or for compile time messages if imprecise matching is required due to
307-
multi-line platform specific diagnostics.
306+
have a specific span, or in exceptional cases for compile time messages.
308307

309308
Let's think about this test:
310309

@@ -318,7 +317,7 @@ fn main() {
318317
```
319318

320319
We want to ensure this shows "index out of bounds" but we cannot use the `ERROR`
321-
annotation since the error doesn't have any span. Then it's time to use the
320+
annotation since the runtime error doesn't have any span. Then it's time to use the
322321
`error-pattern` directive:
323322

324323
```rust,ignore
@@ -331,29 +330,51 @@ fn main() {
331330
}
332331
```
333332

334-
But for strict testing, try to use the `ERROR` annotation as much as possible,
335-
including `//~?` annotations for diagnostics without span.
336-
For compile time diagnostics `error-pattern` should very rarely be necessary.
333+
Use of `error-pattern` is not recommended in general.
337334

338-
Per-line annotations (`//~`) are still checked in tests using `error-pattern`.
339-
To opt out of these checks, use `//@ compile-flags: --error-format=human`.
340-
Do that only in exceptional cases.
335+
For strict testing of compile time output, try to use the line annotations `//~` as much as
336+
possible, including `//~?` annotations for diagnostics without span.
341337

342-
### Error levels
338+
If the compile time output is target dependent or too verbose, use directive
339+
`//@ dont-require-annotations: <diagnostic-kind>` to make the line annotation checking
340+
non-exhaustive, some of the compiler messages can stay uncovered by annotations in this mode.
343341

344-
The error levels that you can have are:
342+
For checking runtime output `//@ check-run-results` may be preferable.
343+
344+
Only use `error-pattern` if none of the above works.
345+
346+
Line annotations `//~` are still checked in tests using `error-pattern`.
347+
In exceptional cases use `//@ compile-flags: --error-format=human` to opt out of these checks.
348+
349+
### Diagnostic kinds (error levels)
350+
351+
The diagnostic kinds that you can have are:
345352

346353
- `ERROR`
347-
- `WARN` or `WARNING`
354+
- `WARN` (or `WARNING`)
348355
- `NOTE`
349-
- `HELP` and `SUGGESTION`
350-
351-
You are allowed to not include a level, but you should include it at least for
352-
the primary message.
356+
- `HELP`
357+
- `SUGGESTION`
353358

354-
The `SUGGESTION` level is used for specifying what the expected replacement text
359+
The `SUGGESTION` kind is used for specifying what the expected replacement text
355360
should be for a diagnostic suggestion.
356361

362+
`ERROR` and `WARN` kinds are required to be exhaustively covered by line annotations
363+
`//~` by default.
364+
365+
Other kinds only need to be line-annotated if at least one annotation of that kind appears
366+
in the test file. For example, one `//~ NOTE` will also require all other `//~ NOTE`s in the file
367+
to be written out explicitly.
368+
369+
Use directive `//@ dont-require-annotations` to opt out of exhaustive annotations.
370+
E.g. use `//@ dont-require-annotations: NOTE` to annotate notes selectively.
371+
Avoid using this directive for `ERROR`s and `WARN`ings, unless there's a serious reason, like
372+
target-dependent compiler output.
373+
374+
Missing diagnostic kinds (`//~ message`) are currently accepted, but are being phased away.
375+
They will match any compiler output kind, but will not force exhaustive annotations for that kind.
376+
Prefer explicit kind and `//@ dont-require-annotations` to achieve the same effect.
377+
357378
UI tests use the `-A unused` flag by default to ignore all unused warnings, as
358379
unused warnings are usually not the focus of a test. However, simple code
359380
samples often have unused warnings. If the test is specifically testing an

Diff for: ‎src/tools/compiletest/src/header.rs

+5-11
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
use std::collections::{HashMap, HashSet};
1+
use std::collections::HashSet;
22
use std::env;
33
use std::fs::File;
44
use std::io::BufReader;
@@ -198,7 +198,7 @@ pub struct TestProps {
198198
/// that don't otherwise want/need `-Z build-std`.
199199
pub add_core_stubs: bool,
200200
/// Whether line annotatins are required for the given error kind.
201-
pub require_annotations: HashMap<ErrorKind, bool>,
201+
pub dont_require_annotations: HashSet<ErrorKind>,
202202
}
203203

204204
mod directives {
@@ -301,13 +301,7 @@ impl TestProps {
301301
no_auto_check_cfg: false,
302302
has_enzyme: false,
303303
add_core_stubs: false,
304-
require_annotations: HashMap::from([
305-
(ErrorKind::Help, true),
306-
(ErrorKind::Note, true),
307-
(ErrorKind::Error, true),
308-
(ErrorKind::Warning, true),
309-
(ErrorKind::Suggestion, false),
310-
]),
304+
dont_require_annotations: Default::default(),
311305
}
312306
}
313307

@@ -593,8 +587,8 @@ impl TestProps {
593587
if let Some(err_kind) =
594588
config.parse_name_value_directive(ln, DONT_REQUIRE_ANNOTATIONS)
595589
{
596-
self.require_annotations
597-
.insert(ErrorKind::expect_from_user_str(&err_kind), false);
590+
self.dont_require_annotations
591+
.insert(ErrorKind::expect_from_user_str(err_kind.trim()));
598592
}
599593
},
600594
);

Diff for: ‎src/tools/compiletest/src/runtest.rs

+13-27
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ use crate::common::{
2222
output_base_dir, output_base_name, output_testname_unique,
2323
};
2424
use crate::compute_diff::{DiffLine, make_diff, write_diff, write_filtered_diff};
25-
use crate::errors::{self, Error, ErrorKind};
25+
use crate::errors::{Error, ErrorKind};
2626
use crate::header::TestProps;
2727
use crate::read2::{Truncated, read2_abbreviated};
2828
use crate::util::{PathBufExt, add_dylib_path, logv, static_regex};
@@ -675,7 +675,7 @@ impl<'test> TestCx<'test> {
675675
}
676676
}
677677

678-
fn check_expected_errors(&self, expected_errors: Vec<errors::Error>, proc_res: &ProcRes) {
678+
fn check_expected_errors(&self, expected_errors: Vec<Error>, proc_res: &ProcRes) {
679679
debug!(
680680
"check_expected_errors: expected_errors={:?} proc_res.status={:?}",
681681
expected_errors, proc_res.status
@@ -710,8 +710,12 @@ impl<'test> TestCx<'test> {
710710
self.testpaths.file.display().to_string()
711711
};
712712

713-
let expect_help = expected_errors.iter().any(|ee| ee.kind == Some(ErrorKind::Help));
714-
let expect_note = expected_errors.iter().any(|ee| ee.kind == Some(ErrorKind::Note));
713+
// Errors and warnings are always expected, other diagnostics are only expected
714+
// if one of them actually occurs in the test.
715+
let expected_kinds: HashSet<_> = [ErrorKind::Error, ErrorKind::Warning]
716+
.into_iter()
717+
.chain(expected_errors.iter().filter_map(|e| e.kind))
718+
.collect();
715719

716720
// Parse the JSON output from the compiler and extract out the messages.
717721
let actual_errors = json::parse_output(&diagnostic_file_name, &proc_res.stderr, proc_res);
@@ -737,8 +741,11 @@ impl<'test> TestCx<'test> {
737741
}
738742

739743
None => {
740-
// If the test is a known bug, don't require that the error is annotated
741-
if self.is_unexpected_compiler_message(&actual_error, expect_help, expect_note)
744+
if actual_error.require_annotation
745+
&& actual_error.kind.map_or(false, |kind| {
746+
expected_kinds.contains(&kind)
747+
&& !self.props.dont_require_annotations.contains(&kind)
748+
})
742749
{
743750
self.error(&format!(
744751
"{}:{}: unexpected {}: '{}'",
@@ -796,27 +803,6 @@ impl<'test> TestCx<'test> {
796803
}
797804
}
798805

799-
/// Returns `true` if we should report an error about `actual_error`,
800-
/// which did not match any of the expected error.
801-
fn is_unexpected_compiler_message(
802-
&self,
803-
actual_error: &Error,
804-
expect_help: bool,
805-
expect_note: bool,
806-
) -> bool {
807-
actual_error.require_annotation
808-
&& actual_error.kind.map_or(false, |err_kind| {
809-
// If the test being checked doesn't contain any "help" or "note" annotations, then
810-
// we don't require annotating "help" or "note" (respecively) diagnostics at all.
811-
let default_require_annotations = self.props.require_annotations[&err_kind];
812-
match err_kind {
813-
ErrorKind::Help => expect_help && default_require_annotations,
814-
ErrorKind::Note => expect_note && default_require_annotations,
815-
_ => default_require_annotations,
816-
}
817-
})
818-
}
819-
820806
fn should_emit_metadata(&self, pm: Option<PassMode>) -> Emit {
821807
match (pm, self.props.fail_mode, self.config.mode) {
822808
(Some(PassMode::Check), ..) | (_, Some(FailMode::Check), Ui) => Emit::Metadata,

Diff for: ‎tests/ui/async-await/suggest-missing-await.rs

+1
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
//@ edition:2018
2+
//@ dont-require-annotations: SUGGESTION
23

34
fn take_u32(_x: u32) {}
45

Diff for: ‎tests/ui/async-await/suggest-missing-await.stderr

+11-11
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,18 @@
11
error[E0308]: mismatched types
2-
--> $DIR/suggest-missing-await.rs:12:14
2+
--> $DIR/suggest-missing-await.rs:13:14
33
|
44
LL | take_u32(x)
55
| -------- ^ expected `u32`, found future
66
| |
77
| arguments to this function are incorrect
88
|
99
note: calling an async function returns a future
10-
--> $DIR/suggest-missing-await.rs:12:14
10+
--> $DIR/suggest-missing-await.rs:13:14
1111
|
1212
LL | take_u32(x)
1313
| ^
1414
note: function defined here
15-
--> $DIR/suggest-missing-await.rs:3:4
15+
--> $DIR/suggest-missing-await.rs:4:4
1616
|
1717
LL | fn take_u32(_x: u32) {}
1818
| ^^^^^^^^ -------
@@ -22,13 +22,13 @@ LL | take_u32(x.await)
2222
| ++++++
2323

2424
error[E0308]: mismatched types
25-
--> $DIR/suggest-missing-await.rs:22:5
25+
--> $DIR/suggest-missing-await.rs:23:5
2626
|
2727
LL | dummy()
2828
| ^^^^^^^ expected `()`, found future
2929
|
3030
note: calling an async function returns a future
31-
--> $DIR/suggest-missing-await.rs:22:5
31+
--> $DIR/suggest-missing-await.rs:23:5
3232
|
3333
LL | dummy()
3434
| ^^^^^^^
@@ -42,7 +42,7 @@ LL | dummy();
4242
| +
4343

4444
error[E0308]: `if` and `else` have incompatible types
45-
--> $DIR/suggest-missing-await.rs:35:9
45+
--> $DIR/suggest-missing-await.rs:36:9
4646
|
4747
LL | let _x = if true {
4848
| ______________-
@@ -64,7 +64,7 @@ LL | dummy().await
6464
| ++++++
6565

6666
error[E0308]: `match` arms have incompatible types
67-
--> $DIR/suggest-missing-await.rs:45:14
67+
--> $DIR/suggest-missing-await.rs:46:14
6868
|
6969
LL | let _x = match 0usize {
7070
| ______________-
@@ -87,7 +87,7 @@ LL ~ 1 => dummy().await,
8787
|
8888

8989
error[E0308]: mismatched types
90-
--> $DIR/suggest-missing-await.rs:53:9
90+
--> $DIR/suggest-missing-await.rs:54:9
9191
|
9292
LL | let _x = match dummy() {
9393
| ------- this expression has type `impl Future<Output = ()>`
@@ -102,7 +102,7 @@ LL | let _x = match dummy().await {
102102
| ++++++
103103

104104
error[E0308]: mismatched types
105-
--> $DIR/suggest-missing-await.rs:67:9
105+
--> $DIR/suggest-missing-await.rs:68:9
106106
|
107107
LL | match dummy_result() {
108108
| -------------- this expression has type `impl Future<Output = Result<(), ()>>`
@@ -118,7 +118,7 @@ LL | match dummy_result().await {
118118
| ++++++
119119

120120
error[E0308]: mismatched types
121-
--> $DIR/suggest-missing-await.rs:69:9
121+
--> $DIR/suggest-missing-await.rs:70:9
122122
|
123123
LL | match dummy_result() {
124124
| -------------- this expression has type `impl Future<Output = Result<(), ()>>`
@@ -134,7 +134,7 @@ LL | match dummy_result().await {
134134
| ++++++
135135

136136
error[E0308]: mismatched types
137-
--> $DIR/suggest-missing-await.rs:77:27
137+
--> $DIR/suggest-missing-await.rs:78:27
138138
|
139139
LL | Some(do_async()).map(|()| {});
140140
| ^^

Diff for: ‎tests/ui/cast/cast-as-bool.rs

+2
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
//@ dont-require-annotations: SUGGESTION
2+
13
fn main() {
24
let u = 5 as bool; //~ ERROR cannot cast `i32` as `bool`
35
//~| HELP compare with zero instead

Diff for: ‎tests/ui/cast/cast-as-bool.stderr

+11-11
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
error[E0054]: cannot cast `i32` as `bool`
2-
--> $DIR/cast-as-bool.rs:2:13
2+
--> $DIR/cast-as-bool.rs:4:13
33
|
44
LL | let u = 5 as bool;
55
| ^^^^^^^^^
@@ -11,7 +11,7 @@ LL + let u = 5 != 0;
1111
|
1212

1313
error[E0054]: cannot cast `i32` as `bool`
14-
--> $DIR/cast-as-bool.rs:6:13
14+
--> $DIR/cast-as-bool.rs:8:13
1515
|
1616
LL | let t = (1 + 2) as bool;
1717
| ^^^^^^^^^^^^^^^
@@ -23,7 +23,7 @@ LL + let t = (1 + 2) != 0;
2323
|
2424

2525
error[E0054]: cannot cast `u32` as `bool`
26-
--> $DIR/cast-as-bool.rs:10:13
26+
--> $DIR/cast-as-bool.rs:12:13
2727
|
2828
LL | let _ = 5_u32 as bool;
2929
| ^^^^^^^^^^^^^
@@ -35,7 +35,7 @@ LL + let _ = 5_u32 != 0;
3535
|
3636

3737
error[E0054]: cannot cast `f64` as `bool`
38-
--> $DIR/cast-as-bool.rs:13:13
38+
--> $DIR/cast-as-bool.rs:15:13
3939
|
4040
LL | let _ = 64.0_f64 as bool;
4141
| ^^^^^^^^^^^^^^^^
@@ -47,43 +47,43 @@ LL + let _ = 64.0_f64 != 0;
4747
|
4848

4949
error[E0054]: cannot cast `IntEnum` as `bool`
50-
--> $DIR/cast-as-bool.rs:24:13
50+
--> $DIR/cast-as-bool.rs:26:13
5151
|
5252
LL | let _ = IntEnum::One as bool;
5353
| ^^^^^^^^^^^^^^^^^^^^ unsupported cast
5454

5555
error[E0054]: cannot cast `fn(u8) -> String {uwu}` as `bool`
56-
--> $DIR/cast-as-bool.rs:33:13
56+
--> $DIR/cast-as-bool.rs:35:13
5757
|
5858
LL | let _ = uwu as bool;
5959
| ^^^^^^^^^^^ unsupported cast
6060

6161
error[E0054]: cannot cast `unsafe fn() {owo}` as `bool`
62-
--> $DIR/cast-as-bool.rs:35:13
62+
--> $DIR/cast-as-bool.rs:37:13
6363
|
6464
LL | let _ = owo as bool;
6565
| ^^^^^^^^^^^ unsupported cast
6666

6767
error[E0054]: cannot cast `fn(u8) -> String` as `bool`
68-
--> $DIR/cast-as-bool.rs:38:13
68+
--> $DIR/cast-as-bool.rs:40:13
6969
|
7070
LL | let _ = uwu as fn(u8) -> String as bool;
7171
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ unsupported cast
7272

7373
error[E0054]: cannot cast `char` as `bool`
74-
--> $DIR/cast-as-bool.rs:40:13
74+
--> $DIR/cast-as-bool.rs:42:13
7575
|
7676
LL | let _ = 'x' as bool;
7777
| ^^^^^^^^^^^ unsupported cast
7878

7979
error[E0054]: cannot cast `*const ()` as `bool`
80-
--> $DIR/cast-as-bool.rs:44:13
80+
--> $DIR/cast-as-bool.rs:46:13
8181
|
8282
LL | let _ = ptr as bool;
8383
| ^^^^^^^^^^^ unsupported cast
8484

8585
error[E0606]: casting `&'static str` as `bool` is invalid
86-
--> $DIR/cast-as-bool.rs:46:13
86+
--> $DIR/cast-as-bool.rs:48:13
8787
|
8888
LL | let v = "hello" as bool;
8989
| ^^^^^^^^^^^^^^^

0 commit comments

Comments
 (0)