From 51ed1ecefd016bd90b09fea399b9a989d756609d Mon Sep 17 00:00:00 2001 From: Richo Healey Date: Mon, 2 Feb 2015 19:33:50 -0800 Subject: [PATCH 1/3] lint: Deny #[no_mangle] const items This renames the PrivateNoMangleFns lint to allow both to happen in a single pass, since they do roughly the same work. --- src/librustc/lint/builtin.rs | 19 ++++++++++++++++--- src/librustc/lint/context.rs | 2 +- .../compile-fail/lint-unexported-no-mangle.rs | 9 ++++++++- 3 files changed, 25 insertions(+), 5 deletions(-) diff --git a/src/librustc/lint/builtin.rs b/src/librustc/lint/builtin.rs index 904c9c3adb567..fdc2bed781a13 100644 --- a/src/librustc/lint/builtin.rs +++ b/src/librustc/lint/builtin.rs @@ -2065,12 +2065,19 @@ declare_lint! { "functions marked #[no_mangle] should be exported" } +declare_lint! { + NO_MANGLE_CONST_ITEMS, + Deny, + "const items will not have their symbols exported" +} + #[derive(Copy)] -pub struct PrivateNoMangleFns; +pub struct InvalidNoMangleItems; -impl LintPass for PrivateNoMangleFns { +impl LintPass for InvalidNoMangleItems { fn get_lints(&self) -> LintArray { - lint_array!(PRIVATE_NO_MANGLE_FNS) + lint_array!(PRIVATE_NO_MANGLE_FNS, + NO_MANGLE_CONST_ITEMS) } fn check_item(&mut self, cx: &Context, it: &ast::Item) { @@ -2083,6 +2090,12 @@ impl LintPass for PrivateNoMangleFns { cx.span_lint(PRIVATE_NO_MANGLE_FNS, it.span, msg.as_slice()); } }, + ast::ItemConst(..) => { + if attr::contains_name(it.attrs.as_slice(), "no_mangle") { + let msg = "const items should never be #[no_mangle]"; + cx.span_lint(NO_MANGLE_CONST_ITEMS, it.span, msg); + } + } _ => {}, } } diff --git a/src/librustc/lint/context.rs b/src/librustc/lint/context.rs index c649ff2635bf0..730a125fe9724 100644 --- a/src/librustc/lint/context.rs +++ b/src/librustc/lint/context.rs @@ -213,7 +213,7 @@ impl LintStore { UnstableFeatures, Stability, UnconditionalRecursion, - PrivateNoMangleFns, + InvalidNoMangleItems, ); add_builtin_with_new!(sess, diff --git a/src/test/compile-fail/lint-unexported-no-mangle.rs b/src/test/compile-fail/lint-unexported-no-mangle.rs index 3227a78c2ef00..fed1157b1cf1c 100644 --- a/src/test/compile-fail/lint-unexported-no-mangle.rs +++ b/src/test/compile-fail/lint-unexported-no-mangle.rs @@ -8,13 +8,20 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -// compile-flags:-F private_no_mangle_fns +// compile-flags:-F private_no_mangle_fns -F no_mangle_const_items // FIXME(#19495) no_mangle'ing main ICE's. #[no_mangle] fn foo() { //~ ERROR function foo is marked #[no_mangle], but not exported } +#[allow(dead_code)] +#[no_mangle] +const FOO: u64 = 1; //~ ERROR const items should never be #[no_mangle] + +#[no_mangle] +pub const PUB_FOO: u64 = 1; //~ ERROR const items should never be #[no_mangle] + #[no_mangle] pub fn bar() { } From 73d5d89567ef155dc12ee7d7ed61e206e43bf74e Mon Sep 17 00:00:00 2001 From: Richo Healey Date: Mon, 2 Feb 2015 22:03:39 -0800 Subject: [PATCH 2/3] lint: Warn about no-mangled statics that are not exported --- src/librustc/lint/builtin.rs | 15 +++++++++++++++ .../compile-fail/lint-unexported-no-mangle.rs | 10 +++++++++- 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/src/librustc/lint/builtin.rs b/src/librustc/lint/builtin.rs index fdc2bed781a13..697810fa0e95e 100644 --- a/src/librustc/lint/builtin.rs +++ b/src/librustc/lint/builtin.rs @@ -2065,6 +2065,12 @@ declare_lint! { "functions marked #[no_mangle] should be exported" } +declare_lint! { + PRIVATE_NO_MANGLE_STATICS, + Warn, + "statics marked #[no_mangle] should be exported" +} + declare_lint! { NO_MANGLE_CONST_ITEMS, Deny, @@ -2077,6 +2083,7 @@ pub struct InvalidNoMangleItems; impl LintPass for InvalidNoMangleItems { fn get_lints(&self) -> LintArray { lint_array!(PRIVATE_NO_MANGLE_FNS, + PRIVATE_NO_MANGLE_STATICS, NO_MANGLE_CONST_ITEMS) } @@ -2090,6 +2097,14 @@ impl LintPass for InvalidNoMangleItems { cx.span_lint(PRIVATE_NO_MANGLE_FNS, it.span, msg.as_slice()); } }, + ast::ItemStatic(..) => { + if attr::contains_name(it.attrs.as_slice(), "no_mangle") && + !cx.exported_items.contains(&it.id) { + let msg = format!("static {} is marked #[no_mangle], but not exported", + it.ident); + cx.span_lint(PRIVATE_NO_MANGLE_STATICS, it.span, msg.as_slice()); + } + }, ast::ItemConst(..) => { if attr::contains_name(it.attrs.as_slice(), "no_mangle") { let msg = "const items should never be #[no_mangle]"; diff --git a/src/test/compile-fail/lint-unexported-no-mangle.rs b/src/test/compile-fail/lint-unexported-no-mangle.rs index fed1157b1cf1c..216fcf9353578 100644 --- a/src/test/compile-fail/lint-unexported-no-mangle.rs +++ b/src/test/compile-fail/lint-unexported-no-mangle.rs @@ -8,7 +8,7 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -// compile-flags:-F private_no_mangle_fns -F no_mangle_const_items +// compile-flags:-F private_no_mangle_fns -F no_mangle_const_items -F private_no_mangle_statics // FIXME(#19495) no_mangle'ing main ICE's. #[no_mangle] @@ -26,6 +26,14 @@ pub const PUB_FOO: u64 = 1; //~ ERROR const items should never be #[no_mangle] pub fn bar() { } +#[no_mangle] +pub static BAR: u64 = 1; + +#[allow(dead_code)] +#[no_mangle] +static PRIVATE_BAR: u64 = 1; //~ ERROR static PRIVATE_BAR is marked #[no_mangle], but not exported + + fn main() { foo(); bar(); From b6f55efd5b21b2132dafa85c112b339ba498bc7c Mon Sep 17 00:00:00 2001 From: Richo Healey Date: Tue, 3 Feb 2015 12:09:45 -0800 Subject: [PATCH 3/3] lint: Document the why and how of the no_mangle const lint --- src/librustc/lint/builtin.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/librustc/lint/builtin.rs b/src/librustc/lint/builtin.rs index 697810fa0e95e..22f5ca150219b 100644 --- a/src/librustc/lint/builtin.rs +++ b/src/librustc/lint/builtin.rs @@ -2107,7 +2107,10 @@ impl LintPass for InvalidNoMangleItems { }, ast::ItemConst(..) => { if attr::contains_name(it.attrs.as_slice(), "no_mangle") { - let msg = "const items should never be #[no_mangle]"; + // Const items do not refer to a particular location in memory, and therefore + // don't have anything to attach a symbol to + let msg = "const items should never be #[no_mangle], consider instead using \ + `pub static`"; cx.span_lint(NO_MANGLE_CONST_ITEMS, it.span, msg); } }