Bug #3882
closed
Fib::const_iterator is not default-constructible
100%
Description
Even though we have the concept checks for default-construction, it fails on Ubuntu 14.04.5 with gcc (Ubuntu 4.8.4-2ubuntu1~14.04.3). The same code compiles on Ubuntu 16.04.1 with stock gcc and on macOS 10.12 with stock clang.
The error appears when compiling the snippet within ndnSIM. I will try to create a simpler snippet later.
...
static PyObject* _wrap_PyNs3NdnNfdFib_Iter__tp_iternext(PyNs3NdnNfdFib_Iter *self)
{
...
ns3::ndn::nfd::Fib::const_iterator iter;
...
}
The error I'm getting is the following
[1377/2352] Compiling build/src/ndnSIM/bindings/ns3module.cc
In file included from /usr/include/boost/range/adaptor/transformed.hpp:16:0,
from /usr/include/boost/range/adaptor/map.hpp:14,
from ./ns3/ndnSIM/NFD/daemon/fw/face-table.hpp:31,
from ./ns3/ndnSIM/NFD/daemon/fw/forwarder.hpp:32,
from ./ns3/ndnSIM/helper/ndn-strategy-choice-helper.hpp:31,
from ./ns3/ndnSIM/helper/ndn-stack-helper.hpp:32,
from ./ns3/ndn-all.hpp:23,
from ./ns3/ndnSIM-module.h:10,
from src/ndnSIM/bindings/ns3module.h:78,
from src/ndnSIM/bindings/ns3module.cc:1:
/usr/include/boost/iterator/transform_iterator.hpp: In instantiation of ‘boost::transform_iterator<UnaryFunction, Iterator, Reference, Value>::transform_iterator() [with UnaryFunc = nfd::name_tree::GetTableEntry<nfd::fib::Entry>; Iterator = nfd::name_tree::Iterator; Reference = boost::use_default; Value = boost::use_default]’:
src/ndnSIM/bindings/ns3module.cc:1723:40: required from here
/usr/include/boost/iterator/transform_iterator.hpp:84:26: error: no matching function for call to ‘nfd::name_tree::GetTableEntry<nfd::fib::Entry>::GetTableEntry()’
transform_iterator() { }
^
/usr/include/boost/iterator/transform_iterator.hpp:84:26: note: candidates are:
In file included from ../src/ndnSIM/NFD/daemon/table/name-tree-hashtable.hpp:29:0,
from ../src/ndnSIM/NFD/daemon/table/name-tree-iterator.hpp:29,
from ../src/ndnSIM/NFD/daemon/table/name-tree.hpp:29,
from ../src/ndnSIM/NFD/daemon/table/fib.hpp:30,
from ./ns3/ndnSIM/NFD/daemon/fw/forwarder.hpp:34,
from ./ns3/ndnSIM/helper/ndn-strategy-choice-helper.hpp:31,
from ./ns3/ndnSIM/helper/ndn-stack-helper.hpp:32,
from ./ns3/ndn-all.hpp:23,
from ./ns3/ndnSIM-module.h:10,
from src/ndnSIM/bindings/ns3module.h:78,
from src/ndnSIM/bindings/ns3module.cc:1:
../src/ndnSIM/NFD/daemon/table/name-tree-entry.hpp:193:3: note: nfd::name_tree::GetTableEntry<ENTRY>::GetTableEntry(nfd::name_tree::GetTableEntry<ENTRY>::Getter) [with ENTRY = nfd::fib::Entry; nfd::name_tree::GetTableEntry<ENTRY>::Getter = nfd::fib::Entry* (nfd::name_tree::Entry::*)()const]
GetTableEntry(Getter getter)
^
../src/ndnSIM/NFD/daemon/table/name-tree-entry.hpp:193:3: note: candidate expects 1 argument, 0 provided
../src/ndnSIM/NFD/daemon/table/name-tree-entry.hpp:185:7: note: constexpr nfd::name_tree::GetTableEntry<nfd::fib::Entry>::GetTableEntry(const nfd::name_tree::GetTableEntry<nfd::fib::Entry>&)
class GetTableEntry
^
../src/ndnSIM/NFD/daemon/table/name-tree-entry.hpp:185:7: note: candidate expects 1 argument, 0 provided
../src/ndnSIM/NFD/daemon/table/name-tree-entry.hpp:185:7: note: constexpr nfd::name_tree::GetTableEntry<nfd::fib::Entry>::GetTableEntry(nfd::name_tree::GetTableEntry<nfd::fib::Entry>&&)
../src/ndnSIM/NFD/daemon/table/name-tree-entry.hpp:185:7: note: candidate expects 1 argument, 0 provided
Is it gcc being stupid?
Updated by Alex Afanasyev over 7 years ago
Here is a workaround I'm pushing ndnSIM-specific branch of NFD: https://gerrit.named-data.net/#/c/3429/
The workaround makes the code compile and I don't expect any side-effects.
Updated by Junxiao Shi over 7 years ago
- Category set to Tables
- Assignee set to Junxiao Shi
- Estimated time set to 2.00 h
I'm able to reproduce the problem by simply writing
Fib::const_iterator it;
inside fib.cpp. Error messages are the same.
I notice that
static_assert(std::is_default_constructible<Fib::const_iterator>::value, "");
cannot catch Fib::const_iterator it is invalid, but
BOOST_CONCEPT_ASSERT((boost::DefaultConstructible<Fib::const_iterator>));
fails correctly.
The implementation of std::is_default_constructible is in /usr/include/c++/4.8/type_traits, and the core part is:
struct __do_is_default_constructible_impl
{
template<typename _Tp, typename = decltype(_Tp())>
static true_type __test(int);
template<typename>
static false_type __test(...);
};
template<typename _Tp>
struct __is_default_constructible_impl
: public __do_is_default_constructible_impl
{
typedef decltype(__test<_Tp>(0)) type;
};
template<typename _Tp>
struct __is_default_constructible_atom
: public __and_<__not_<is_void<_Tp>>,
__is_default_constructible_impl<_Tp>>::type
{ };
The most important piece of this snippet is decltype(_Tp()) on the third line. It checks whether _Tp() is valid.
In our case, it checks whether Fib::const_iterator() is valid.
I changed my expression to
Fib::const_iterator it();
and, surprisingly, this compiles correctly.
The implementation of boost::DefaultConstructible is in /usr/include/boost/concept_check.hpp:
BOOST_concept(DefaultConstructible,(TT))
{
BOOST_CONCEPT_USAGE(DefaultConstructible) {
TT a; // require default constructor
ignore_unused_variable_warning(a);
}
};
It attempts to construct a variable with the equivalent of Fib::const_iterator it, which fails as we have seen.
The latest gcc has same code in type_traits, which indicates this is possibly a compiler bug that has been fixed after gcc48.
Given boost::DefaultConstructible can catch the problem but std::is_default_constructible cannot, my proposal is to use boost::DefaultConstructible on gcc48.
Specifically, I plan to declare macro NFD_ASSERT_DEFAULT_CONSTRUCTIBLE to perform this check, and also NFD_ASSERT_FORWARD_ITERATOR which calls boost::ForwardIterator (SGI standard) + NFD_ASSERT_DEFAULT_CONSTRUCTIBLE. They will be defined in common.hpp.
I agree with note-3 workaround but it should only be applied to affected compilers, and marked as a workaround.
Updated by Davide Pesavento over 7 years ago
You haven't explained why std::is_default_constructible doesn't work.
Updated by Junxiao Shi over 7 years ago
Answer to note-5:
The answer is already in note-4:
Fib::const_iterator it(); and Fib::const_iterator it; should be equivalent, but the first one compiles and the second one doesn't.
std::is_default_constructible tests with Fib::const_iterator() (the first one above), which compiles correctly.
Updated by Alex Afanasyev over 7 years ago
What if we simply add boost's concept check, regardless of the compiler version? This would add a bit more of assurance without creating complications with configure/build scripts.
Updated by Junxiao Shi over 7 years ago
I agree with note-7.
Can you agree with adding the macros indicated at the end of note-4? This would allow us to change assertions in the future without touching too many files.
Also, as indicated in #3599-11, the std::is_default_constructible check in .waf-tools/compiler-features.py is unnecessary on supported platforms, but dropping that was previously rejected because it would be painful to change many files when porting to other platforms.
If we have the macros, can we drop the waf check, given that only a single macro need to be changed when porting to other platforms?
Updated by Junxiao Shi over 7 years ago
- Status changed from In Progress to Code review
- % Done changed from 0 to 100
Updated by Junxiao Shi over 7 years ago
- Status changed from Code review to Closed