Don't use global (external) symbols for symbols that are used in only one translation unit #16092

Don't use global (external) symbols for symbols that are used in only one translation unit.

Before:

$ for SYMBOL in $(nm src/bitcoind | grep -E ' [BD] ' | c++filt | cut -f3- -d' ' | grep -v @ | grep -v : | sort | grep '[a-z]' | sort -u | grep -vE '(^_|typeinfo|vtable)'); do
      REFERENCES=$(git grep -lE "([^a-zA-Z]|^)${SYMBOL}([^a-zA-Z]|\$)" -- "*.cpp" "*.h")
      N_REFERENCES=$(wc -l <<< "${REFERENCES}")
      if [[ ${N_REFERENCES} > 1 ]]; then
          continue
      fi
      echo "Global symbol ${SYMBOL} is used in only one translation unit: ${REFERENCES}"
  done
Global symbol g_chainstate is used in only one translation unit: src/validation.cpp
Global symbol g_ui_signals is used in only one translation unit: src/ui_interface.cpp
Global symbol instance_of_cmaincleanup is used in only one translation unit: src/validation.cpp
Global symbol instance_of_cnetcleanup is used in only one translation unit: src/net.cpp
Global symbol instance_of_cnetprocessingcleanup is used in only one translation unit: src/net_processing.cpp
Global symbol pindexBestForkBase is used in only one translation unit: src/validation.cpp
Global symbol pindexBestForkTip is used in only one translation unit: src/validation.cpp
$ 

After:

$ for SYMBOL in $(nm src/bitcoind | grep -E ' [BD] ' | c++filt | cut -f3- -d' ' | grep -v @ | grep -v : | sort | grep '[a-z]' | sort -u | grep -vE '(^_|typeinfo|vtable)'); do
      REFERENCES=$(git grep -lE "([^a-zA-Z]|^)${SYMBOL}([^a-zA-Z]|\$)" -- "*.cpp" "*.h")
      N_REFERENCES=$(wc -l <<< "${REFERENCES}")
      if [[ ${N_REFERENCES} > 1 ]]; then
          continue
      fi
      echo "Global symbol ${SYMBOL} is used in only one translation unit: ${REFERENCES}"
  done
$ 

♻️ Think about future generations: save the global namespace from unnecessary pollution! ♻️

I whould say utACK b5476332cb5653552eefe25cc96131f04e2c8d8c

<!--e57a25ab6845829454e8d69fc972939a-->

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

<!--174a7506f384e20aa4161008e828411d-->

Conflicts

Reviewers, this pull request conflicts with the following ones:

• #16194 (refactor: share blockmetadata with BlockManager by jamesob)
• #15606 ([experimental] UTXO snapshots by jamesob)

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.

Concept ACK. Could be better to use an anon namespace where possible for these as that will make the variables static and enforce local-only access to the associated class as well.

@Empact The end-result would be the same as using static, right?

$ cat exporting.cpp
static int _static_int = 1;
namespace {
  int _namespace_int = 1;
  static int _namespace_static_int = 1;
}
int _int = 1;
$ clang++ -c exporting.cpp
$ nm exporting.o
0000000000000000 D _int

I don't have any strong preference -- I just want to minimise the diffs and make them super-trivial to review. Please read #15622 (comment) and other comments in that PR.

Sure, fine as is.

Rebased!

ACK https://github.com/bitcoin/bitcoin/pull/16092/commits/0959d37e3e0f80010a78d175e3846dabf5d35919

ACK 0959d37e3e0f80010a78d175e3846dabf5d35919

ACK 0959d37e3e0f80010a78d175e3846dabf5d35919 I have not tested the code, but I have reviewed it and it LGTM. Ref: https://en.cppreference.com/w/cpp/language/storage_duration#Linkage

ACK 0959d37.