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

Stronger macro check for identity-op and type-complexity #301

Closed
llogiq opened this issue Sep 6, 2015 · 3 comments
Closed

Stronger macro check for identity-op and type-complexity #301

llogiq opened this issue Sep 6, 2015 · 3 comments
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 S-needs-discussion Status: Needs further discussion before merging or work can be started T-middle Type: Probably requires verifiying types

Comments

@llogiq
Copy link
Contributor

llogiq commented Sep 6, 2015

Macros are both used to make types easier to read and to create basic operations, sometimes relying on the compiler to remove identity operations so the macro is easier. Therefore I have written a new utility method in_macro that is more strict than in_external_macro, on the grounds that we want to rule out all macro expansion for some lints, e.g. the ones above (I'll make a PR shortly once my current rust build finishes).

In the meantime I want to discuss which other lints should rule out not only internal but external macros? I share @Manishearth's uneasiness about false positives (not only) resulting from macro expansion, however, we should not throw out the baby with the bathwater.

@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-middle Type: Probably requires verifiying types S-needs-discussion Status: Needs further discussion before merging or work can be started labels Sep 6, 2015
@llogiq llogiq self-assigned this Sep 6, 2015
@llogiq
Copy link
Contributor Author

llogiq commented Sep 6, 2015

Ok, so we have #308 unit_cmp. What else?

@llogiq
Copy link
Contributor Author

llogiq commented Sep 10, 2015

With #323 closed we have let_unit_value – this is an interesting case, because it required not one, but two macro checks (one for the declaration that appears to catch compiler plugins like regex, and one on the pattern, which catches MacroBangs in the test.

I also have a helper function I use to check on the expansion info:

fn note_macro(cx: &Context, span: Span) {
    let code = snippet(cx, span, "..");
    cx.sess().note(&cx.sess().codemap().with_expn_info(span.expn_id,
        |info| info.map_or(format!("{}: no info", code),
            |i| format!("{}: {:?}", code, i.callee.format))
    ));
}

I'm not including it in the code, because it doesn't belong there, but it's certainly helpful to find out why macro checks fail to catch something.

@phansch
Copy link
Member

phansch commented Dec 18, 2020

Going to close this 5 year old issue mostly because there wasn't a MCVE at the time and we probably have more specific issues for those cases now.

@phansch phansch closed this as completed Dec 18, 2020
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 S-needs-discussion Status: Needs further discussion before merging or work can be started T-middle Type: Probably requires verifiying types
Projects
None yet
Development

No branches or pull requests

3 participants