[PATCH] D21343: Implement `lcm` and `gcd` from library fundamentals V2

Eric Fiselier via cfe-commits cfe-commits at lists.llvm.org
Tue Jun 14 16:30:49 PDT 2016


EricWF added inline comments.

================
Comment at: include/experimental/numeric:44
@@ +43,3 @@
+
+_LIBCPP_BEGIN_NAMESPACE_EXPERIMENTAL
+
----------------
This should be `_LIBCPP_BEGIN_NAMESPACE_LFTS`.

================
Comment at: include/experimental/numeric:46
@@ +45,3 @@
+
+template <typename _Tp, bool _IsSigned = _VSTD::is_signed<_Tp>::value> struct __abs;
+
----------------
`_VSTD::` is redundant here.

================
Comment at: include/experimental/numeric:61
@@ +60,3 @@
+constexpr common_type_t<_Tp,_Up> _LIBCPP_INLINE_VISIBILITY
+gcd(_Tp __m, _Up __n)
+{
----------------
It might be beneficial to calculate the absolute values during the initial call instead of at every level of recursion, since most of those calls will be redundant.

================
Comment at: include/experimental/numeric:79
@@ +78,2 @@
+
+#endif /* _LIBCPP_EXPERIMENTAL_MAP */
----------------
Copy/Paste error

================
Comment at: test/std/experimental/numeric/numeric.ops.overview/nothing_to_do.pass.cpp:9
@@ +8,3 @@
+//===----------------------------------------------------------------------===//
+
+#include <experimental/numeric>
----------------
Since it includes the header it should have: `// UNSUPPORTED: c++98, c++03, c++11`.


================
Comment at: test/std/experimental/numeric/numeric.ops/nothing_to_do.pass.cpp:9
@@ +8,3 @@
+//===----------------------------------------------------------------------===//
+
+#include <experimental/numeric>
----------------
Since it includes the header it should have: `// UNSUPPORTED: c++98, c++03, c++11`.



http://reviews.llvm.org/D21343





More information about the cfe-commits mailing list