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

Tracking issue for core::hint::must_use #94745

Open
1 of 5 tasks
dtolnay opened this issue Mar 8, 2022 · 12 comments
Open
1 of 5 tasks

Tracking issue for core::hint::must_use #94745

dtolnay opened this issue Mar 8, 2022 · 12 comments
Labels
C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@dtolnay
Copy link
Member

dtolnay commented Mar 8, 2022

Feature gate: #![feature(hint_must_use)]

This is a tracking issue for the function core::hint::must_use

Public API

// core::hint

#[must_use]
pub const fn must_use<T>(value: T) -> T {
    value
}

Steps / History

Unresolved Questions

  • Alternative: #[must_use] on an arbitrary block or expression?

    #[must_use]
    {
        some_expr
    }

    This feels more native, and might behave slightly better in some subtle edge cases—type inference may not always 'see through' the identity function in terms of whether unsize coercions occur before or after the call, whereas through a block that isn't an issue.

    Blocked on Tracking issue for stmt_expr_attributes: Add attributes to expressions, etc. #15701.

  • Alternative: #[must_use] integrated into macro_rules, perhaps on each arm, or on the whole macro, perhaps with a lint or error if the macro does not produce an expression.

@dtolnay dtolnay added T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC labels Mar 8, 2022
@dtolnay
Copy link
Member Author

dtolnay commented Mar 8, 2022

Copied over 2 proposed alternatives from #94723 (comment). I like #[must_use] on blocks if that can be made to work. Though it would be blocked on #15701 which has been open since 2014.

@Mark-Simulacrum
Copy link
Member

I don't think it would be blocked on #15701 -- I believe exprs on blocks are currently permitted, e.g., cfg and lint attrs (allow etc) work:

fn foo() -> i32 {
    #[cfg(unix)]
    #[allow(unused)]
    {
        println!("test");
        let bar = 5;
        3
    }
}

I am not familiar with the implementation details of must_use enough to know whether putting it on a block causes problems, though naively I would assume it should not.

@nagisa
Copy link
Member

nagisa commented Mar 8, 2022

I'm somewhat concerned it might give an impression to a developer that

fn my_function() -> u32 {
    core::hint::must_use(42)
}

is equivalent to

#[must_use]
fn my_function() -> u32 {
    42
}

@dtolnay
Copy link
Member Author

dtolnay commented Mar 8, 2022

@Mark-Simulacrum what currently works is attributes on statements. In order for us to use #[must_use] as a replacement for core::hint::must_use it would need to work on expressions in general, not on statements, which is not supported:

let _ = #[allow(unused_must_use)] { 0 };
error[E0658]: attributes on expressions are experimental
 --> src/lib.rs:2:13
  |
2 |     let _ = #[allow(unused_must_use)] { 0 };
  |             ^^^^^^^^^^^^^^^^^^^^^^^^^
  |
  = note: see issue #15701 <https://github.com/rust-lang/rust/issues/15701> for more information

@dtolnay
Copy link
Member Author

dtolnay commented Mar 8, 2022

Alternatively I suppose it would be possible to support attributes only on an outermost macro-produced expression/block, without going all the way to attributes on expressions in general, if that's less prone to grammatical ambiguities or whatever is holding up #15701.

That is we could conceivably make the following work (which it doesn't today) without making the code in my previous comment work:

macro_rules! repro {
    () => {
        #[allow(unused_must_use)] { 0 }
    };
}


fn f() {
    let _ = repro!();
}

@CAD97
Copy link
Contributor

CAD97 commented Mar 17, 2022

To clarify, what @Mark-Simulacrum shows is not a statement; its' the entire body of the function, including tail return. You can use this to put an attribute on the body of any block:

[playground]

fn main() {
    let _ = { #[allow(unused_must_use)] 0 };
}

[playground]

pub fn is_foo_enabled() -> bool {
    #[cfg(feature = "foo")] {
        true
    }
    #[cfg(not(feature = "foo"))] {
        false
    }
}

[playground]

macro_rules! repro {
    () => {
        {
            // does not create a lint; the outer block "uses" it, I suspect
            #[must_use] {
                let [some, complicated, expansion] = [(); 3];
                let _ = [some, complicated, expansion];
                5
            }
        }
    };
}


pub fn f() {
    repro!();
}

@dtolnay
Copy link
Member Author

dtolnay commented Mar 17, 2022

All of what you showed are attributes on statements (StmtKind::Expr specifically). Attributes on expressions, where the expression is not a statement, are not supported on stable yet and would require #15701.

@dtolnay
Copy link
Member Author

dtolnay commented Mar 18, 2022

The reason #[must_use] on statements isn't suitable for the use case described in the docs of https://doc.rust-lang.org/nightly/core/hint/fn.must_use.html, which is an expression macro, is because you'd either need to write:

macro_rules! make_error {
    (...) => {
        #[must_use]
        {
            let error = ...;
            error
        }
    };
}

in which case a call site let _ = make_error!(...); becomes illegal (because attributes are not allowed on expressions); or you'd need to write:

macro_rules! make_error {
    (...) => {
        {
            #[must_use]
            {
                let error = ...;
                error
            }
        }
    };
}

in which case the #[must_use] doesn't do anything, because the value is always used as the value of the outer block, even if the macro caller does not use the value of the outer block:

macro_rules! repro {
    () => {
        {
            f()
        }
    }
}

#[must_use]
fn f() -> i32 {
    0
}

fn main() {
    repro!(); // not used, but no warning
}

@est31
Copy link
Member

est31 commented May 17, 2023

I've been wondering about using this function in the offset_of macro (#111669). It would be nice if you could customize the message. Ideally this would work via a &'static str const param:

#[lang = "must_use_fn"]
pub const fn must_use<T, const msg: &'static str>(value: T) -> T {
    value
}

This would require the adt_const_params feature though (#95174).

@wmmc88
Copy link

wmmc88 commented Aug 29, 2023

I'm somewhat concerned it might give an impression to a developer that

fn my_function() -> u32 {
    core::hint::must_use(42)
}

is equivalent to

#[must_use]
fn my_function() -> u32 {
    42
}

I am that developer 😄 . What exactly is the difference between these two must-use code blocks?

@est31
Copy link
Member

est31 commented Aug 31, 2023

@wmmc88 the former attaches the lint to the specific site (so it only fires once), while latter attaches it to every site my_function is called (so it fires as many times as you call my_function. Also, for former, only code inside the function matters while for latter, code at the site the function is called does. There is no cross-function dataflow analysis for the unused_must_use lint.

@tgross35
Copy link
Contributor

tgross35 commented Jul 6, 2024

Possible unresolved question: #124493

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Jul 8, 2024
Mark format! with must_use hint

Uses unstable feature rust-lang#94745

Part of rust-lang#126475

First contribution to rust, please let me know if the blessing of tests is correct
Thanks `@bjorn3` for the help
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Jul 8, 2024
Rollup merge of rust-lang#127355 - aceArt-GmbH:126475, r=oli-obk

Mark format! with must_use hint

Uses unstable feature rust-lang#94745

Part of rust-lang#126475

First contribution to rust, please let me know if the blessing of tests is correct
Thanks `@bjorn3` for the help
github-actions bot pushed a commit to rust-lang/miri that referenced this issue Jul 9, 2024
Mark format! with must_use hint

Uses unstable feature rust-lang/rust#94745

Part of #126475

First contribution to rust, please let me know if the blessing of tests is correct
Thanks `@bjorn3` for the help
flip1995 pushed a commit to flip1995/rust-clippy that referenced this issue Jul 11, 2024
Mark format! with must_use hint

Uses unstable feature rust-lang/rust#94745

Part of #126475

First contribution to rust, please let me know if the blessing of tests is correct
Thanks `@bjorn3` for the help
lnicola pushed a commit to lnicola/rust-analyzer that referenced this issue Jul 11, 2024
Mark format! with must_use hint

Uses unstable feature rust-lang/rust#94745

Part of #126475

First contribution to rust, please let me know if the blessing of tests is correct
Thanks `@bjorn3` for the help
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

7 participants