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

Not using the result of offset_of should lint #111669

Closed
est31 opened this issue May 17, 2023 · 3 comments · Fixed by #111684
Closed

Not using the result of offset_of should lint #111669

est31 opened this issue May 17, 2023 · 3 comments · Fixed by #111684
Assignees
Labels
A-lints Area: Lints (warnings about flaws in source code) such as unused_mut. F-offset_of `#![feature(offset_of)]`

Comments

@est31
Copy link
Member

est31 commented May 17, 2023

The offset_of macro (#106655) is side effect free. For standard library functions that are side effect free, or that have side effects but there is versions of them that don't return anything, it is policy to add a must_use attribute to them so that users are warned if they invoke the function without doing anything with the result (#89692).

Ideally, the same should hold for the offset_of macro as well. One of the lints in the unused lint group should fire if you don't use the result of the offset_of!() macro:

fn foo() {
    offset_of!(F, field); // WARNING result must be used
}
struct F { field: u8, }

Ideally it would be integrated into the must_use system, say via supporting #[must_use] attributes on macros. Currently you are getting:

warning: `#[must_use]` has no effect when applied to a macro def
 --> src/main.rs:1:1
  |
1 | #[must_use = "hi"]
  | ^^^^^^^^^^^^^^^^^^
  |
  = note: `#[warn(unused_attributes)]` on by default

for

#[must_use = "hi"]
macro_rules! foo {
    () => {0};
}
fn main() {
    foo!();
}

It could e.g. expand to some AST node that is just the identity.

Alternatively, one could also just solve it for the offset_of macro via sending the result through a function:

macro_rules! offset_of_must_use {
    () => {
            #[must_use = "offset_of result must be used"]
            const fn must_use(v: usize) -> usize { v }
            must_use(0)
    };
}

fn main() {
    offset_of_must_use!();
}

Sadly this pollutes the context with a function however. If one surrounds the entire expansion with {}s, the lint is silent again.

I don't think it's worth its own lint in itself.

@est31
Copy link
Member Author

est31 commented May 17, 2023

Ohh there is core::hint::must_use (#94745).... seems we can use that one? :)

@est31
Copy link
Member Author

est31 commented May 17, 2023

@rustbot label F-offset_of A-lint

@rustbot rustbot added A-lints Area: Lints (warnings about flaws in source code) such as unused_mut. F-offset_of `#![feature(offset_of)]` labels May 17, 2023
@ChayimFriedman2
Copy link
Contributor

@rustbot claim

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lints Area: Lints (warnings about flaws in source code) such as unused_mut. F-offset_of `#![feature(offset_of)]`
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants