Join GitHub today
GitHub is home to over 20 million developers working together to host and review code, manage projects, and build software together.
Avoid some new gcc warnings in 15 #10808
Conversation
MeshCollider
commented
Jul 12, 2017
|
utACK |
|
utACK a8a602e |
| - prevector() : _size(0) {} | ||
| + prevector() : _size(0) { | ||
| + // Explicitly initialize indirect ptr to avoid "used uninitialized" warning | ||
| + _union.indirect = nullptr; |
sipa
Jul 13, 2017
Owner
I was vaguely concerned that this assignment violates type punning through a union, but (1) some sources say that is permitted anyway (2) the other type is char (for which exceptions to type punning exist) and (3) we'll write to the union again before ever using data in it.
laanwj
Jul 13, 2017
Owner
I though type-punning through an union was the only acceptable way to do it?
theuni
Jul 13, 2017
Member
From IRC:
sipa: why not use aggregate initialization: prevector() : _size(0), _union{{}}
or give the union a ctor?
I can't reproduce the warning, so I can't be sure that it satisfies whatever compiler is complaining, but the above compiles fine for me, and I'd think that initializing via a ctor would work otherwise.
gmaxwell
Jul 13, 2017
Member
@laanwj There is a faction of language wonks/compiler folks that argues that it is never acceptable (except with char) per the language specs; -- and I believe there is nothing in any of the C++ standards that make it clear that its kosher (though sipa tells me apparently C11 has something explicit). Since having no way to do it is not a very realistic position it doesn't reflect the behavior of compilers today. Generally better to avoid it at least where its convenient, or at least be aware that someday it might become non-kosher depending on which evil spirits control the souls of the compiler authors next week. :)
TheBlueMatt
Jul 14, 2017
Contributor
I'm super not convinced this is an issue. If we're really concerned, easiest fix is probably require C11 and turn prevector into C++ wrapper around C. More realistically, gcc explicitly supports this, and I believe many other compilers as well.
|
utACK |
|
ACK Please fix typo in the second commit message (initizlie). |
fanquake
referenced
this pull request
Jul 13, 2017
Closed
Change type of op to agree with type of MAX_OPCODE. #10814
|
@theuni's suggestion looks the cleanest to me. Would you mind trying that? |
|
OK, took @theuni's version (which has no warnings for me). |
|
utACK c73b8be |
sipa
merged commit c73b8be
into
bitcoin:master
Jul 15, 2017
1 check was pending
sipa
added a commit
that referenced
this pull request
Jul 15, 2017
|
|
sipa |
ec8a50b
|
TheBlueMatt commentedJul 12, 2017
This, plus #10714 avoids adding new compile-time warnings in 15.
Fixes a warning added in #10792.