Skip to content

ci: Run iwyu on all src files #27571

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

Merged
merged 1 commit into from
May 17, 2023
Merged

Conversation

maflcko
Copy link
Member

@maflcko maflcko commented May 4, 2023

This makes it easier to look at the CI output of a file without having to manually add it first to the list.

@DrahtBot
Copy link
Contributor

DrahtBot commented May 4, 2023

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

Reviews

See the guideline for information on the review process.

Type Reviewers
ACK hebasto
Concept ACK fanquake, jonatack

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

Conflicts

Reviewers, this pull request conflicts with the following ones:

  • #27636 (kernel: Remove interface_ui, util/system from kernel library by TheCharlatan)
  • #27576 (kernel: Remove args, chainparams, chainparamsbase from kernel library by TheCharlatan)
  • #27425 (test: move remaining rand code from util/setup_common to util/random by jonatack)
  • #27385 (net, refactor: extract Network and BIP155Network logic to node/network by jonatack)

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
Copy link
Member

fanquake commented May 9, 2023

Is this dependant on #27573

@maflcko
Copy link
Member Author

maflcko commented May 9, 2023

Yes, I can't figure out how to fight bash -c, so I removed it there

@fanquake
Copy link
Member

fanquake commented May 9, 2023

Concept ACK on removing the continual manual updating of this list

@maflcko maflcko marked this pull request as ready for review May 10, 2023 11:06
@maflcko maflcko force-pushed the 2305-ci-iwyu- branch 3 times, most recently from fa1adc7 to fade263 Compare May 10, 2023 11:22
@fanquake
Copy link
Member

(qt/qrc_bitcoin_locale.cpp has correct #includes/fwd-decls)

Looks like it's mostly working, except GUI stuff is still being dragged in

@hebasto
Copy link
Member

hebasto commented May 10, 2023

(qt/qrc_bitcoin_locale.cpp has correct #includes/fwd-decls)

Looks like it's mostly working, except GUI stuff is still being dragged in

And qt/qrc_bitcoin.cpp as well.

@maflcko maflcko force-pushed the 2305-ci-iwyu- branch 2 times, most recently from fa09249 to fa08bbf Compare May 10, 2023 14:00
@maflcko maflcko marked this pull request as draft May 10, 2023 14:27
@jonatack
Copy link
Member

Concept ACK. I've been adding sources files to this list while working on each pull.

This makes it easier to look at the CI output of a file without having
to manually add it first.
@maflcko maflcko marked this pull request as ready for review May 13, 2023 09:34
@maflcko
Copy link
Member Author

maflcko commented May 15, 2023

Should be ready for review

# accepted in src/.bear-tidy-config
# Filter out:
# * qt qrc and moc generated files
# * walletutil (temporarily)
Copy link
Member

@hebasto hebasto May 16, 2023

Choose a reason for hiding this comment

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

Mind elaborating about skipping walletutil?

Copy link
Member Author

Choose a reason for hiding this comment

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

because it crashes. See:

# python3 "/include-what-you-use/iwyu_tool.py" -p . ./src/wallet/walletutil.cpp -j 9 -- -Xiwyu --cxx17ns -Xiwyu --mapping_file="$PWD/contrib/devtools/iwyu/bitcoin.core.imp"  
In file included from wallet/walletutil.cpp:5:
In file included from ./wallet/walletutil.h:8:
In file included from ./script/descriptor.h:8:
In file included from ./outputtype.h:9:
In file included from ./script/signingprovider.h:10:
In file included from ./key.h:10:
In file included from ./pubkey.h:10:
In file included from ./hash.h:10:
In file included from ./crypto/common.h:12:
In file included from /usr/lib64/clang/16/include/stdint.h:52:
In file included from /usr/include/stdint.h:26:
In file included from /usr/include/bits/libc-header-start.h:33:
/usr/include/features.h:413:4: warning: _FORTIFY_SOURCE requires compiling with optimization (-O) [-W#warnings]
#  warning _FORTIFY_SOURCE requires compiling with optimization (-O)
   ^
In file included from wallet/walletutil.cpp:5:
In file included from ./wallet/walletutil.h:8:
In file included from ./script/descriptor.h:8:
In file included from ./outputtype.h:9:
In file included from ./script/signingprovider.h:10:
In file included from ./key.h:10:
In file included from ./pubkey.h:10:
In file included from ./hash.h:14:
In file included from ./serialize.h:17:
In file included from /usr/lib/gcc/x86_64-redhat-linux/13/../../../../include/c++/13/memory:78:
/usr/lib/gcc/x86_64-redhat-linux/13/../../../../include/c++/13/bits/unique_ptr.h:97:16: error: invalid application of 'sizeof' to an incomplete type 'wallet::DescriptorScriptPubKeyMan'
        static_assert(sizeof(_Tp)>0,
                      ^~~~~~~~~~~
/usr/lib/gcc/x86_64-redhat-linux/13/../../../../include/c++/13/bits/unique_ptr.h:404:4: note: in instantiation of member function 'std::default_delete<wallet::DescriptorScriptPubKeyMan>::operator()' requested here
          get_deleter()(std::move(__ptr));
          ^
/usr/lib/gcc/x86_64-redhat-linux/13/../../../../include/c++/13/bits/stl_construct.h:151:19: note: in instantiation of member function 'std::unique_ptr<wallet::DescriptorScriptPubKeyMan>::~unique_ptr' requested here
      __pointer->~_Tp();
                  ^
/usr/lib/gcc/x86_64-redhat-linux/13/../../../../include/c++/13/bits/stl_construct.h:196:2: note: in instantiation of function template specialization 'std::_Destroy_aux<false>::__destroy<std::unique_ptr<wallet::DescriptorScriptPubKeyMan> *>' requested here
        __destroy(__first, __last);
        ^
/usr/lib/gcc/x86_64-redhat-linux/13/../../../../include/c++/13/bits/alloc_traits.h:947:7: note: in instantiation of function template specialization 'std::_Destroy<std::unique_ptr<wallet::DescriptorScriptPubKeyMan> *>' requested here
      _Destroy(__first, __last);
      ^
/usr/lib/gcc/x86_64-redhat-linux/13/../../../../include/c++/13/bits/stl_vector.h:732:7: note: in instantiation of function template specialization 'std::_Destroy<std::unique_ptr<wallet::DescriptorScriptPubKeyMan> *, std::unique_ptr<wallet::DescriptorScriptPubKeyMan>>' requested here
        std::_Destroy(this->_M_impl._M_start, this->_M_impl._M_finish,
             ^
./wallet/walletutil.h:122:8: note: in instantiation of member function 'std::vector<std::unique_ptr<wallet::DescriptorScriptPubKeyMan>>::~vector' requested here
struct MigrationData
       ^
./wallet/walletutil.h:119:7: note: forward declaration of 'wallet::DescriptorScriptPubKeyMan'
class DescriptorScriptPubKeyMan;
      ^

Copy link
Member

@hebasto hebasto left a comment

Choose a reason for hiding this comment

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

ACK ddddf49

@fanquake fanquake merged commit 4c3d67a into bitcoin:master May 17, 2023
@maflcko maflcko deleted the 2305-ci-iwyu- branch May 17, 2023 12:09
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request May 17, 2023
fanquake added a commit to bitcoin-core/gui that referenced this pull request May 29, 2023
1f97572 Fix `#include`s in `src/wallet` (Hennadii Stepanov)

Pull request description:

  This PR is a minimum required changes to fix bitcoin/bitcoin#27571 (comment).

ACKs for top commit:
  MarcoFalke:
    lgtm ACK 1f97572

Tree-SHA512: de885210076d23f3394c42ca50e6ae2470c0ae6523399a2fa3ebb7c06383bdacef9c26166fa19747200396bed796c8772165e24416eb30ed8edd024e3394b2fe
@bitcoin bitcoin locked and limited conversation to collaborators May 16, 2024
DashCoreAutoGuix pushed a commit to DashCoreAutoGuix/dash that referenced this pull request Jul 26, 2025
1f97572 Fix `#include`s in `src/wallet` (Hennadii Stepanov)

Pull request description:

  This PR is a minimum required changes to fix bitcoin#27571 (comment).

ACKs for top commit:
  MarcoFalke:
    lgtm ACK 1f97572

Tree-SHA512: de885210076d23f3394c42ca50e6ae2470c0ae6523399a2fa3ebb7c06383bdacef9c26166fa19747200396bed796c8772165e24416eb30ed8edd024e3394b2fe
DashCoreAutoGuix pushed a commit to DashCoreAutoGuix/dash that referenced this pull request Jul 26, 2025
1f97572 Fix `#include`s in `src/wallet` (Hennadii Stepanov)

Pull request description:

  This PR is a minimum required changes to fix bitcoin#27571 (comment).

ACKs for top commit:
  MarcoFalke:
    lgtm ACK 1f97572

Tree-SHA512: de885210076d23f3394c42ca50e6ae2470c0ae6523399a2fa3ebb7c06383bdacef9c26166fa19747200396bed796c8772165e24416eb30ed8edd024e3394b2fe
DashCoreAutoGuix pushed a commit to DashCoreAutoGuix/dash that referenced this pull request Aug 1, 2025
1f97572 Fix `#include`s in `src/wallet` (Hennadii Stepanov)

Pull request description:

  This PR is a minimum required changes to fix bitcoin#27571 (comment).

ACKs for top commit:
  MarcoFalke:
    lgtm ACK 1f97572

Tree-SHA512: de885210076d23f3394c42ca50e6ae2470c0ae6523399a2fa3ebb7c06383bdacef9c26166fa19747200396bed796c8772165e24416eb30ed8edd024e3394b2fe
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants