Skip to content

-Wbraced-scalar-init warns inappropriately #54702

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

Open
Leengit opened this issue Apr 1, 2022 · 0 comments
Open

-Wbraced-scalar-init warns inappropriately #54702

Leengit opened this issue Apr 1, 2022 · 0 comments
Labels
clang:diagnostics New/improved warning or error message in Clang, but not in clang-tidy or static analyzer

Comments

@Leengit
Copy link

Leengit commented Apr 1, 2022

Braces around the returned value serve a legitimate purpose in

char f()
{
  int i = 65537;
  return{ i };
}

They tell the compiler to fail because the return value is narrowed; without the braces this would compile. Yet, -Wbraced-scalar-init warns that this is a superfluous use of braces. Likewise if the type of i is instead char the braces legitimately ask the compiler to check for narrowing, which in this case is not occurring; in this situation too, -Wbraced-scalar-init should not be warning that this is a superfluous use of braces. (In practical cases, the returned value might be from a function that has a return type that is not so readily apparent and the code writer should instead be encouraged to use braces in situations where narrowing is a concern!)

This occurs with both:

clang version 10.0.0-4ubuntu1
Target: x86_64-pc-linux-gnu
Thread model: posix
InstalledDir: /usr/bin

and:

Ubuntu clang version 12.0.0-3ubuntu1~20.04.5
Target: x86_64-pc-linux-gnu
Thread model: posix
InstalledDir: /usr/bin

@EugeneZelenko EugeneZelenko added clang:diagnostics New/improved warning or error message in Clang, but not in clang-tidy or static analyzer and removed new issue labels Apr 1, 2022
hsutter added a commit to hsutter/cppfront that referenced this issue Apr 1, 2023
Note this change adds `{` `}` around member assignment statements in assignment operators.

That change now triggers Clang's `-Wbraced-scalar-init` warning, but this warning appears to be spurious. See the open Clang issue here llvm/llvm-project#54702 which mentions the same justification I had in mind (disallow implicit narrowing, that's the #1 reason to recommend using `{` `}` init where possible). Unfortunately that Clang issue still open without comments after a year and so it's hard to know the status.

(BTW, what a coincidence -- I actually hit this on the exact first anniversary of that issue, which also happens to be April 1 but I swear this is not a joke and I think the LLVM issue wasn't either...)
vLKp added a commit to dxx-rebirth/dxx-rebirth that referenced this issue Nov 13, 2023
As reported in <llvm/llvm-project#54702>,
clang warns about use of braces in contexts where they are useful to
explicitly disallow narrowing conversions.  Such braces should not be
removed, as it would allow a future change to accidentally introduce a
narrowing conversion and not receive a diagnostic.  As of this commit,
the LLVM issue has seen no activity and there is no indication the
spurious warning will be fixed.

Add an SConf test to detect if the compiler warns for these cases, and
suppress it if so.
zaucy pushed a commit to zaucy/cppfront that referenced this issue Dec 5, 2023
Note this change adds `{` `}` around member assignment statements in assignment operators.

That change now triggers Clang's `-Wbraced-scalar-init` warning, but this warning appears to be spurious. See the open Clang issue here llvm/llvm-project#54702 which mentions the same justification I had in mind (disallow implicit narrowing, that's the hsutter#1 reason to recommend using `{` `}` init where possible). Unfortunately that Clang issue still open without comments after a year and so it's hard to know the status.

(BTW, what a coincidence -- I actually hit this on the exact first anniversary of that issue, which also happens to be April 1 but I swear this is not a joke and I think the LLVM issue wasn't either...)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:diagnostics New/improved warning or error message in Clang, but not in clang-tidy or static analyzer
Projects
None yet
Development

No branches or pull requests

2 participants