Skip to content
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

Assert that all attributes are actually checked via CheckAttrVisitor and aren't accidentally usable on completely unrelated HIR nodes #128581

Merged
merged 2 commits into from
Aug 3, 2024
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
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?
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added this FIXME for #128488, but elevating this to error may require a RFC tweak.

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

Loading