NFD » ndn-cxx

Question: should the backport be placed in std namespace or ndn namespace?

Junxiao Shi wrote:

This issue does not involve changing existing code to use make_unique.

Why not? It's trivial to port existing code over to make_unique, and not invasive, plus keeping a "mixed" codebase for a long time could be confusing to contributors.

Answer to note-3:

I don't know about an easy regex to exhaustively locate all violations in the codebase.
Without this regex, one cannot quickly determine whether a commit for this issue changes every possible place in the codebase to use make_unique.

However, an issue should have a clear boundary of completion, so I decide not to require such changes.
But I won't reject a patch that make one or more changes to use make_unique.

Something like 'unique_ptr.*new ' maybe (note the trailing whitespace)? It's not important if we do not catch all cases, the majority would be enough.

20150901 conference call approves this feature.

Answer to note-1: This should be declared in std, and imported to ndn.

What do we do if we want to add make_unique to NFD?

• We can't rely on the definition in ndn-cxx's common.hpp, because that file is considered private.
• We probably can't define another make_unique in the std namespace, because it will conflict with the ndn-cxx definition, via indirect inclusion of common.hpp.

So I disagree with note 7. We should declare make_unique in namespace ndn only.

Moreover, adding new definitions to the std namespace is prohibited by the ISO C++ standard.

17.6.4.2.1 Namespace std

The behavior of a C++ program is undefined if it adds declarations or definitions to namespace std or to a
namespace within namespace std unless otherwise specified. A program may add a template specialization
for any standard library template to namespace std only if the declaration depends on a user-defined type
and the specialization meets the standard library requirements for the original template and is not explicitly
prohibited.

Similar change should be applied to NFD as part of this issue. nfd::make_unique

It should be committed to master branch, and then feature-lp branch can be rebased to make use of it.

Having ndn::make_unique in common.hpp prevents it from being reused in other projects, because common.hpp is an implementation detail of ndn-cxx.

Other projects would have to make the same change, which causes code duplication and increases maintenance cost should a bug occur.

Recommendation:

1. declare ndn::make_unique in util/backports.hpp

   Other backports, such as ndn::to_string, can be added to this file in the future.

2. put #include "util/backports.hpp" at the end of common.hpp

   Usually common.hpp does not include other headers within ndn-cxx, but this one could be a special case.
   To avoid circular dependency, backports.hpp should not include common.hpp.

3. test suite should be placed in backports.t.cpp