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

let_unit_value should be allowed in macros #323

Closed
durka opened this issue Sep 10, 2015 · 4 comments · Fixed by #326
Closed

let_unit_value should be allowed in macros #323

durka opened this issue Sep 10, 2015 · 4 comments · Fixed by #326
Assignees
Labels
C-bug Category: Clippy is not doing the correct thing good-first-issue These issues are a good way to get started with Clippy T-AST Type: Requires working with the AST

Comments

@durka
Copy link
Contributor

durka commented Sep 10, 2015

I have the following macro:

#[macro_export]
macro_rules! prof {
    ($b:expr) => { prof!(stringify!($b), $b) };
    //($n:expr, $b:expr) => ($b)
    ($n:expr, $b:expr) => {{
        $crate::PROF.with(|wrapped_prof| {
            let appease_borrowck = wrapped_prof.borrow();
            let g = match *appease_borrowck {
                Some(ref prof) => prof.enter($n),
                None => $crate::hprof::enter($n)
            };
            let ret = { $b }; //~ WARN let_unit_value
            drop(g);
            ret
        })
    }}
}

clippy warns let_unit_value on the commented line when the macro is invoked with a $b which returns ().

@llogiq llogiq added C-bug Category: Clippy is not doing the correct thing good-first-issue These issues are a good way to get started with Clippy T-AST Type: Requires working with the AST labels Sep 10, 2015
@llogiq
Copy link
Contributor

llogiq commented Sep 10, 2015

The in_macro-check seems to fail here. I'm investigating.

@llogiq llogiq self-assigned this Sep 10, 2015
@llogiq
Copy link
Contributor

llogiq commented Sep 10, 2015

Strangely, we need both the check of decl.span and local.pat.span. I'll push a PR shortly.

@durka
Copy link
Contributor Author

durka commented Sep 10, 2015

Yep, the warning is gone!

@llogiq
Copy link
Contributor

llogiq commented Sep 10, 2015

Thanks for the heads-up!

The PR included a test case, so we can be sure it stays that way 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: Clippy is not doing the correct thing good-first-issue These issues are a good way to get started with Clippy T-AST Type: Requires working with the AST
Projects
None yet
2 participants