gcc: fix uninitialized constructor warnings #8979
Conversation
| @@ -77,15 +77,15 @@ namespace cryptonote | |||
| // outputs <= HF_VERSION_VIEW_TAGS | |||
| struct txout_to_key | |||
| { | |||
| txout_to_key() { } | |||
| txout_to_key() = default; | |||
There was a problem hiding this comment.
From cppreference.com under "implicitly-defined default constructor":
it has the same effect as a user-defined constructor with empty body and empty initializer list.
So this should be the same as existing code? Or is that the intent?
There was a problem hiding this comment.
Yes it should... I looked into it a bit more and I'm convinced that there's a bug with my version of g++ or maybe there's some obscure undefined behavior that is triggered in the verRctNonSemanticsSimple.serializable_sig_changes unit test. Either way, changing a ._push_back({}) to a ._emplace_back() fixed the warning which was generating pages and pages of garbage messages.
jeffro256
force-pushed
the
bc_and_pool_unit_warning
branch
from
52ae338 to
414847c
Compare
|
@UkoeHB latest commit also fixes the Microsoft C++ ABI -Wmismatched-tags warning you mentioned. |
src/ringct/rctTypes.h
Outdated
| @@ -217,8 +217,7 @@ namespace rct { | |||
| rct::keyV L, R; | |||
| rct::key a, b, t; | |||
|
|
|||
| Bulletproof(): | |||
| A({}), S({}), T1({}), T2({}), taux({}), mu({}), a({}), b({}), t({}) {} | |||
| Bulletproof() = default; | |||
There was a problem hiding this comment.
This changes the behavior, and now the keys are not initialized to zero (except for V) if Bulletproof is default constructed.
There was a problem hiding this comment.
Why would they not be initialized to zero? rct::key is a class type with no user provided constructor, and the default constructor of Bulletproof will call the implicit default constructor of rct::key, which will zero out the bytes[32] array.
There was a problem hiding this comment.
@vtnerd can you also confirm this is resolved?
There was a problem hiding this comment.
The default constructor does default initialization, not value initialization.
There was a problem hiding this comment.
@selsta the problem is not resolved
There was a problem hiding this comment.
https://godbolt.org/z/rMhd75PEn notice that there is no zero-initialization in the ASM.
There was a problem hiding this comment.
You're right, I was mixing up default initialization and value initialization
There was a problem hiding this comment.
Latest commit fixes that
jeffro256
force-pushed
the
bc_and_pool_unit_warning
branch
from
414847c to
80b5bf8
Compare
No description provided.