Minor refactor for Range class (facebookincubator#10991)

Summary:
1. Add comments to make it a little bit easier to understand.
2. Correct type for parameter.
3. Remove unused header include.
4. Add `explicit` to single-argument constructor.
5. Use nested namespace declaration.

No functional changes.

Pull Request resolved: facebookincubator#10991

Reviewed By: amitkdutta

Differential Revision: D62584017

Pulled By: kevinwilfong

fbshipit-source-id: 747494c6e9d094346d240131b2a4f3b6aa1db953

velox/common/base/Range.h

@@ -16,12 +16,10 @@
 
 #pragma once
 
-#include <cstddef>
 #include <cstdint>
 #include "velox/common/base/BitUtil.h"
 
-namespace facebook {
-namespace velox {
+namespace facebook::velox {
 
 template <typename T>
 class Range {
@@ -63,12 +61,12 @@ inline bool Range<bool>::operator[](int32_t idx) const {
 template <typename T>
 class WritablePosition {
  public:
-  WritablePosition(uint64_t* pointer, unsigned char bit)
+  WritablePosition(uint64_t* pointer, int8_t bitIndex)
       : pointer_(
             reinterpret_cast<uint64_t>(pointer) |
-            (static_cast<uint64_t>(bit) << 56)) {}
+            (static_cast<uint64_t>(bitIndex) << 56)) {}
 
-  WritablePosition(T* pointer)
+  explicit WritablePosition(T* pointer)
       : pointer_(reinterpret_cast<uint64_t>(pointer)) {}
 
   operator T() const {
@@ -144,8 +142,8 @@ template <>
 inline WritablePosition<bool> MutableRange<bool>::operator[](int32_t idx) {
   int32_t bit = begin_ + idx;
   return WritablePosition<bool>(
-      reinterpret_cast<uint64_t*>(data_) + (bit / 64), bit & 63);
+      reinterpret_cast<uint64_t*>(data_) + (bit / 64),
+      static_cast<int8_t>(bit & 63));
 }
 
-} // namespace velox
-} // namespace facebook
+} // namespace facebook::velox

velox/common/base/tests/RangeTest.cpp

@@ -18,30 +18,35 @@
 
 #include <gtest/gtest.h>
 
-namespace facebook {
-namespace velox {
+namespace facebook::velox {
 
 TEST(RangeTest, ranges) {
   std::vector<uint64_t> bits(10);
   uint64_t* data = &bits[0];
+
   Range<bool> readable(data, 11, 511);
   MutableRange<bool> writable(data, 9, 509);
   Range<uint8_t> readableBytes(&bits[0], 1, 79);
   MutableRange<uint8_t> writableBytes(&bits[0], 0, 80);
-  // Bit 13 appears as bit 2 in readable and as bit 4 in writable.
+
+  // Bit 13 appears as bit 2 in 'readable' and as bit 4 in 'writable'.
   bits::setBit(data, 13);
   EXPECT_TRUE(readable[2]);
   EXPECT_TRUE(writable[4]);
+
+  // Bit 21 appears as bit 11 in 'readable' and as bit 13 in 'writable'.
   writable[13] = true;
   EXPECT_TRUE(readable[11]);
+
   writable[13] = false;
   EXPECT_FALSE(readable[11]);
 
+  // Byte 10 (ie, bit[80~87]), which corresponds to byte 9 in 'readableBytes'
+  // and bit[69~76] in 'readable'.
   writableBytes[10] = 123;
   EXPECT_EQ(readableBytes[9], 123);
   // Bit 80 is set.
   EXPECT_TRUE(readable[69]);
 }
 
-} // namespace velox
-} // namespace facebook
+} // namespace facebook::velox