Skip to content

[clang-tidy] unexpected readability-math-missing-parentheses warnings for assignment #92516

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

Closed
zufuliu opened this issue May 17, 2024 · 4 comments · Fixed by #94654
Closed

[clang-tidy] unexpected readability-math-missing-parentheses warnings for assignment #92516

zufuliu opened this issue May 17, 2024 · 4 comments · Fixed by #94654
Assignees

Comments

@zufuliu
Copy link

zufuliu commented May 17, 2024

see https://godbolt.org/z/7nK7fG4Gs, reduced from https://github.com/zufuliu/notepad2/blob/main/scintilla/src/Document.cxx#L691

// clang-tidy -checks=readability-math-missing-parentheses test.cpp --
int LevelNumberPart(int level);
bool LevelIsHeader(int level);
bool LevelIsWhitespace(int level);
int GetFoldLevel(int line);
int GetFoldParent(int line);
int GetLastChild(int line);

int GetHighlightDelimiters(int line) {
    const int level = GetFoldLevel(line);
    int lookLine = line;
    int lookLineLevel = level;
    int lookLineLevelNum = 0;

    const int beginFoldBlock = LevelIsHeader(lookLineLevel) ? lookLine : GetFoldParent(lookLine);
    const int endFoldBlock = GetLastChild(beginFoldBlock);
    int firstChangeableLineBefore = -1;
    for (lookLine = line - 1, lookLineLevel = GetFoldLevel(lookLine), lookLineLevelNum = LevelNumberPart(lookLineLevel);
        lookLine >= beginFoldBlock;
        lookLineLevel = GetFoldLevel(--lookLine), lookLineLevelNum = LevelNumberPart(lookLineLevel)) {
        if (LevelIsWhitespace(lookLineLevel) || (lookLineLevelNum > LevelNumberPart(level))) {
            firstChangeableLineBefore = lookLine;
            break;
        }
    }

    int firstChangeableLineAfter = -1;
    for (lookLine = line + 1, lookLineLevel = GetFoldLevel(lookLine), lookLineLevelNum = LevelNumberPart(lookLineLevel);
        lookLine <= endFoldBlock;
        lookLineLevel = GetFoldLevel(++lookLine), lookLineLevelNum = LevelNumberPart(lookLineLevel)) {
        if (LevelIsHeader(lookLineLevel) && (lookLineLevelNum < LevelNumberPart(GetFoldLevel(lookLine + 1)))) {
            firstChangeableLineAfter = lookLine;
            break;
        }
    }
    return firstChangeableLineBefore + firstChangeableLineAfter;
}
[<source>:18:21: warning: '-' has higher precedence than '='; add parentheses to explicitly specify the order of operations [readability-math-missing-parentheses]](javascript:;)
   18 |     for (lookLine = line - 1, lookLineLevel = GetFoldLevel(lookLine), lookLineLevelNum = LevelNumberPart(lookLineLevel);
      |                     ^~~~~~~~~
      |                     (       )
[<source>:28:21: warning: '+' has higher precedence than '='; add parentheses to explicitly specify the order of operations [readability-math-missing-parentheses]](javascript:;)
   28 |     for (lookLine = line + 1, lookLineLevel = GetFoldLevel(lookLine), lookLineLevelNum = LevelNumberPart(lookLineLevel);
      |                     ^~~~~~~~~
      |                     (       )
2 warnings generated.
@chrchr-github
Copy link

Reduced:

void f(int i) {
    int j, k;
    for (j = i + 1, k = 0; j < 1; ++j) {}
}

@PiotrZSL
Copy link
Member

Agree, check shouldn't warn for assignments, only for math operators.

@zufuliu
Copy link
Author

zufuliu commented May 17, 2024

Not sure why, but following code has no warnings:

void f(int i) {
    int j;
    for (j = i + 1; j < 1; ++j) {}
}

@zufuliu
Copy link
Author

zufuliu commented May 17, 2024

It seems somewhat related to the comma, following code has the warning too:

void f(int i) {
    int j;
    for (j = i + 1, 2; j < 1; ++j) {}
}

@PiotrZSL PiotrZSL self-assigned this Jun 6, 2024
PiotrZSL added a commit to PiotrZSL/llvm-project that referenced this issue Jun 6, 2024
…rentheses

Do not emit warnings for non-math operators.

Closes llvm#92516
PiotrZSL added a commit to PiotrZSL/llvm-project that referenced this issue Jun 8, 2024
…rentheses

Do not emit warnings for non-math operators.

Closes llvm#92516
PiotrZSL added a commit that referenced this issue Jun 9, 2024
…rentheses (#94654)

Do not emit warnings for non-math operators.

Closes #92516
nekoshirro pushed a commit to nekoshirro/Alchemist-LLVM that referenced this issue Jun 9, 2024
…rentheses (llvm#94654)

Do not emit warnings for non-math operators.

Closes llvm#92516

Signed-off-by: Hafidz Muzakky <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants