Dealing with generic code and clippy warnings on macro invocations

help
1

Hi all,

Is there a nice way to work around clippy warning in "generic" code on macros? (by generic here, I mean generic through cfg not through generic types)

#[cfg(feature = "multi-foo")]
type Foo = i32;
#[cfg(feature = "single-foo")]
type Foo = ();

struct Bar {
    /// Makes sure we only operate on "compatible" Bars, identified by this Foo.
    foo: Foo,
    ...
}

fn bar(x: Bar, y: Bar) {
    assert_eq!(x.foo, y.foo);  // triggers clippy::unit_cmp with --features=single-foo
    ...
}

We cannot simply prefix assert_eq with #[allow(clippy::unit_cmp)] because the attribute applies to the macro invocation in the case, while we want it to apply on the expansion.

Here are the current workarounds I know:

// the most obvious workaround, but it's very verbose
fn bar_1(x: Bar, y: Bar) {
    #[allow(clippy::unit_cmp)]
    {
        assert_eq!(x.foo, y.foo);
    }
}
// same idea but shorter although uglier
fn bar_2(x: Bar, y: Bar) {
    #[allow(clippy::unit_cmp)]
    (assert_eq!(x.foo, y.foo));
}
// same idea, but requires hiding another clippy warning
fn bar_3(x: Bar, y: Bar) {
    #[allow(clippy::unit_cmp, clippy::let_unit_value)]
    let () = assert_eq!(x.foo, y.foo);
}
// same idea, but requires hiding another clippy warning
fn bar_4(x: Bar, y: Bar) {
    #[allow(clippy::unit_cmp, clippy::drop_copy)]
    drop(assert_eq!(x.foo, y.foo));
}

The best option seems to be bar_2, but I'm not satisfied with it. Is there any other alternative? What is the most common workaround?

2

If you need to disable a warning in a specific scope, then use a block (solution #1). Solution #2 apparently works but it's weird and definitely non-idiomatic.

3

That's only one workaround to my problem but not my root problem. My root problem is applying an attribute (in particular an allow(clippy::)) on the expansion of a macro (in particular assert_eq). However, if you're implicitly telling me that there's no way to talk about this scope (the expansion of a macro) than by creating a new one around it, then this means the workarounds I suggest above are probably complete (i.e. variations around hiding an expression under a bigger expression/statement which is not a macro invocation). I agree bar_2 is not great but bar_1 is definitely too verbose. I ended up going with bar_3 for now (because it's a common way to document unit return types) but will still monitor this thread in case there are new one-line alternatives.

4

If the clippy::unit_cmp lint is doing more harm than good in your crate, then just add #![allow(clippy::unit_cmp)] to the top of your lib.rs or maybe just the offending module.

The circumstances the lint is checking for are pretty obscure, so I doubt it has actually flagged much in your codebase anyway.

Alternatively, you could define Foo as a struct that may or may not have an i32 field.

#[derive(PartialEq)]
struct Foo(#[feature = "multi-foo"] i32);

That way you can encapsulate the single-foo/multi-foo code inside some abstraction while still getting the desired semantics.

5

I think the lint is useful for non-"generic" code or "generic"-code where all "variants" are units. That's why I'd like to keep the allow-scope as small as possible.

Oh wow that's actually a pretty good idea. I ended up doing a variation of it though:

#[derive(Debug, Copy, Clone, PartialEq, Eq)]  // new
struct Unit;  // new
#[cfg(feature = "multi-foo")]
type Foo = i32;
#[cfg(feature = "single-foo")]
type Foo = Unit;  // updated from ()

The reason I use this variation is because the definition of Foo is actually something like <Type as Trait>::T so I changed the definition of T for the implementation of the trait where it was unit.

6

This topic was automatically closed 90 days after the last reply. We invite you to open a new topic if you have further questions or comments.