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

#[deny] in expansion of proc macro is ignored #53975

Closed
japaric opened this issue Sep 5, 2018 · 2 comments
Closed

#[deny] in expansion of proc macro is ignored #53975

japaric opened this issue Sep 5, 2018 · 2 comments
Labels
A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. A-macros-1.2 Area: Declarative macros 1.2

Comments

@japaric
Copy link
Member

japaric commented Sep 5, 2018

Affects proc_macro_bang! and #[proc_macro_attr]

STR

// crate: bar
// proc-macro = true
extern crate proc_macro;
#[macro_use]
extern crate quote;

use proc_macro::TokenStream;

#[proc_macro_attribute]
pub fn bar(_: TokenStream, _: TokenStream) -> TokenStream {
    quote!(
        #[deny(dead_code)]
        fn bar() {}
    ).into()
}

#[proc_macro]
pub fn baz(_: TokenStream) -> TokenStream {
    quote!(
        #[deny(dead_code)]
        fn baz() {}
    ).into()
}
// crate: baz
// crate-type: rlib

extern crate bar;

use bar::{bar, baz};

#[bar]
const FOO: () = ();

baz!();

Compiling this code succeeds

$ cargo build && echo OK
OK
$ cargo expand | tail -n9
extern crate bar;

use bar::{bar, baz};

#[deny(dead_code)]
fn bar() {}

#[deny(dead_code)]
fn baz() {}

Compiling the expanded code results in two errors, as expected:

$ cargo build
cargo build
   Compiling foo v0.1.0 (file:///home/japaric/tmp/foo)
warning: unused imports: `bar`, `baz`
 --> src/lib.rs:3:11
  |
3 | use bar::{bar, baz};
  |           ^^^  ^^^
  |
  = note: #[warn(unused_imports)] on by default

error: function is never used: `bar`
 --> src/lib.rs:6:1
  |
6 | fn bar() {}
  | ^^^^^^^^
  |
note: lint level defined here
 --> src/lib.rs:5:8
  |
5 | #[deny(dead_code)]
  |        ^^^^^^^^^

error: function is never used: `baz`
 --> src/lib.rs:9:1
  |
9 | fn baz() {}
  | ^^^^^^^^
  |
note: lint level defined here
 --> src/lib.rs:8:8
  |
8 | #[deny(dead_code)]
  |        ^^^^^^^^^

error: aborting due to 2 previous errors

Meta

$ rustc -V
rustc 1.30.0-nightly (1c2e17f4e 2018-09-04)

The name of the expanded function is not a problem. Renaming them to e.g. foo and quux doesn't change the outcome.

cc @alexcrichton @petrochenkov apologies if this has already been reported. I didn't search too hard.

@Havvy Havvy added A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. A-macros-1.2 Area: Declarative macros 1.2 labels Sep 5, 2018
@alexcrichton
Copy link
Member

This phenomena is currently intentional and a relatively recent change, specifically this code --

// If this code originates in a foreign macro, aka something that this crate
// did not itself author, then it's likely that there's nothing this crate
// can do about it. We probably want to skip the lint entirely.
if err.span.primary_spans().iter().any(|s| in_external_macro(sess, *s)) {
// Any suggestions made here are likely to be incorrect, so anything we
// emit shouldn't be automatically fixed by rustfix.
err.allow_suggestions(false);
// If this is a future incompatible lint it'll become a hard error, so
// we have to emit *something*. Also allow lints to whitelist themselves
// on a case-by-case basis for emission in a foreign macro.
if future_incompatible.is_none() && !lint.report_in_external_macro {
err.cancel()
}
}

which was updated in #52467

@japaric
Copy link
Member Author

japaric commented Sep 5, 2018

Interesting.

On one hand, I like this change because it means I no longer have to sprinkle #[allow(unsafe_code)] all over the expansion of my proc macros so that they won't break builds where the user is using #![deny(unsafe_code)].

On the other hand, I was relying on this to prevent a user error. I have this attribute that turns a function into #[export_name = ".."] pub fn name(.. and was using #[deny(dead_code)] to force the user to make the function reachable. If the function is not reachable the symbol won't be external or worst, it could be dropped by the compiler; this would cause linking to fail, or worst linking could succeed but the user function would be ignored. I have written this requirement in the docs but wanted a compile time check because some people don't (carefully) read the API docs.

Ideally, I would be using some feature to make a symbol external regardless of whether is pub or reachable or neither and this wouldn't be a problem but such feature doesn't exist AFAIK.

Oh well, I'll write the requirement in bold and just deal with the future "why isn't this working?" questions. Thanks for the info.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. A-macros-1.2 Area: Declarative macros 1.2
Projects
None yet
Development

No branches or pull requests

3 participants