Skip to content

use ErrorGuaranteed instead of booleans in rustc_builtin_macros #112802

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

Merged
merged 1 commit into from
Jun 24, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 9 additions & 5 deletions compiler/rustc_builtin_macros/src/derive.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use rustc_feature::AttributeTemplate;
use rustc_parse::validate_attr;
use rustc_session::Session;
use rustc_span::symbol::{sym, Ident};
use rustc_span::Span;
use rustc_span::{ErrorGuaranteed, Span};

pub(crate) struct Expander(pub bool);

Expand All @@ -22,7 +22,7 @@ impl MultiItemModifier for Expander {
_: bool,
) -> ExpandResult<Vec<Annotatable>, Annotatable> {
let sess = ecx.sess;
if report_bad_target(sess, &item, span) {
if report_bad_target(sess, &item, span).is_err() {
// We don't want to pass inappropriate targets to derive macros to avoid
// follow up errors, all other errors below are recoverable.
return ExpandResult::Ready(vec![item]);
Expand Down Expand Up @@ -103,7 +103,11 @@ fn dummy_annotatable() -> Annotatable {
})
}

fn report_bad_target(sess: &Session, item: &Annotatable, span: Span) -> bool {
fn report_bad_target(
sess: &Session,
item: &Annotatable,
span: Span,
) -> Result<(), ErrorGuaranteed> {
let item_kind = match item {
Annotatable::Item(item) => Some(&item.kind),
Annotatable::Stmt(stmt) => match &stmt.kind {
Expand All @@ -116,9 +120,9 @@ fn report_bad_target(sess: &Session, item: &Annotatable, span: Span) -> bool {
let bad_target =
!matches!(item_kind, Some(ItemKind::Struct(..) | ItemKind::Enum(..) | ItemKind::Union(..)));
if bad_target {
sess.emit_err(errors::BadDeriveTarget { span, item: item.span() });
return Err(sess.emit_err(errors::BadDeriveTarget { span, item: item.span() }));
}
bad_target
Ok(())
}

fn report_unexpected_meta_item_lit(sess: &Session, lit: &ast::MetaItemLit) {
Expand Down
111 changes: 52 additions & 59 deletions compiler/rustc_builtin_macros/src/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use rustc_ast_pretty::pprust;
use rustc_errors::Applicability;
use rustc_expand::base::*;
use rustc_span::symbol::{sym, Ident, Symbol};
use rustc_span::{FileNameDisplayPreference, Span};
use rustc_span::{ErrorGuaranteed, FileNameDisplayPreference, Span};
use std::iter;
use thin_vec::{thin_vec, ThinVec};

Expand Down Expand Up @@ -128,12 +128,15 @@ pub fn expand_test_or_bench(
};
};

// has_*_signature will report any errors in the type so compilation
// check_*_signature will report any errors in the type so compilation
// will fail. We shouldn't try to expand in this case because the errors
// would be spurious.
if (!is_bench && !has_test_signature(cx, &item))
|| (is_bench && !has_bench_signature(cx, &item))
{
let check_result = if is_bench {
check_bench_signature(cx, &item, &fn_)
} else {
check_test_signature(cx, &item, &fn_)
};
if check_result.is_err() {
return if is_stmt {
vec![Annotatable::Stmt(P(cx.stmt_item(item.span, item)))]
} else {
Expand Down Expand Up @@ -523,72 +526,62 @@ fn test_type(cx: &ExtCtxt<'_>) -> TestType {
}
}

fn has_test_signature(cx: &ExtCtxt<'_>, i: &ast::Item) -> bool {
fn check_test_signature(
cx: &ExtCtxt<'_>,
i: &ast::Item,
f: &ast::Fn,
) -> Result<(), ErrorGuaranteed> {
let has_should_panic_attr = attr::contains_name(&i.attrs, sym::should_panic);
let sd = &cx.sess.parse_sess.span_diagnostic;
match &i.kind {
ast::ItemKind::Fn(box ast::Fn { sig, generics, .. }) => {
if let ast::Unsafe::Yes(span) = sig.header.unsafety {
sd.emit_err(errors::TestBadFn { span: i.span, cause: span, kind: "unsafe" });
return false;
}
if let ast::Async::Yes { span, .. } = sig.header.asyncness {
sd.emit_err(errors::TestBadFn { span: i.span, cause: span, kind: "async" });
return false;
}

// If the termination trait is active, the compiler will check that the output
// type implements the `Termination` trait as `libtest` enforces that.
let has_output = match &sig.decl.output {
ast::FnRetTy::Default(..) => false,
ast::FnRetTy::Ty(t) if t.kind.is_unit() => false,
_ => true,
};

if !sig.decl.inputs.is_empty() {
sd.span_err(i.span, "functions used as tests can not have any arguments");
return false;
}
if let ast::Unsafe::Yes(span) = f.sig.header.unsafety {
return Err(sd.emit_err(errors::TestBadFn { span: i.span, cause: span, kind: "unsafe" }));
}

if has_should_panic_attr && has_output {
sd.span_err(i.span, "functions using `#[should_panic]` must return `()`");
return false;
}
if let ast::Async::Yes { span, .. } = f.sig.header.asyncness {
return Err(sd.emit_err(errors::TestBadFn { span: i.span, cause: span, kind: "async" }));
}

if generics.params.iter().any(|param| !matches!(param.kind, GenericParamKind::Lifetime))
{
sd.span_err(
i.span,
"functions used as tests can not have any non-lifetime generic parameters",
);
return false;
}
// If the termination trait is active, the compiler will check that the output
// type implements the `Termination` trait as `libtest` enforces that.
let has_output = match &f.sig.decl.output {
ast::FnRetTy::Default(..) => false,
ast::FnRetTy::Ty(t) if t.kind.is_unit() => false,
_ => true,
};

true
}
_ => {
// should be unreachable because `is_test_fn_item` should catch all non-fn items
debug_assert!(false);
false
}
if !f.sig.decl.inputs.is_empty() {
return Err(sd.span_err(i.span, "functions used as tests can not have any arguments"));
}
}

fn has_bench_signature(cx: &ExtCtxt<'_>, i: &ast::Item) -> bool {
let has_sig = match &i.kind {
// N.B., inadequate check, but we're running
// well before resolve, can't get too deep.
ast::ItemKind::Fn(box ast::Fn { sig, .. }) => sig.decl.inputs.len() == 1,
_ => false,
};
if has_should_panic_attr && has_output {
return Err(sd.span_err(i.span, "functions using `#[should_panic]` must return `()`"));
}

if f.generics.params.iter().any(|param| !matches!(param.kind, GenericParamKind::Lifetime)) {
return Err(sd.span_err(
i.span,
"functions used as tests can not have any non-lifetime generic parameters",
));
}

Ok(())
}

if !has_sig {
cx.sess.parse_sess.span_diagnostic.span_err(
fn check_bench_signature(
cx: &ExtCtxt<'_>,
i: &ast::Item,
f: &ast::Fn,
) -> Result<(), ErrorGuaranteed> {
// N.B., inadequate check, but we're running
// well before resolve, can't get too deep.
if f.sig.decl.inputs.len() != 1 {
return Err(cx.sess.parse_sess.span_diagnostic.span_err(
i.span,
"functions used as benches must have \
signature `fn(&mut Bencher) -> impl Termination`",
);
));
}

has_sig
Ok(())
}