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

Outdated eval_order_dependence documentation #7624

Closed
shampoofactory opened this issue Sep 2, 2021 · 1 comment · Fixed by #7634
Closed

Outdated eval_order_dependence documentation #7624

shampoofactory opened this issue Sep 2, 2021 · 1 comment · Fixed by #7634
Labels
A-documentation Area: Adding or improving documentation good-first-issue These issues are a good way to get started with Clippy

Comments

@shampoofactory
Copy link

Lint name: eval_order_dependence

The documentation states:

In addition, the sub-expression evaluation order for Rust is not well documented.

Since the merging of Document execution order, is this not outdated advice? Sub-expression/ operand evaluation is now documented here.

Discussion

Take the following code, which is based on a heavily distilled macro expansion:

pub fn case_0() {
    let mut count = 0;
    let arr = [
        {
            let v = count;
            count += 1;
            v
        },
        {
            let v = count;
            count += 1;
            v
        },
        {
            let v = count;
            count += 1;
            v
        },
        { count },
    ];
    assert_eq!(arr, [0, 1, 2, 3]);
}

It fires off the warning: unsequenced read of 'count'/ whether read occurs before this write depends on evaluation order.

Fair enough. The code looks messy and the evaluation order is something we need to consider. However if we then read the documentation:

In addition, the sub-expression evaluation order for Rust is not well documented.

We are left with the impression that we have potentially undefined behaviour and unsound code.

However, based on my understanding of the updated reference, the evaluation order is well defined. The code is sound.

The following list of expressions all evaluate their operands the same way, as described after the list.
Other expressions either don't take operands or evaluate them conditionally as described on their respective pages.

    Dereference expression
    Error propagation expression
    Negation expression
    Arithmetic and logical binary operators
    Comparison operators
    Type cast expression
    Grouped expression
    Array expression
    Await expression
    Index expression
    Tuple expression
    Tuple index expression
    Struct expression
    Call expression
    Method call expression
    Field expression
    Break expression
    Range expression
    Return expression

The operands of these expressions are evaluated prior to applying the effects of the expression.
Expressions taking multiple operands are evaluated left to right as written in the source code.

Meta

clippy 0.1.54 (a178d03 2021-07-26)

rustc 1.54.0 (a178d0322 2021-07-26)
binary: rustc
commit-hash: a178d0322ce20e33eac124758e837cbd80a6f633
commit-date: 2021-07-26
host: x86_64-unknown-linux-gnu
release: 1.54.0
LLVM version: 12.0.1
```.
@camsteffen camsteffen added A-documentation Area: Adding or improving documentation good-first-issue These issues are a good way to get started with Clippy labels Sep 3, 2021
bors added a commit that referenced this issue Sep 5, 2021
…shearth

Fix documentation of eval_order_dependence

Fixes #7624.
@xFrednet
Copy link
Member

xFrednet commented Sep 5, 2021

Hey @chansuke, you can also claim issues by commenting @rustbot claim in an issue that you want to work on. A bot will then assign it to you. That simply avoids that several people start to work on the same issue 🙃

@bors bors closed this as completed in 17294b8 Sep 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-documentation Area: Adding or improving documentation good-first-issue These issues are a good way to get started with Clippy
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants