Fix -Wmismatched-tags warnings #21051

pull hebasto wants to merge 2 commits into bitcoin:master from hebasto:210201-warning changing 2 files +2 −1

hebasto commented at 12:38 PM on February 1, 2021: member

Warnings were introduced in #20749:

./validation.h:43:1: warning: class 'CCheckpointData' was previously declared as a struct; this is valid, but may result in linker errors under the Microsoft C++ ABI [-Wmismatched-tags]
class CCheckpointData;
^
./chainparams.h:24:8: note: previous use is here
struct CCheckpointData {
       ^
./validation.h:43:1: note: did you mean struct here?
class CCheckpointData;
^~~~~
struct
1 warning generated.

This change fixes AppVeyor build: https://ci.appveyor.com/project/DrahtBot/bitcoin/builds/37547435

Fix -Wmismatched-tags warnings 1485124291
fanquake added the label Validation on Feb 1, 2021
jnewbery commented at 1:28 PM on February 1, 2021: member

Code review ACK 1485124291368c4a2ca8ea09c18e813f1dbabf5c

Thanks @hebasto!
practicalswift commented at 3:47 PM on February 1, 2021: contributor

cr ACK 1485124291368c4a2ca8ea09c18e813f1dbabf5c: patch looks correct

Thanks for fixing warnings!
hebasto commented at 4:27 PM on February 1, 2021: member

Thanks for fixing warnings!

Not warnings only, but MSVC builds are fixed as well :)
dongcarl commented at 5:41 PM on February 1, 2021: member

Code Review ACK 1485124291368c4a2ca8ea09c18e813f1dbabf5c 🤦 @dongcarl
ryanofsky approved
ryanofsky commented at 8:02 PM on February 1, 2021: member

Code review ACK. But I unclear on how appveyor failed to catch this before merging. Or maybe changes were merged despite appveyor failing? I do see appveyor errors one of my recent pushes: https://ci.appveyor.com/project/DrahtBot/bitcoin/builds/37552923

Would be nice to know what's going on with appveyor. Also ideally we would make either make this an error on linux, or make this not an error on windows to be consistent across platforms.
ajtowns commented at 8:59 PM on February 1, 2021: member
ACK 1485124291368c4a2ca8ea09c18e813f1dbabf5c

I think adding:
```
AX_CHECK_COMPILE_FLAG([-Werror=mismatched-tags],[ERROR_CXXFLAGS="$ERROR_CXXFLAGS -Werror=mismatched-tags"],,[[$CXXFLAG_WERROR]])
```
after the -Werror=unreachable-code-loop-increment line is all you need to make this an error for clang on linux when configuring with enable-werror.
build: Add -Werror=mismatched-tags b6aadcd5b4
hebasto commented at 9:04 PM on February 1, 2021: member

Updated.
glozow commented at 10:39 PM on February 1, 2021: member

utACK https://github.com/bitcoin/bitcoin/pull/21051/commits/b6aadcd5b4350a6ebcd57e88e7a0853cedf7c2fb 🚗

Thanks @hebasto, u so fast ⚡
practicalswift commented at 10:41 PM on February 1, 2021: contributor

cr ACK b6aadcd5b4350a6ebcd57e88e7a0853cedf7c2fb: patch looks correct
DrahtBot commented at 11:48 PM on February 1, 2021: member


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



Conflicts

Reviewers, this pull request conflicts with the following ones:
- #20544 (build: Do not repeat warning names in -Werror=... options by hebasto)
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.
fanquake merged this on Feb 2, 2021
fanquake closed this on Feb 2, 2021
jnewbery commented at 8:19 AM on February 2, 2021: member

But I unclear on how appveyor failed to catch this before merging. Or maybe changes were merged despite appveyor failing?

This is at least partially my fault. I encouraged @laanwj to merge #20749. I'd seen a "QT download hash mismatch" error in appveyor and assumed it was spurious.
hebasto deleted the branch on Feb 2, 2021
laanwj commented at 6:05 PM on February 2, 2021: member

Appveyor is flaky enough, and has been flaky enough historically (with really strange issues like sudden compiler upgrades breaking compatibility) that I must admit to sometimes ignore it. Sorry for that in this case.
sidhujag referenced this in commit 3a79b1874b on Feb 3, 2021
DrahtBot locked this on Aug 16, 2022