Page MenuHomePhabricator

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

Authored by jasonbcox on Jun 10 2020, 22:34.

Details

Summary

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

Pull request description:

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

ACKs for commit 0959d3:

Empact:
  ACK https://github.com/bitcoin/bitcoin/pull/16092/commits/0959d37e3e0f80010a78d175e3846dabf5d35919
MarcoFalke:
  ACK 0959d37e3e0f80010a78d175e3846dabf5d35919
hebasto:
  ACK 0959d37e3e0f80010a78d175e3846dabf5d35919
promag:
  ACK 0959d37.

Tree-SHA512: 722f66bb50450f19b57e8a8fbe949f30cd651eb8564e5787cbb772a539bf3a288c048dc49e655fd73ece6a46f6dafade515ec4004729bf2b3ab83117b7c5d153

Backport of Core PR16092

I followed the test plan in the original PR, but do not see those global symbols on master to begin with.
Regardless, this brings our codebase more inline with Core for future backporting's sake.

Test Plan

ninja check check-functional

Diff Detail

Repository
rABC Bitcoin ABC
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

[Bot Message]
One or more PR numbers were detected in the summary.
Links to those PRs have been inserted into the summary for reference.

This revision is now accepted and ready to land.Jun 10 2020, 23:34