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

build: Disable -Wbraced-scalar-init, which is incompatible with -Wc++11-narrowing #23191

Closed
wants to merge 1 commit into from

Conversation

maflcko
Copy link
Member

@maflcko maflcko commented Oct 5, 2021

To test:

void Fun(int) {}
int main() {
    unsigned a(-1);
    Fun({a});
}

Before:

$ clang++ -Wbraced-scalar-init -Wc++11-narrowing -std=c++17 /tmp/1.cpp -o /tmp/exe 
/tmp/1.cpp:4:10: error: non-constant-expression cannot be narrowed from type 'unsigned int' to 'int' in initializer list [-Wc++11-narrowing]
    Fun({a});
         ^
/tmp/1.cpp:4:10: note: insert an explicit cast to silence this issue
    Fun({a});
         ^
         static_cast<int>( )
/tmp/1.cpp:4:9: warning: braces around scalar initializer [-Wbraced-scalar-init]
    Fun({a});
        ^~~
1 warning and 1 error generated.

After:

$ clang++ -Wno-braced-scalar-init -Wc++11-narrowing -std=c++17 /tmp/1.cpp -o /tmp/exe 
/tmp/1.cpp:4:10: error: non-constant-expression cannot be narrowed from type 'unsigned int' to 'int' in initializer list [-Wc++11-narrowing]
    Fun({a});
         ^
/tmp/1.cpp:4:10: note: insert an explicit cast to silence this issue
    Fun({a});
         ^
         static_cast<int>( )
1 error generated.

@practicalswift
Copy link
Contributor

practicalswift commented Oct 5, 2021

Concept ACK

@vasild
Copy link
Contributor

vasild commented Oct 5, 2021

Why is it incompatible? In the output above there is 1 error due to Wc++11-narrowing and 1 warning due to Wbraced-scalar-init. When the latter is disabled the warning is not shown. I don't see the relation (incompatibility) between the two options.

OTOH, F({a}) looks wrong, I would like the compiler to point it to me so that I can fix it to F(a), no?

@vasild
Copy link
Contributor

vasild commented Oct 5, 2021

Is the intention to use F({a}) with Wc++11-narrowing to get a warning for passing "incompatible" types? Could the same be achieved by F(a) with Wimplicit-int-conversion and/or Wsign-conversion?

@maflcko
Copy link
Member Author

maflcko commented Oct 5, 2021

OTOH, F({a}) looks wrong, I would like the compiler to point it to me so that I can fix it to F(a), no?

F(a) will disable -Wc++11-narrowing.

@maflcko
Copy link
Member Author

maflcko commented Oct 5, 2021

Could the same be achieved by F(a) with Wimplicit-int-conversion and/or Wsign-conversion?

It is currently not possible to compile Bitcoin Core with either of those settings.

@DrahtBot
Copy link
Contributor

DrahtBot commented Oct 6, 2021

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

Reviewers, this pull request conflicts with the following ones:

  • #23269 (build: remove redundant warning flags by fanquake)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

@DrahtBot
Copy link
Contributor

🐙 This pull request conflicts with the target branch and needs rebase.

Want to unsubscribe from rebase notifications on this pull request? Just convert this pull request to a "draft".

@@ -475,6 +475,8 @@ if test "x$CXXFLAGS_overridden" = "xno"; then
AX_CHECK_COMPILE_FLAG([-Wunused-parameter],[NOWARN_CXXFLAGS="$NOWARN_CXXFLAGS -Wno-unused-parameter"],,[[$CXXFLAG_WERROR]])
AX_CHECK_COMPILE_FLAG([-Wself-assign],[NOWARN_CXXFLAGS="$NOWARN_CXXFLAGS -Wno-self-assign"],,[[$CXXFLAG_WERROR]])
AX_CHECK_COMPILE_FLAG([-Wunused-local-typedef],[NOWARN_CXXFLAGS="$NOWARN_CXXFLAGS -Wno-unused-local-typedef"],,[[$CXXFLAG_WERROR]])
dnl -Wbraced-scalar-init is incompatible with -Wc++11-narrowing
AX_CHECK_COMPILE_FLAG([-Wbraced-scalar-init],[NOWARN_CXXFLAGS="$NOWARN_CXXFLAGS -Wno-braced-scalar-init"],,[[$CXXFLAG_WERROR]])
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
AX_CHECK_COMPILE_FLAG([-Wbraced-scalar-init],[NOWARN_CXXFLAGS="$NOWARN_CXXFLAGS -Wno-braced-scalar-init"],,[[$CXXFLAG_WERROR]])
AX_CHECK_COMPILE_FLAG([-Wbraced-scalar-init], [NOWARN_CXXFLAGS="$NOWARN_CXXFLAGS -Wno-braced-scalar-init"], [], [$CXXFLAG_WERROR])

@maflcko maflcko closed this Nov 17, 2021
@maflcko maflcko deleted the 2110-buildW branch November 17, 2021 19:23
@bitcoin bitcoin locked and limited conversation to collaborators Nov 17, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants