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

mut_mut triggers for regex! macro #67

Closed
killercup opened this issue May 24, 2015 · 9 comments
Closed

mut_mut triggers for regex! macro #67

killercup opened this issue May 24, 2015 · 9 comments
Assignees
Labels
C-bug Category: Clippy is not doing the correct thing

Comments

@killercup
Copy link
Member

I just came across this warning:

src/helpers/adjust_header_level.rs:9:28: 9:71 warning: This expression mutably borrows a mutable reference. Consider reborrowing, #[warn(mut_mut)] on by default
src/helpers/adjust_header_level.rs:9     let headline_pattern = regex!(r"^(?P<level>[#]+)\s(?P<title>.+)$");
                                                                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
src/main.rs:1:1: 24:1 note: in expansion of regex!

This does not seem very useful. (I totally understand why this can be problematic to fix in clippy, though.)

@llogiq
Copy link
Contributor

llogiq commented May 24, 2015

I think it may not be too problematic and in fact makes a lot of sense: We should watch out for macro expansion and (perhaps optionally) if the macro is external to the current crate, omit the warning.

@Manishearth
Copy link
Member

That sounds good.

We can pick up the syntaxcontext of idents and stuff I think. If an ident is not doc.rust-lang.org/syntax/ast/constant.EMPTY_CTXT.html, we ignore it. Not sure though.

@llogiq
Copy link
Contributor

llogiq commented May 24, 2015

We may want to look up if the macro is defined in the current crate, and issue a warning in that case anyway.

@Manishearth
Copy link
Member

I'm not sure if that info is kept around post-expansion in an easy-to-access manner.

The spans might help here.

@llogiq
Copy link
Contributor

llogiq commented May 25, 2015

I'm curious how the compiler does it when reporting errors – it always tells me which macro was expanded.

Looking into the code, the code map has a function we may be able to use. I'll push an untested prototype into a 'macro_expn' branch. However, I'm not sure if we should include regex! as a test dependency or if there is another alternative.

@llogiq
Copy link
Contributor

llogiq commented May 25, 2015

@killercup: Are you ok with me reusing your line of code as a test case? I may be able to reduce it, but it's the best starting point we have.

@killercup
Copy link
Member Author

@llogiq Sure, you can reproduce about a gazillion lines of this when you compile trpl-ebook with cargo run --features "dev" --release :)

@llogiq llogiq added the C-bug Category: Clippy is not doing the correct thing label May 26, 2015
@llogiq llogiq self-assigned this May 26, 2015
@llogiq
Copy link
Contributor

llogiq commented May 26, 2015

In PR #68 I have added a better in_macro check that we should be able to reuse for other lints (btw. we should even be able to create a basic check_expr(…) to closure adapter that will only trigger when not within an external macro expansion.

Note that this enables you to write e.g. let mut zero = &mut &mut "zero"; within a macro invocation without the lint triggering, but I'm ok with paying that price for getting rid of the false positives.

@llogiq
Copy link
Contributor

llogiq commented Jun 1, 2015

The latest merged PR #82 makes this issue go away (as well as similar issues in other lints)

@llogiq llogiq closed this as completed Jun 1, 2015
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
Projects
None yet
Development

No branches or pull requests

3 participants