[WIP] Refactor: Remove all uses of using namespace in all source files.

kallewoof commented at 1:49 am on November 29, 2016: member

According to the “Source code organization” section in Developer notes, using namespace should be avoided. This commit removes all uses of using namespace <xxx> in all source files, including test and bench related files, ~~as well as the univalue library~~.

This PR is being split into multiple PR’s as described below.

bench and test (#9281):

 0bench/bench.cpp:          benchmark
 1bench/coin_selection.cpp: std
 2test/addrman_tests.cpp:   std
 3test/bloom_tests.cpp:     std
 4test/dbwrapper_tests.cpp: std [namespace boost::filesystem mapped to io for brevity]
 5test/hash_tests.cpp:      std [unused]
 6test/key_tests.cpp:       std
 7test/multisig_tests.cpp:  std
 8test/net_tests.cpp:       std
 9test/netbase_tests.cpp:   std
10test/pmt_tests.cpp:       std [unused]
11test/pow_tests.cpp:       std [unused]
12test/rpc_tests.cpp:       std
13test/script_P2SH_tests.cpp: std
14test/script_tests.cpp:    std
15test/serialize_tests.cpp: std
16test/sigopcount_tests.cpp: std
17test/streams_tests.cpp:   std [using boost::assign remains, for +=() operator]
18test/timedata_tests.cpp:  std [unused]
19test/transaction_tests.cpp: std
20test/univalue_tests.cpp:  std
21test/util_tests.cpp:      std [unused]

rpc and script (#9476):

 0rpc/blockchain.cpp:       std
 1rpc/client.cpp:           std
 2rpc/mining.cpp:           std
 3rpc/misc.cpp:             std
 4rpc/net.cpp:              std
 5rpc/protocol.cpp:         std
 6rpc/rawtransaction.cpp:   std
 7rpc/server.cpp:           RPCServer [unused], std
 8script/interpreter.cpp:   std
 9script/ismine.cpp:        std
10script/script.cpp:        std
11script/sign.cpp:          std
12script/standard.cpp:      std

wallet and util* (#9643):

0util.cpp:                 std
1utilmoneystr.cpp:         std
2utilstrencodings.cpp:     std
3utiltime.cpp:             std [unused]
4wallet/db.cpp:            std
5wallet/rpcdump.cpp:       std
6wallet/rpcwallet.cpp:     std
7wallet/test/wallet_tests.cpp: std
8wallet/wallet.cpp:        std
9wallet/walletdb.cpp:      std

Remaining (#9644):

 0bloom.cpp:                std
 1chain.cpp:                std [unused]
 2core_read.cpp:            std
 3core_write.cpp:           std
 4init.cpp:                 std, boost::filesystem [inside void CleanupBlockRevFiles()]
 5merkleblock.cpp:          std
 6miner.cpp:                std
 7net_processing.cpp:       std
 8rest.cpp:                 std
 9timedata.cpp:             std
10txdb.cpp:                 std
11txmempool.cpp:            std
12validation.cpp:           std

kallewoof renamed this:
~~Refactor: Removes all uses of `using namespace` in all source files.~~
Refactor: Removes all uses of using namespace in all source files.
on Nov 29, 2016

fanquake added the label Refactoring on Nov 29, 2016

dcousens commented at 3:36 am on November 29, 2016: contributor

concept ACK, the diff is small enough I don’t see this is as a change that would ruin history, while still being a large scale [trivial] refactor.

paveljanik commented at 8:59 am on November 29, 2016: contributor

Concept ACK. It has to be done.

Do not change univalue tree here. Why is using namespace bench bad here?

kallewoof commented at 9:07 am on November 29, 2016: member

@paveljanik I reverted the univalue changes. Using namespaces anywhere is generally a bad idea. The primary reason is explained better than I can here. The gist of it is captured in this sentence: “Library Foo 2.0 could introduce a function, Quux(), that is an unambiguously better match for some of your calls to Quux() than the bar::Quux() your code called for years. Then your code still compiles, but it silently calls the wrong function and does god-knows-what. That’s about as bad as things can get.” Also, a user may see namespace use in one file and think it’s fine to do it elsewhere.

kallewoof commented at 9:30 am on November 29, 2016: member

FWIW, binary check still passes (updated initial post with output for e81f1a0).

jonasschnelli commented at 10:49 am on November 29, 2016: contributor

Concept ACK

laanwj commented at 11:30 am on November 29, 2016: member

“Using namespaces anywhere is generally a bad idea.”

To disambiguate this: The use of namespaces in general is not a bad idea, right? Namespaces can IMO be a useful code-organization principle. But the using namespace statement is - that part I agree on, certainly in global scope.

This changes so many source files though :( Let’s make sure that we first merge in the features and fixes that we really want in 0.14, to avoid giving people extra rebasing busywork.

kallewoof commented at 11:51 am on November 29, 2016: member

@laanwj Oh, yes apologies about ambiguous wording. If I were to decide there’d be a lot more namespaces in the code, but one step at a time, right? :)

There are a lot of files changed, yeah. Luckily, rebasing is extremely trivial and straight-forward, and that goes both ways, so I will gladly rebase this PR once you feel ready for it.

A secondary option is that you pick files that you want and I make separate PR:s for those files and we do this iteratively.

kallewoof force-pushed on Nov 29, 2016

fanquake commented at 6:15 am on December 2, 2016: member

@kallewoof I’ve merged this, and can still see lots of usage of namespace std i.e in addrman_tests, or dbwrapper_tests. Have they been left on purpose?

kallewoof commented at 6:23 am on December 2, 2016: member

@fanquake I used a “find all” for using namespace and fixed everything, but you’re right, somehow there’s a lot left.

For starters I’m going to fix the leftovers so this PR is complete. Thanks for pointing that out!

kallewoof renamed this:
~~Refactor: Removes all uses of using namespace in all source files.~~
[WIP] Refactor: Removes all uses of using namespace in all source files.
on Dec 2, 2016

kallewoof force-pushed on Dec 2, 2016

kallewoof force-pushed on Dec 5, 2016

Removes all uses of `using namespace <xxx>` in all source files, including test and bench related files. 2bfef03102

kallewoof force-pushed on Dec 5, 2016

kallewoof renamed this:
~~[WIP] Refactor: Removes all uses of using namespace in all source files.~~
[WIP] Refactor: Remove all uses of using namespace in all source files.
on Jan 4, 2017

jtimon commented at 4:02 pm on January 13, 2017: contributor

Concept ACK, but this may be too disruptive for a single PR, perhaps you should consider dividing it in a few smaller ones. Maybe start with namespace std? Personally other uses like using namespace benchmark ins src/bench/bench.cpp don’t bother me that much, but I’m not against removing them either. Another advantage of dividing it in smaller ones (apart from each being more reviewable and mergeable separated) is that they will individually require rebase less often. Needs rebase.

kallewoof commented at 11:03 pm on January 13, 2017: member

Yeah, I started doing that (2 PRs so far; I plan to make the next one whenever the current one is merged to not swamp the members), but felt I could keep this PR around as a central point of sorts. I am closing this one though as it can be closed and still be referenced.

kallewoof closed this on Jan 13, 2017

kallewoof deleted the branch on Oct 17, 2019

DrahtBot locked this on Dec 16, 2021

[WIP] Refactor: Remove all uses of using namespace in all source files. #9235