From f4e7a99662f710c57b6166418f1c3ac496269c96 Mon Sep 17 00:00:00 2001
From: Ezra Shaw <ezrasure@outlook.com>
Date: Fri, 7 Apr 2023 14:32:55 +1200
Subject: [PATCH 1/2] fix: ensure bad `#[test]` invocs retain correct AST
---
compiler/rustc_builtin_macros/src/test.rs | 62 ++++++++++++-------
tests/ui/test-attrs/issue-109816.rs | 7 +++
tests/ui/test-attrs/issue-109816.stderr | 16 +++++
.../test-attr-non-associated-functions.rs | 6 +-
.../test-attr-non-associated-functions.stderr | 32 +++++-----
tests/ui/test-attrs/test-on-not-fn.stderr | 24 +++----
6 files changed, 95 insertions(+), 52 deletions(-)
create mode 100644 tests/ui/test-attrs/issue-109816.rs
create mode 100644 tests/ui/test-attrs/issue-109816.stderr
diff --git a/compiler/rustc_builtin_macros/src/test.rs b/compiler/rustc_builtin_macros/src/test.rs
index a76ed4ee6ce..9a50999faa0 100644
--- a/compiler/rustc_builtin_macros/src/test.rs
+++ b/compiler/rustc_builtin_macros/src/test.rs
@@ -107,6 +107,36 @@ pub fn expand_test_or_bench(
return vec![];
}
+ let not_testable_error = |item: Option<&ast::Item>| {
+ let diag = &cx.sess.parse_sess.span_diagnostic;
+ let msg = "the `#[test]` attribute may only be used on a non-associated function";
+ let mut err = match item.map(|i| &i.kind) {
+ // These were a warning before #92959 and need to continue being that to avoid breaking
+ // stable user code (#94508).
+ Some(ast::ItemKind::MacCall(_)) => diag.struct_span_warn(attr_sp, msg),
+ // `.forget_guarantee()` needed to get these two arms to match types. Because of how
+ // locally close the `.emit()` call is I'm comfortable with it, but if it can be
+ // reworked in the future to not need it, it'd be nice.
+ _ => diag.struct_span_err(attr_sp, msg).forget_guarantee(),
+ };
+ if let Some(item) = item {
+ err.span_label(
+ item.span,
+ format!(
+ "expected a non-associated function, found {} {}",
+ item.kind.article(),
+ item.kind.descr()
+ ),
+ );
+ }
+ err.span_label(attr_sp, "the `#[test]` macro causes a function to be run as a test and has no effect on non-functions")
+ .span_suggestion(attr_sp,
+ "replace with conditional compilation to make the item only exist when tests are being run",
+ "#[cfg(test)]",
+ Applicability::MaybeIncorrect)
+ .emit();
+ };
+
let (item, is_stmt) = match item {
Annotatable::Item(i) => (i, false),
Annotatable::Stmt(stmt) if matches!(stmt.kind, ast::StmtKind::Item(_)) => {
@@ -118,34 +148,22 @@ pub fn expand_test_or_bench(
}
}
other => {
- cx.struct_span_err(
- other.span(),
- "`#[test]` attribute is only allowed on non associated functions",
- )
- .emit();
+ not_testable_error(None);
return vec![other];
}
};
- // Note: non-associated fn items are already handled by `expand_test_or_bench`
let ast::ItemKind::Fn(fn_) = &item.kind else {
- let diag = &cx.sess.parse_sess.span_diagnostic;
- let msg = "the `#[test]` attribute may only be used on a non-associated function";
- let mut err = match item.kind {
- // These were a warning before #92959 and need to continue being that to avoid breaking
- // stable user code (#94508).
- ast::ItemKind::MacCall(_) => diag.struct_span_warn(attr_sp, msg),
- // `.forget_guarantee()` needed to get these two arms to match types. Because of how
- // locally close the `.emit()` call is I'm comfortable with it, but if it can be
- // reworked in the future to not need it, it'd be nice.
- _ => diag.struct_span_err(attr_sp, msg).forget_guarantee(),
+ not_testable_error(Some(&item));
+ return if is_stmt {
+ vec![Annotatable::Stmt(P(ast::Stmt {
+ id: ast::DUMMY_NODE_ID,
+ span: item.span,
+ kind: ast::StmtKind::Item(item),
+ }))]
+ } else {
+ vec![Annotatable::Item(item)]
};
- err.span_label(attr_sp, "the `#[test]` macro causes a function to be run on a test and has no effect on non-functions")
- .span_label(item.span, format!("expected a non-associated function, found {} {}", item.kind.article(), item.kind.descr()))
- .span_suggestion(attr_sp, "replace with conditional compilation to make the item only exist when tests are being run", "#[cfg(test)]", Applicability::MaybeIncorrect)
- .emit();
-
- return vec![Annotatable::Item(item)];
};
// has_*_signature will report any errors in the type so compilation
diff --git a/tests/ui/test-attrs/issue-109816.rs b/tests/ui/test-attrs/issue-109816.rs
new file mode 100644
index 00000000000..21fe5bc53b7
--- /dev/null
+++ b/tests/ui/test-attrs/issue-109816.rs
@@ -0,0 +1,7 @@
+// compile-flags: --test
+
+fn align_offset_weird_strides() {
+ #[test]
+ //~^ ERROR the `#[test]` attribute may only be used on a non-associated function
+ struct A5(u32, u8);
+}
diff --git a/tests/ui/test-attrs/issue-109816.stderr b/tests/ui/test-attrs/issue-109816.stderr
new file mode 100644
index 00000000000..e6993287555
--- /dev/null
+++ b/tests/ui/test-attrs/issue-109816.stderr
@@ -0,0 +1,16 @@
+error: the `#[test]` attribute may only be used on a non-associated function
+ --> $DIR/issue-109816.rs:4:5
+ |
+LL | #[test]
+ | ^^^^^^^ the `#[test]` macro causes a function to be run as a test and has no effect on non-functions
+LL |
+LL | struct A5(u32, u8);
+ | ------------------- expected a non-associated function, found a struct
+ |
+help: replace with conditional compilation to make the item only exist when tests are being run
+ |
+LL | #[cfg(test)]
+ |
+
+error: aborting due to previous error
+
diff --git a/tests/ui/test-attrs/test-attr-non-associated-functions.rs b/tests/ui/test-attrs/test-attr-non-associated-functions.rs
index 31e567c3960..2481919b616 100644
--- a/tests/ui/test-attrs/test-attr-non-associated-functions.rs
+++ b/tests/ui/test-attrs/test-attr-non-associated-functions.rs
@@ -1,18 +1,16 @@
-// #[test] attribute is not allowed on associated functions or methods
-// reworded error message
// compile-flags:--test
struct A {}
impl A {
#[test]
+ //~^ ERROR the `#[test]` attribute may only be used on a non-associated function
fn new() -> A {
- //~^ ERROR `#[test]` attribute is only allowed on non associated functions
A {}
}
#[test]
+ //~^ ERROR the `#[test]` attribute may only be used on a non-associated function
fn recovery_witness() -> A {
- //~^ ERROR `#[test]` attribute is only allowed on non associated functions
A {}
}
}
diff --git a/tests/ui/test-attrs/test-attr-non-associated-functions.stderr b/tests/ui/test-attrs/test-attr-non-associated-functions.stderr
index a81b8f3980c..3e3a951aff3 100644
--- a/tests/ui/test-attrs/test-attr-non-associated-functions.stderr
+++ b/tests/ui/test-attrs/test-attr-non-associated-functions.stderr
@@ -1,20 +1,24 @@
-error: `#[test]` attribute is only allowed on non associated functions
- --> $DIR/test-attr-non-associated-functions.rs:9:5
+error: the `#[test]` attribute may only be used on a non-associated function
+ --> $DIR/test-attr-non-associated-functions.rs:6:5
+ |
+LL | #[test]
+ | ^^^^^^^ the `#[test]` macro causes a function to be run as a test and has no effect on non-functions
+ |
+help: replace with conditional compilation to make the item only exist when tests are being run
+ |
+LL | #[cfg(test)]
|
-LL | / fn new() -> A {
-LL | |
-LL | | A {}
-LL | | }
- | |_____^
-error: `#[test]` attribute is only allowed on non associated functions
- --> $DIR/test-attr-non-associated-functions.rs:14:5
+error: the `#[test]` attribute may only be used on a non-associated function
+ --> $DIR/test-attr-non-associated-functions.rs:11:5
+ |
+LL | #[test]
+ | ^^^^^^^ the `#[test]` macro causes a function to be run as a test and has no effect on non-functions
+ |
+help: replace with conditional compilation to make the item only exist when tests are being run
+ |
+LL | #[cfg(test)]
|
-LL | / fn recovery_witness() -> A {
-LL | |
-LL | | A {}
-LL | | }
- | |_____^
error: aborting due to 2 previous errors
diff --git a/tests/ui/test-attrs/test-on-not-fn.stderr b/tests/ui/test-attrs/test-on-not-fn.stderr
index fc2c5f62bed..7a9913fbcfa 100644
--- a/tests/ui/test-attrs/test-on-not-fn.stderr
+++ b/tests/ui/test-attrs/test-on-not-fn.stderr
@@ -2,7 +2,7 @@ error: the `#[test]` attribute may only be used on a non-associated function
--> $DIR/test-on-not-fn.rs:3:1
|
LL | #[test]
- | ^^^^^^^ the `#[test]` macro causes a function to be run on a test and has no effect on non-functions
+ | ^^^^^^^ the `#[test]` macro causes a function to be run as a test and has no effect on non-functions
LL | mod test {}
| ----------- expected a non-associated function, found a module
|
@@ -15,7 +15,7 @@ error: the `#[test]` attribute may only be used on a non-associated function
--> $DIR/test-on-not-fn.rs:6:1
|
LL | #[test]
- | ^^^^^^^ the `#[test]` macro causes a function to be run on a test and has no effect on non-functions
+ | ^^^^^^^ the `#[test]` macro causes a function to be run as a test and has no effect on non-functions
LL | / mod loooooooooooooong_teeeeeeeeeest {
LL | | /*
LL | | this is a comment
@@ -34,7 +34,7 @@ error: the `#[test]` attribute may only be used on a non-associated function
--> $DIR/test-on-not-fn.rs:20:1
|
LL | #[test]
- | ^^^^^^^ the `#[test]` macro causes a function to be run on a test and has no effect on non-functions
+ | ^^^^^^^ the `#[test]` macro causes a function to be run as a test and has no effect on non-functions
LL | extern "C" {}
| ------------- expected a non-associated function, found an extern block
|
@@ -47,7 +47,7 @@ error: the `#[test]` attribute may only be used on a non-associated function
--> $DIR/test-on-not-fn.rs:23:1
|
LL | #[test]
- | ^^^^^^^ the `#[test]` macro causes a function to be run on a test and has no effect on non-functions
+ | ^^^^^^^ the `#[test]` macro causes a function to be run as a test and has no effect on non-functions
LL | trait Foo {}
| ------------ expected a non-associated function, found a trait
|
@@ -60,7 +60,7 @@ error: the `#[test]` attribute may only be used on a non-associated function
--> $DIR/test-on-not-fn.rs:26:1
|
LL | #[test]
- | ^^^^^^^ the `#[test]` macro causes a function to be run on a test and has no effect on non-functions
+ | ^^^^^^^ the `#[test]` macro causes a function to be run as a test and has no effect on non-functions
LL | impl Foo for i32 {}
| ------------------- expected a non-associated function, found an implementation
|
@@ -73,7 +73,7 @@ error: the `#[test]` attribute may only be used on a non-associated function
--> $DIR/test-on-not-fn.rs:29:1
|
LL | #[test]
- | ^^^^^^^ the `#[test]` macro causes a function to be run on a test and has no effect on non-functions
+ | ^^^^^^^ the `#[test]` macro causes a function to be run as a test and has no effect on non-functions
LL | const FOO: i32 = -1_i32;
| ------------------------ expected a non-associated function, found a constant item
|
@@ -86,7 +86,7 @@ error: the `#[test]` attribute may only be used on a non-associated function
--> $DIR/test-on-not-fn.rs:32:1
|
LL | #[test]
- | ^^^^^^^ the `#[test]` macro causes a function to be run on a test and has no effect on non-functions
+ | ^^^^^^^ the `#[test]` macro causes a function to be run as a test and has no effect on non-functions
LL | static BAR: u64 = 10_000_u64;
| ----------------------------- expected a non-associated function, found a static item
|
@@ -99,7 +99,7 @@ error: the `#[test]` attribute may only be used on a non-associated function
--> $DIR/test-on-not-fn.rs:35:1
|
LL | #[test]
- | ^^^^^^^ the `#[test]` macro causes a function to be run on a test and has no effect on non-functions
+ | ^^^^^^^ the `#[test]` macro causes a function to be run as a test and has no effect on non-functions
LL | / enum MyUnit {
LL | | Unit,
LL | | }
@@ -114,7 +114,7 @@ error: the `#[test]` attribute may only be used on a non-associated function
--> $DIR/test-on-not-fn.rs:40:1
|
LL | #[test]
- | ^^^^^^^ the `#[test]` macro causes a function to be run on a test and has no effect on non-functions
+ | ^^^^^^^ the `#[test]` macro causes a function to be run as a test and has no effect on non-functions
LL | struct NewI32(i32);
| ------------------- expected a non-associated function, found a struct
|
@@ -127,7 +127,7 @@ error: the `#[test]` attribute may only be used on a non-associated function
--> $DIR/test-on-not-fn.rs:43:1
|
LL | #[test]
- | ^^^^^^^ the `#[test]` macro causes a function to be run on a test and has no effect on non-functions
+ | ^^^^^^^ the `#[test]` macro causes a function to be run as a test and has no effect on non-functions
LL | / union Spooky {
LL | | x: i32,
LL | | y: u32,
@@ -143,7 +143,7 @@ error: the `#[test]` attribute may only be used on a non-associated function
--> $DIR/test-on-not-fn.rs:50:1
|
LL | #[test]
- | ^^^^^^^ the `#[test]` macro causes a function to be run on a test and has no effect on non-functions
+ | ^^^^^^^ the `#[test]` macro causes a function to be run as a test and has no effect on non-functions
LL | #[derive(Copy, Clone, Debug)]
LL | / struct MoreAttrs {
LL | | a: i32,
@@ -160,7 +160,7 @@ warning: the `#[test]` attribute may only be used on a non-associated function
--> $DIR/test-on-not-fn.rs:61:1
|
LL | #[test]
- | ^^^^^^^ the `#[test]` macro causes a function to be run on a test and has no effect on non-functions
+ | ^^^^^^^ the `#[test]` macro causes a function to be run as a test and has no effect on non-functions
LL | foo!();
| ------- expected a non-associated function, found an item macro invocation
|
From 9e70541eacf60f8ed0b38d7b62315cd730a00821 Mon Sep 17 00:00:00 2001
From: Ezra Shaw <ezrasure@outlook.com>
Date: Tue, 11 Apr 2023 21:07:02 +1200
Subject: [PATCH 2/2] refactor: extract `not_testable_error` into function
---
compiler/rustc_builtin_macros/src/test.rs | 64 +++++++++++------------
1 file changed, 32 insertions(+), 32 deletions(-)
diff --git a/compiler/rustc_builtin_macros/src/test.rs b/compiler/rustc_builtin_macros/src/test.rs
index 9a50999faa0..79d8be2484b 100644
--- a/compiler/rustc_builtin_macros/src/test.rs
+++ b/compiler/rustc_builtin_macros/src/test.rs
@@ -107,36 +107,6 @@ pub fn expand_test_or_bench(
return vec![];
}
- let not_testable_error = |item: Option<&ast::Item>| {
- let diag = &cx.sess.parse_sess.span_diagnostic;
- let msg = "the `#[test]` attribute may only be used on a non-associated function";
- let mut err = match item.map(|i| &i.kind) {
- // These were a warning before #92959 and need to continue being that to avoid breaking
- // stable user code (#94508).
- Some(ast::ItemKind::MacCall(_)) => diag.struct_span_warn(attr_sp, msg),
- // `.forget_guarantee()` needed to get these two arms to match types. Because of how
- // locally close the `.emit()` call is I'm comfortable with it, but if it can be
- // reworked in the future to not need it, it'd be nice.
- _ => diag.struct_span_err(attr_sp, msg).forget_guarantee(),
- };
- if let Some(item) = item {
- err.span_label(
- item.span,
- format!(
- "expected a non-associated function, found {} {}",
- item.kind.article(),
- item.kind.descr()
- ),
- );
- }
- err.span_label(attr_sp, "the `#[test]` macro causes a function to be run as a test and has no effect on non-functions")
- .span_suggestion(attr_sp,
- "replace with conditional compilation to make the item only exist when tests are being run",
- "#[cfg(test)]",
- Applicability::MaybeIncorrect)
- .emit();
- };
-
let (item, is_stmt) = match item {
Annotatable::Item(i) => (i, false),
Annotatable::Stmt(stmt) if matches!(stmt.kind, ast::StmtKind::Item(_)) => {
@@ -148,13 +118,13 @@ pub fn expand_test_or_bench(
}
}
other => {
- not_testable_error(None);
+ not_testable_error(cx, attr_sp, None);
return vec![other];
}
};
let ast::ItemKind::Fn(fn_) = &item.kind else {
- not_testable_error(Some(&item));
+ not_testable_error(cx, attr_sp, Some(&item));
return if is_stmt {
vec![Annotatable::Stmt(P(ast::Stmt {
id: ast::DUMMY_NODE_ID,
@@ -416,6 +386,36 @@ pub fn expand_test_or_bench(
}
}
+fn not_testable_error(cx: &ExtCtxt<'_>, attr_sp: Span, item: Option<&ast::Item>) {
+ let diag = &cx.sess.parse_sess.span_diagnostic;
+ let msg = "the `#[test]` attribute may only be used on a non-associated function";
+ let mut err = match item.map(|i| &i.kind) {
+ // These were a warning before #92959 and need to continue being that to avoid breaking
+ // stable user code (#94508).
+ Some(ast::ItemKind::MacCall(_)) => diag.struct_span_warn(attr_sp, msg),
+ // `.forget_guarantee()` needed to get these two arms to match types. Because of how
+ // locally close the `.emit()` call is I'm comfortable with it, but if it can be
+ // reworked in the future to not need it, it'd be nice.
+ _ => diag.struct_span_err(attr_sp, msg).forget_guarantee(),
+ };
+ if let Some(item) = item {
+ err.span_label(
+ item.span,
+ format!(
+ "expected a non-associated function, found {} {}",
+ item.kind.article(),
+ item.kind.descr()
+ ),
+ );
+ }
+ err.span_label(attr_sp, "the `#[test]` macro causes a function to be run as a test and has no effect on non-functions")
+ .span_suggestion(attr_sp,
+ "replace with conditional compilation to make the item only exist when tests are being run",
+ "#[cfg(test)]",
+ Applicability::MaybeIncorrect)
+ .emit();
+}
+
fn get_location_info(cx: &ExtCtxt<'_>, item: &ast::Item) -> (Symbol, usize, usize, usize, usize) {
let span = item.ident.span;
let (source_file, lo_line, lo_col, hi_line, hi_col) =