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

[practicalswift]

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! ♻️

[JosuGZ]

I whould say utACK b547633

[DrahtBot]

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

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.

[Empact]

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.

[practicalswift]

@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.

[Empact]

Sure, fine as is.

[practicalswift]

Rebased!

[Empact]

ACK 0959d37

[maflcko]

ACK 0959d37

[hebasto]

ACK 0959d37
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

[promag]

ACK 0959d37.