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

[libyul] Fix false-negative maybe-uninitialized error on gcc. #15724

Merged
merged 1 commit into from
Jan 17, 2025

Conversation

aarlt
Copy link
Member

@aarlt aarlt commented Jan 17, 2025

Building with gcc v13.3.0 was not possible due to maybe-uninitialized errors, e.g.

In file included from /usr/include/c++/13/bits/shared_ptr.h:53,
                 from /usr/include/c++/13/memory:80,
                 from /home/parallels/git/http/solidity/liblangutil/SourceLocation.h:27,
                 from /home/parallels/git/http/solidity/libsolutil/Exceptions.h:21,
                 from /home/parallels/git/http/solidity/libyul/Exceptions.h:24,
                 from /home/parallels/git/http/solidity/libyul/optimiser/ASTWalker.h:26,
                 from /home/parallels/git/http/solidity/libyul/optimiser/ForLoopConditionIntoBody.h:20,
                 from /home/parallels/git/http/solidity/libyul/optimiser/ForLoopConditionIntoBody.cpp:19:
In destructor ‘std::__shared_count<_Lp>::~__shared_count() [with __gnu_cxx::_Lock_policy _Lp = __gnu_cxx::_S_atomic]’,
    inlined from ‘std::__shared_ptr<_Tp, _Lp>::~__shared_ptr() [with _Tp = const solidity::langutil::DebugData; __gnu_cxx::_Lock_policy _Lp = __gnu_cxx::_S_atomic]’ at /usr/include/c++/13/bits/shared_ptr_base.h:1524:7,
    inlined from ‘std::shared_ptr<const solidity::langutil::DebugData>::~shared_ptr()’ at /usr/include/c++/13/bits/shared_ptr.h:175:11,
    inlined from ‘virtual void solidity::yul::ForLoopConditionIntoBody::operator()(solidity::yul::ForLoop&)’ at /home/parallels/git/http/solidity/libyul/optimiser/ForLoopConditionIntoBody.cpp:50:7:
/usr/include/c++/13/bits/shared_ptr_base.h:1070:13: error: ‘<unnamed>.solidity::yul::BuiltinName::debugData.std::shared_ptr<const solidity::langutil::DebugData>::<unnamed>.std::__shared_ptr<const solidity::langutil::DebugData, __gnu_cxx::_S_atomic>::_M_refcount.std::__shared_count<>::_M_pi’ may be used uninitialized [-Werror=maybe-uninitialized]
 1070 |         if (_M_pi != nullptr)
      |             ^~~~~
/home/parallels/git/http/solidity/libyul/optimiser/ForLoopConditionIntoBody.cpp: In member function ‘virtual void solidity::yul::ForLoopConditionIntoBody::operator()(solidity::yul::ForLoop&)’:
/home/parallels/git/http/solidity/libyul/optimiser/ForLoopConditionIntoBody.cpp:50:114: note: ‘<anonymous>’ declared here
   50 |                                                 BuiltinName{debugData, *m_dialect.booleanNegationFunctionHandle()},
      |                                                                                                                  ^
cc1plus: all warnings being treated as errors
make[2]: *** [libyul/CMakeFiles/yul.dir/build.make:860: libyul/CMakeFiles/yul.dir/optimiser/ForLoopConditionIntoBody.cpp.o] Error 1
make[2]: *** Waiting for unfinished jobs....
make[1]: *** [CMakeFiles/Makefile2:562: libyul/CMakeFiles/yul.dir/all] Error 2
make: *** [Makefile:136: all] Error 2

@aarlt aarlt changed the title [libyul] Fix false-negative maybe-uninitialized warning on gcc. [libyul] Fix false-negative maybe-uninitialized error on gcc. Jan 17, 2025
Copy link
Member

@cameel cameel left a comment

Choose a reason for hiding this comment

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

Looks fine, but things like for things like this you should always include some context. I.e. add a comment saying why you're doing it and maybe paste the relevant bits of the discussion into the PR description. If I hadn't seen the discussion, I'd have no idea whether this change makes sense or not.

@aarlt aarlt merged commit 53fc9f4 into develop Jan 17, 2025
74 checks passed
@aarlt aarlt deleted the fix_uninitialized_errors branch January 17, 2025 14:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants