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

Validate built-in attribute placement #73461

Merged
merged 6 commits into from
Sep 12, 2020
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
197 changes: 183 additions & 14 deletions compiler/rustc_passes/src/check_attr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,12 +66,26 @@ impl CheckAttrVisitor<'tcx> {
} else if self.tcx.sess.check_name(attr, sym::marker) {
self.check_marker(attr, span, target)
} else if self.tcx.sess.check_name(attr, sym::target_feature) {
self.check_target_feature(attr, span, target)
self.check_target_feature(hir_id, attr, span, target)
} else if self.tcx.sess.check_name(attr, sym::track_caller) {
self.check_track_caller(&attr.span, attrs, span, target)
} else if self.tcx.sess.check_name(attr, sym::doc) {
self.check_doc_alias(attr, hir_id, target)
} else if self.tcx.sess.check_name(attr, sym::no_link) {
self.check_no_link(&attr, span, target)
} else if self.tcx.sess.check_name(attr, sym::export_name) {
self.check_export_name(&attr, span, target)
} else {
// lint-only checks
if self.tcx.sess.check_name(attr, sym::cold) {
self.check_cold(hir_id, attr, span, target);
} else if self.tcx.sess.check_name(attr, sym::link_name) {
self.check_link_name(hir_id, attr, span, target);
} else if self.tcx.sess.check_name(attr, sym::link_section) {
self.check_link_section(hir_id, attr, span, target);
} else if self.tcx.sess.check_name(attr, sym::no_mangle) {
self.check_no_mangle(hir_id, attr, span, target);
}
true
};
}
Expand Down Expand Up @@ -109,12 +123,12 @@ impl CheckAttrVisitor<'tcx> {
lint.build("`#[inline]` is ignored on constants")
.warn(
"this was previously accepted by the compiler but is \
being phased out; it will become a hard error in \
a future release!",
being phased out; it will become a hard error in \
a future release!",
)
.note(
"see issue #65833 <https://github.com/rust-lang/rust/issues/65833> \
for more information",
for more information",
)
.emit();
});
Expand Down Expand Up @@ -153,7 +167,7 @@ impl CheckAttrVisitor<'tcx> {
.emit();
false
}
Target::Fn | Target::Method(..) | Target::ForeignFn => true,
Target::Fn | Target::Method(..) | Target::ForeignFn | Target::Closure => true,
_ => {
struct_span_err!(
self.tcx.sess,
Expand Down Expand Up @@ -202,10 +216,31 @@ impl CheckAttrVisitor<'tcx> {
}

/// Checks if the `#[target_feature]` attribute on `item` is valid. Returns `true` if valid.
fn check_target_feature(&self, attr: &Attribute, span: &Span, target: Target) -> bool {
fn check_target_feature(
&self,
hir_id: HirId,
attr: &Attribute,
span: &Span,
target: Target,
) -> bool {
match target {
Target::Fn
| Target::Method(MethodKind::Trait { body: true } | MethodKind::Inherent) => true,
// FIXME: #[target_feature] was previously erroneously allowed on statements and some
// crates used this, so only emit a warning.
Target::Statement => {
self.tcx.struct_span_lint_hir(UNUSED_ATTRIBUTES, hir_id, attr.span, |lint| {
lint.build("attribute should be applied to a function")
.warn(
"this was previously accepted by the compiler but is \
being phased out; it will become a hard error in \
a future release!",
)
.span_label(*span, "not a function")
.emit();
});
true
}
_ => {
self.tcx
.sess
Expand Down Expand Up @@ -277,6 +312,136 @@ impl CheckAttrVisitor<'tcx> {
true
}

/// Checks if `#[cold]` is applied to a non-function. Returns `true` if valid.
fn check_cold(&self, hir_id: HirId, attr: &Attribute, span: &Span, target: Target) {
match target {
Target::Fn | Target::Method(..) | Target::ForeignFn | Target::Closure => {}
_ => {
// FIXME: #[cold] was previously allowed on non-functions and some crates used
// this, so only emit a warning.
self.tcx.struct_span_lint_hir(UNUSED_ATTRIBUTES, hir_id, attr.span, |lint| {
lint.build("attribute should be applied to a function")
.warn(
"this was previously accepted by the compiler but is \
being phased out; it will become a hard error in \
a future release!",
)
.span_label(*span, "not a function")
.emit();
});
}
}
}

/// Checks if `#[link_name]` is applied to an item other than a foreign function or static.
fn check_link_name(&self, hir_id: HirId, attr: &Attribute, span: &Span, target: Target) {
match target {
Target::ForeignFn | Target::ForeignStatic => {}
_ => {
// FIXME: #[cold] was previously allowed on non-functions/statics and some crates
// used this, so only emit a warning.
self.tcx.struct_span_lint_hir(UNUSED_ATTRIBUTES, hir_id, attr.span, |lint| {
let mut diag =
lint.build("attribute should be applied to a foreign function or static");
diag.warn(
"this was previously accepted by the compiler but is \
being phased out; it will become a hard error in \
a future release!",
);

// See issue #47725
if let Target::ForeignMod = target {
if let Some(value) = attr.value_str() {
diag.span_help(
attr.span,
&format!(r#"try `#[link(name = "{}")]` instead"#, value),
);
} else {
diag.span_help(attr.span, r#"try `#[link(name = "...")]` instead"#);
}
}

diag.span_label(*span, "not a foreign function or static");
diag.emit();
});
}
}
}

/// Checks if `#[no_link]` is applied to an `extern crate`. Returns `true` if valid.
fn check_no_link(&self, attr: &Attribute, span: &Span, target: Target) -> bool {
if target == Target::ExternCrate {
true
} else {
self.tcx
.sess
.struct_span_err(attr.span, "attribute should be applied to an `extern crate` item")
.span_label(*span, "not an `extern crate` item")
.emit();
false
}
}

/// Checks if `#[export_name]` is applied to a function or static. Returns `true` if valid.
fn check_export_name(&self, attr: &Attribute, span: &Span, target: Target) -> bool {
match target {
Target::Static | Target::Fn | Target::Method(..) => true,
_ => {
self.tcx
.sess
.struct_span_err(
attr.span,
"attribute should be applied to a function or static",
)
.span_label(*span, "not a function or static")
.emit();
false
}
}
}

/// Checks if `#[link_section]` is applied to a function or static.
fn check_link_section(&self, hir_id: HirId, attr: &Attribute, span: &Span, target: Target) {
match target {
Target::Static | Target::Fn | Target::Method(..) => {}
_ => {
// FIXME: #[link_section] was previously allowed on non-functions/statics and some
// crates used this, so only emit a warning.
self.tcx.struct_span_lint_hir(UNUSED_ATTRIBUTES, hir_id, attr.span, |lint| {
lint.build("attribute should be applied to a function or static")
.warn(
"this was previously accepted by the compiler but is \
being phased out; it will become a hard error in \
a future release!",
)
.span_label(*span, "not a function or static")
.emit();
});
}
}
}

/// Checks if `#[no_mangle]` is applied to a function or static.
fn check_no_mangle(&self, hir_id: HirId, attr: &Attribute, span: &Span, target: Target) {
match target {
Target::Static | Target::Fn | Target::Method(..) => {}
_ => {
// FIXME: #[no_mangle] was previously allowed on non-functions/statics and some
// crates used this, so only emit a warning.
self.tcx.struct_span_lint_hir(UNUSED_ATTRIBUTES, hir_id, attr.span, |lint| {
lint.build("attribute should be applied to a function or static")
.warn(
"this was previously accepted by the compiler but is \
being phased out; it will become a hard error in \
a future release!",
)
.span_label(*span, "not a function or static")
.emit();
});
}
}
}

/// Checks if the `#[repr]` attributes on `item` are valid.
fn check_repr(
&self,
Expand Down Expand Up @@ -321,7 +486,11 @@ impl CheckAttrVisitor<'tcx> {
}
sym::simd => {
is_simd = true;
if target != Target::Struct { ("a", "struct") } else { continue }
if target != Target::Struct {
("a", "struct")
} else {
continue;
}
}
sym::transparent => {
is_transparent = true;
Expand Down Expand Up @@ -358,7 +527,11 @@ impl CheckAttrVisitor<'tcx> {
| sym::isize
| sym::usize => {
int_reprs += 1;
if target != Target::Enum { ("an", "enum") } else { continue }
if target != Target::Enum {
("an", "enum")
} else {
continue;
}
}
_ => continue,
};
Expand Down Expand Up @@ -421,10 +594,8 @@ impl CheckAttrVisitor<'tcx> {
fn check_stmt_attributes(&self, stmt: &hir::Stmt<'_>) {
// When checking statements ignore expressions, they will be checked later
if let hir::StmtKind::Local(ref l) = stmt.kind {
self.check_attributes(l.hir_id, &l.attrs, &stmt.span, Target::Statement, None);
for attr in l.attrs.iter() {
if self.tcx.sess.check_name(attr, sym::inline) {
self.check_inline(l.hir_id, attr, &stmt.span, Target::Statement);
}
if self.tcx.sess.check_name(attr, sym::repr) {
self.emit_repr_error(
attr.span,
Expand All @@ -442,10 +613,8 @@ impl CheckAttrVisitor<'tcx> {
hir::ExprKind::Closure(..) => Target::Closure,
_ => Target::Expression,
};
self.check_attributes(expr.hir_id, &expr.attrs, &expr.span, target, None);
for attr in expr.attrs.iter() {
if self.tcx.sess.check_name(attr, sym::inline) {
self.check_inline(expr.hir_id, attr, &expr.span, target);
}
if self.tcx.sess.check_name(attr, sym::repr) {
self.emit_repr_error(
attr.span,
Expand Down
30 changes: 11 additions & 19 deletions compiler/rustc_typeck/src/collect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2490,10 +2490,17 @@ fn codegen_fn_attrs(tcx: TyCtxt<'_>, id: DefId) -> CodegenFnAttrs {
codegen_fn_attrs.export_name = Some(s);
}
} else if tcx.sess.check_name(attr, sym::target_feature) {
if !tcx.features().target_feature_11 {
check_target_feature_safe_fn(tcx, id, attr.span);
} else if let Some(local_id) = id.as_local() {
if tcx.fn_sig(id).unsafety() == hir::Unsafety::Normal {
if !tcx.is_closure(id) && tcx.fn_sig(id).unsafety() == hir::Unsafety::Normal {
if !tcx.features().target_feature_11 {
let mut err = feature_err(
&tcx.sess.parse_sess,
sym::target_feature_11,
attr.span,
"`#[target_feature(..)]` can only be applied to `unsafe` functions",
);
err.span_label(tcx.def_span(id), "not an `unsafe` function");
err.emit();
} else if let Some(local_id) = id.as_local() {
check_target_feature_trait_unsafe(tcx, local_id, attr.span);
}
}
Expand Down Expand Up @@ -2750,21 +2757,6 @@ fn check_link_name_xor_ordinal(
}
}

/// Checks the function annotated with `#[target_feature]` is unsafe,
/// reporting an error if it isn't.
fn check_target_feature_safe_fn(tcx: TyCtxt<'_>, id: DefId, attr_span: Span) {
if tcx.is_closure(id) || tcx.fn_sig(id).unsafety() == hir::Unsafety::Normal {
let mut err = feature_err(
&tcx.sess.parse_sess,
sym::target_feature_11,
attr_span,
"`#[target_feature(..)]` can only be applied to `unsafe` functions",
);
err.span_label(tcx.def_span(id), "not an `unsafe` function");
err.emit();
}
}

/// Checks the function annotated with `#[target_feature]` is not a safe
/// trait method implementation, reporting an error if it is.
fn check_target_feature_trait_unsafe(tcx: TyCtxt<'_>, id: LocalDefId, attr_span: Span) {
Expand Down
5 changes: 1 addition & 4 deletions src/test/ui/check-static-recursion-foreign.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
// run-pass

#![allow(dead_code)]
// Static recursion check shouldn't fail when given a foreign item (#18279)

// aux-build:check_static_recursion_foreign_helper.rs
Expand All @@ -15,12 +14,10 @@ extern crate libc;

use libc::c_int;

#[link_name = "check_static_recursion_foreign_helper"]
extern "C" {
calebzulawski marked this conversation as resolved.
Show resolved Hide resolved
#[allow(dead_code)]
static test_static: c_int;
}

static B: &'static c_int = unsafe { &test_static };
pub static B: &'static c_int = unsafe { &test_static };

pub fn main() {}
Loading