Skip to content

Commit

Permalink
Rollup merge of rust-lang#128581 - jieyouxu:checked-attr, r=nnethercote
Browse files Browse the repository at this point in the history
Assert that all attributes are actually checked via `CheckAttrVisitor` and aren't accidentally usable on completely unrelated HIR nodes

`@oli-obk's` rust-lang#128444 with unreachable case removed to avoid that PR bitrotting away.
Based on rust-lang#128402.

This PR will make adding a new attribute ICE on any use of that attribute unless it gets a handler added in `rustc_passes::CheckAttrVisitor`.

r? `@nnethercote` (since you were the reviewer of the original PR)
  • Loading branch information
tgross35 authored Aug 3, 2024
2 parents a8e58c3 + 33cb334 commit 6b3160e
Show file tree
Hide file tree
Showing 6 changed files with 104 additions and 6 deletions.
8 changes: 4 additions & 4 deletions compiler/rustc_feature/src/builtin_attrs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -703,21 +703,21 @@ pub const BUILTIN_ATTRIBUTES: &[BuiltinAttribute] = &[
EncodeCrossCrate::No, allocator_internals, experimental!(needs_allocator),
),
gated!(
panic_runtime, Normal, template!(Word), WarnFollowing,
panic_runtime, CrateLevel, template!(Word), WarnFollowing,
EncodeCrossCrate::No, experimental!(panic_runtime)
),
gated!(
needs_panic_runtime, Normal, template!(Word), WarnFollowing,
needs_panic_runtime, CrateLevel, template!(Word), WarnFollowing,
EncodeCrossCrate::No, experimental!(needs_panic_runtime)
),
gated!(
compiler_builtins, Normal, template!(Word), WarnFollowing,
compiler_builtins, CrateLevel, template!(Word), WarnFollowing,
EncodeCrossCrate::No,
"the `#[compiler_builtins]` attribute is used to identify the `compiler_builtins` crate \
which contains compiler-rt intrinsics and will never be stable",
),
gated!(
profiler_runtime, Normal, template!(Word), WarnFollowing,
profiler_runtime, CrateLevel, template!(Word), WarnFollowing,
EncodeCrossCrate::No,
"the `#[profiler_runtime]` attribute is used to identify the `profiler_builtins` crate \
which contains the profiler runtime and will never be stable",
Expand Down
4 changes: 4 additions & 0 deletions compiler/rustc_passes/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,10 @@ passes_continue_labeled_block =
.label = labeled blocks cannot be `continue`'d
.block_label = labeled block the `continue` points to
passes_coroutine_on_non_closure =
attribute should be applied to closures
.label = not a closure
passes_coverage_not_fn_or_closure =
attribute should be applied to a function definition or closure
.label = not a function or closure
Expand Down
66 changes: 64 additions & 2 deletions compiler/rustc_passes/src/check_attr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,13 @@ use rustc_hir::{
TraitItem, CRATE_HIR_ID, CRATE_OWNER_ID,
};
use rustc_macros::LintDiagnostic;
use rustc_middle::bug;
use rustc_middle::hir::nested_filter;
use rustc_middle::middle::resolve_bound_vars::ObjectLifetimeDefault;
use rustc_middle::query::Providers;
use rustc_middle::traits::ObligationCause;
use rustc_middle::ty::error::{ExpectedFound, TypeError};
use rustc_middle::ty::{self, TyCtxt};
use rustc_middle::{bug, span_bug};
use rustc_session::lint::builtin::{
CONFLICTING_REPR_HINTS, INVALID_DOC_ATTRIBUTES, INVALID_MACRO_EXPORT_ARGUMENTS,
UNKNOWN_OR_MALFORMED_DIAGNOSTIC_ATTRIBUTES, UNUSED_ATTRIBUTES,
Expand Down Expand Up @@ -239,7 +239,59 @@ impl<'tcx> CheckAttrVisitor<'tcx> {
self.check_generic_attr(hir_id, attr, target, Target::Fn);
self.check_proc_macro(hir_id, target, ProcMacroKind::Derive)
}
_ => {}
[sym::coroutine] => {
self.check_coroutine(attr, target);
}
[
// ok
sym::allow
| sym::expect
| sym::warn
| sym::deny
| sym::forbid
| sym::cfg
// need to be fixed
| sym::cfi_encoding // FIXME(cfi_encoding)
| sym::may_dangle // FIXME(dropck_eyepatch)
| sym::pointee // FIXME(derive_smart_pointer)
| sym::linkage // FIXME(linkage)
| sym::no_sanitize // FIXME(no_sanitize)
| sym::omit_gdb_pretty_printer_section // FIXME(omit_gdb_pretty_printer_section)
| sym::used // handled elsewhere to restrict to static items
| sym::repr // handled elsewhere to restrict to type decls items
| sym::instruction_set // broken on stable!!!
| sym::windows_subsystem // broken on stable!!!
| sym::patchable_function_entry // FIXME(patchable_function_entry)
| sym::deprecated_safe // FIXME(deprecated_safe)
// internal
| sym::prelude_import
| sym::panic_handler
| sym::allow_internal_unsafe
| sym::fundamental
| sym::lang
| sym::needs_allocator
| sym::default_lib_allocator
| sym::start
| sym::custom_mir,
] => {}
[name, ..] => {
match BUILTIN_ATTRIBUTE_MAP.get(name) {
// checked below
Some(BuiltinAttribute { type_: AttributeType::CrateLevel, .. }) => {}
Some(_) => {
// FIXME: differentiate between unstable and internal attributes just like we do with features instead
// of just accepting `rustc_` attributes by name. That should allow trimming the above list, too.
if !name.as_str().starts_with("rustc_") {
span_bug!(
attr.span,
"builtin attribute {name:?} not handled by `CheckAttrVisitor`"
)
}
}
None => (),
}
}
[] => unreachable!(),
}

let builtin = attr.ident().and_then(|ident| BUILTIN_ATTRIBUTE_MAP.get(&ident.name));
Expand Down Expand Up @@ -376,6 +428,7 @@ impl<'tcx> CheckAttrVisitor<'tcx> {

/// Checks that `#[optimize(..)]` is applied to a function/closure/method,
/// or to an impl block or module.
// FIXME(#128488): this should probably be elevated to an error?
fn check_optimize(&self, hir_id: HirId, attr: &Attribute, target: Target) {
match target {
Target::Fn
Expand Down Expand Up @@ -2279,6 +2332,15 @@ impl<'tcx> CheckAttrVisitor<'tcx> {
self.abort.set(true);
}
}

fn check_coroutine(&self, attr: &Attribute, target: Target) {
match target {
Target::Closure => return,
_ => {
self.dcx().emit_err(errors::CoroutineOnNonClosure { span: attr.span });
}
}
}
}

impl<'tcx> Visitor<'tcx> for CheckAttrVisitor<'tcx> {
Expand Down
7 changes: 7 additions & 0 deletions compiler/rustc_passes/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -636,6 +636,13 @@ pub struct Confusables {
pub attr_span: Span,
}

#[derive(Diagnostic)]
#[diag(passes_coroutine_on_non_closure)]
pub struct CoroutineOnNonClosure {
#[primary_span]
pub span: Span,
}

#[derive(Diagnostic)]
#[diag(passes_empty_confusables)]
pub(crate) struct EmptyConfusables {
Expand Down
11 changes: 11 additions & 0 deletions tests/ui/coroutine/invalid_attr_usage.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
//! The `coroutine` attribute is only allowed on closures.

#![feature(coroutines)]

#[coroutine]
//~^ ERROR: attribute should be applied to closures
struct Foo;

#[coroutine]
//~^ ERROR: attribute should be applied to closures
fn main() {}
14 changes: 14 additions & 0 deletions tests/ui/coroutine/invalid_attr_usage.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
error: attribute should be applied to closures
--> $DIR/invalid_attr_usage.rs:5:1
|
LL | #[coroutine]
| ^^^^^^^^^^^^

error: attribute should be applied to closures
--> $DIR/invalid_attr_usage.rs:9:1
|
LL | #[coroutine]
| ^^^^^^^^^^^^

error: aborting due to 2 previous errors

0 comments on commit 6b3160e

Please sign in to comment.