Review and classify TODOs.

c++/src/capnproto/arena.c++

@@ -62,9 +62,9 @@ SegmentReader* ReaderArena::tryGetSegment(SegmentId id) {
     }
   }
 
-  // TODO:  Lock a mutex so that reading is thread-safe.  Take a reader lock during the first
-  //   lookup, unlock it before calling getSegment(), then take a writer lock to update the map.
-  //   Bleh, lazy initialization is sad.
+  // TODO(someday):  Lock a mutex so that reading is thread-safe.  Take a reader lock during the
+  //   first lookup, unlock it before calling getSegment(), then take a writer lock to update the
+  //   map.  Bleh, lazy initialization is sad.
 
   if (moreSegments != nullptr) {
     auto iter = moreSegments->find(id.value);
@@ -108,8 +108,8 @@ SegmentBuilder* BuilderArena::getSegment(SegmentId id) {
 }
 
 SegmentBuilder* BuilderArena::getSegmentWithAvailable(WordCount minimumAvailable) {
-  // TODO:  Mutex-locking?  Do we want to allow people to build different parts of the same message
-  // in different threads?
+  // TODO(someday):  Mutex-locking?  Do we want to allow people to build different parts of the
+  //   same message in different threads?
 
   if (segment0.getArena() == nullptr) {
     // We're allocating the first segment.
@@ -127,12 +127,12 @@ SegmentBuilder* BuilderArena::getSegmentWithAvailable(WordCount minimumAvailable
     if (moreSegments == nullptr) {
       moreSegments = std::unique_ptr<MultiSegmentState>(new MultiSegmentState());
     } else {
-      // TODO:  Check for available space in more than just the last segment.  We don't want this
-      //   to be O(n), though, so we'll need to maintain some sort of table.  Complicating matters,
-      //   we want SegmentBuilders::allocate() to be fast, so we can't update any such table when
-      //   allocation actually happens.  Instead, we could have a priority queue based on the
-      //   last-known available size, and then re-check the size when we pop segments off it and
-      //   shove them to the back of the queue if they have become too small.
+      // TODO(perf):  Check for available space in more than just the last segment.  We don't
+      //   want this to be O(n), though, so we'll need to maintain some sort of table.  Complicating
+      //   matters, we want SegmentBuilders::allocate() to be fast, so we can't update any such
+      //   table when allocation actually happens.  Instead, we could have a priority queue based
+      //   on the last-known available size, and then re-check the size when we pop segments off it
+      //   and shove them to the back of the queue if they have become too small.
       if (moreSegments->builders.back()->available() >= minimumAvailable) {
         return moreSegments->builders.back().get();
       }

c++/src/capnproto/arena.h

@@ -145,7 +145,7 @@ public:
   // the VALIDATE_INPUT() macro which may throw an exception; if it return normally, the caller
   // will need to continue with default values.
 
-  // TODO:  Methods to deal with bundled capabilities.
+  // TODO(someday):  Methods to deal with bundled capabilities.
 };
 
 class ReaderArena final: public Arena {
@@ -187,7 +187,7 @@ public:
   // portion of each segment, whereas tryGetSegment() returns something that includes
   // not-yet-allocated space.
 
-  // TODO:  Methods to deal with bundled capabilities.
+  // TODO(someday):  Methods to deal with bundled capabilities.
 
   // implements Arena ------------------------------------------------
   SegmentReader* tryGetSegment(SegmentId id) override;
@@ -262,8 +262,8 @@ inline word* SegmentBuilder::allocate(WordCount amount) {
   if (amount > intervalLength(pos, ptr.end())) {
     return nullptr;
   } else {
-    // TODO:  Atomic increment, backtracking if we go over, would make this thread-safe.  How much
-    //   would it cost in the single-threaded case?  Is it free?  Benchmark it.
+    // TODO(someday):  Atomic increment, backtracking if we go over, would make this thread-safe.
+    //   How much would it cost in the single-threaded case?  Is it free?  Benchmark it.
     word* result = pos;
     pos += amount;
     return result;

c++/src/capnproto/benchmark/capnproto-carsales.c++

@@ -73,7 +73,6 @@ void randomCar(Car::Builder car) {
   car.setMake(MAKES[fastRand(sizeof(MAKES) / sizeof(MAKES[0]))]);
   car.setModel(MODELS[fastRand(sizeof(MODELS) / sizeof(MODELS[0]))]);
 
-  // TODO: Color_RANGE or something.
   car.setColor((Color)fastRand((uint)Color::SILVER + 1));
   car.setSeats(2 + fastRand(6));
   car.setDoors(2 + fastRand(3));

c++/src/capnproto/benchmark/capnproto-eval.c++

@@ -29,7 +29,6 @@ namespace benchmark {
 namespace capnp {
 
 int32_t makeExpression(Expression::Builder exp, uint depth) {
-  // TODO:  Operation_RANGE or something.
   exp.setOp((Operation)(fastRand((int)Operation::MODULUS + 1)));
 
   uint32_t left, right;

c++/src/capnproto/benchmark/common.h

@@ -127,13 +127,13 @@ private:
   sem_t semaphore;
 };
 
+// TODO(cleanup):  Use SYSCALL(), get rid of this exception class.
 class OsException: public std::exception {
 public:
   OsException(int error): error(error) {}
   ~OsException() noexcept {}
 
   const char* what() const noexcept override {
-    // TODO:  Use strerror_r or whatever for thread-safety.  Ugh.
     return strerror(error);
   }
 

c++/src/capnproto/compiler/capnpc-capnp.c++

@@ -406,7 +406,6 @@ TextBlob genAnnotation(schema::Annotation::Reader annotation,
   PRECOND(body.which() == schema::Node::Body::ANNOTATION_NODE);
   auto annDecl = body.getAnnotationNode();
 
-  // TODO:  Don't use displayName.
   return text(prefix, "$", nodeName(decl, scope), "(",
               genValue(annDecl.getType(), annotation.getValue(), scope), ")", suffix);
 }

c++/src/capnproto/io.c++

@@ -129,7 +129,7 @@ BufferedOutputStreamWrapper::~BufferedOutputStreamWrapper() {
       try {
         inner.write(buffer.begin(), bufferPos - buffer.begin());
       } catch (...) {
-        // TODO:  Report secondary faults.
+        // TODO(someday):  Report secondary faults.
       }
     } else {
       flush();

c++/src/capnproto/io.h

@@ -27,7 +27,6 @@
 #include <cstddef>
 #include "macros.h"
 #include "type-safety.h"
-#include <utility>  // for std::forward; TODO: pulls in too much stuff just for std::forward.
 
 namespace capnproto {
 
@@ -220,9 +219,6 @@ class AutoCloseFd {
   //
   // If your code is not exception-safe, you should not use AutoCloseFd.  In this case you will
   // have to call close() yourself and handle errors appropriately.
-  //
-  // TODO:  Create a general helper library for reporting/detecting secondary exceptions that
-  //   occurred during unwind of some primary exception.
 
 public:
   inline AutoCloseFd(): fd(-1) {}

c++/src/capnproto/layout-test.c++

@@ -256,7 +256,6 @@ static void checkStruct(StructReader reader) {
   }
 
   {
-    // TODO:  Use valid default value.
     ListReader list = reader.getListField(3 * POINTERS, FieldSize::POINTER, nullptr);
     ASSERT_EQ(5 * ELEMENTS, list.size());
     for (uint i = 0; i < 5; i++) {

c++/src/capnproto/layout.c++

@@ -178,7 +178,7 @@ struct WirePointer {
   CAPNPROTO_ALWAYS_INLINE(bool isNull() const) {
     // If the upper 32 bits are zero, this is a pointer to an empty struct.  We consider that to be
     // our "null" value.
-    // TODO:  Maybe this would be faster, but violates aliasing rules; does it matter?:
+    // TODO(perf):  Maybe this would be faster, but violates aliasing rules; does it matter?:
     //    return *reinterpret_cast<const uint64_t*>(this) == 0;
     return (offsetAndKind.get() == 0) & (upper32Bits == 0);
   }

c++/src/capnproto/layout.h

@@ -52,7 +52,7 @@ class SegmentBuilder;
 // =============================================================================
 
 enum class FieldSize: uint8_t {
-  // TODO:  Rename to FieldLayout or maybe ValueLayout.
+  // TODO(cleanup):  Rename to FieldLayout or maybe ValueLayout.
 
   // Notice that each member of this enum, when representing a list element size, represents a
   // size that is greater than or equal to the previous members, since INLINE_COMPOSITE is used
@@ -271,7 +271,7 @@ class WireValue {
   // Wraps a primitive value as it appears on the wire.  Namely, values are little-endian on the
   // wire, because little-endian is the most common endianness in modern CPUs.
   //
-  // TODO:  On big-endian systems, inject byte-swapping here.  Most big-endian CPUs implement
+  // TODO(soon):  On big-endian systems, inject byte-swapping here.  Most big-endian CPUs implement
   //   dedicated instructions for this, so use those rather than writing a bunch of shifts and
   //   masks.  Note that GCC has e.g. __builtin__bswap32() for this.
   //
@@ -495,7 +495,7 @@ private:
   int nestingLimit;
   // Limits the depth of message structures to guard against stack-overflow-based DoS attacks.
   // Once this reaches zero, further pointers will be pruned.
-  // TODO:  Limit to 8 bits for better alignment?
+  // TODO(perf):  Limit to 8 bits for better alignment?
 
   inline StructReader(SegmentReader* segment, const void* data, const WirePointer* pointers,
                       BitCount dataSize, WirePointerCount pointerCount, BitCount8 bit0Offset,

c++/src/capnproto/logging.c++

@@ -130,8 +130,8 @@ static Array<char> makeDescription(DescriptionStyle style, const char* code, int
       sysErrorArray = arrayPtr(strerror_r(errorNumber, buffer, sizeof(buffer)));
     }
 #else
-    // TODO:  Other unixes should have strerror_r but it may have a different signature.  Port for
-    //   thread-safety.
+    // TODO(port):  Other unixes should have strerror_r but it may have a different signature.
+    //   Port for thread-safety.
     sysErrorArray = arrayPtr(strerror(errorNumber));
 #endif
 

c++/src/capnproto/message-test.c++

@@ -46,7 +46,7 @@ TEST(Message, MallocBuilderWithFirstSegment) {
   EXPECT_EQ(16u, segment.size());
 }
 
-// TODO:  More tests.
+// TODO(test):  More tests.
 
 }  // namespace
 }  // namespace internal

c++/src/capnproto/schema.capnp

@@ -235,10 +235,6 @@ struct StructNode {
     # Fields of this union, ordered by ordinal.  Currently all members are fields, but
     # consumers should skip member types that they don't understand.  The first member in this list
     # gets discriminant value zero, the next gets one, and so on.
-    #
-    # TODO(soon):  Discriminant zero should be reserved to mean "unset", unless the first field in
-    #   the union actually predates the union (it was retroactively unionized), in which case it
-    #   gets discriminant zero.
   }
 }
 

c++/src/capnproto/serialize-packed-test.c++

@@ -543,7 +543,7 @@ TEST(Packed, RoundTripHugeStringEvenSegmentCountLazy) {
   EXPECT_TRUE(reader.getRoot<TestAllTypes>().getTextField() == std::string(5023, 'x'));
 }
 
-// TODO:  Test error cases.
+// TODO(test):  Test error cases.
 
 }  // namespace
 }  // namespace internal

c++/src/capnproto/serialize-packed.c++

@@ -382,7 +382,7 @@ void PackedOutputStream::write(const void* src, size_t size) {
       // Count the number of consecutive words in the input which have no more than a single
       // zero-byte.  We look for at least two zeros because that's the point where our compression
       // scheme becomes a net win.
-      // TODO:  Maybe look for three zeros?  Compressing a two-zero word is a loss if the
+      // TODO(perf):  Maybe look for three zeros?  Compressing a two-zero word is a loss if the
       //   following word has no zeros.
       const uint8_t* runStart = in;
 

c++/src/capnproto/serialize-snappy-test.c++

@@ -253,7 +253,7 @@ TEST(Snappy, RoundTripTwoMessages) {
   EXPECT_TRUE(pipe.allRead());
 }
 
-// TODO:  Test error cases.
+// TODO(test):  Test error cases.
 
 }  // namespace
 }  // namespace internal

c++/src/capnproto/serialize-snappy.c++

@@ -144,7 +144,7 @@ SnappyOutputStream::~SnappyOutputStream() {
       try {
         flush();
       } catch (...) {
-        // TODO: report secondary faults
+        // TODO(someday): report secondary faults
       }
     } else {
       flush();

c++/src/capnproto/serialize-test.c++

@@ -324,7 +324,7 @@ TEST(Serialize, RejectHugeMessage) {
   }
 }
 
-// TODO:  Test error cases.
+// TODO(test):  Test error cases.
 
 }  // namespace
 }  // namespace internal

c++/src/capnproto/serialize.c++

@@ -167,7 +167,8 @@ InputStreamMessageReader::InputStreamMessageReader(
   }
 
   if (scratchSpace.size() < totalWords) {
-    // TODO:  Consider allocating each segment as a separate chunk to reduce memory fragmentation.
+    // TODO(perf):  Consider allocating each segment as a separate chunk to reduce memory
+    //   fragmentation.
     ownedSpace = newArray<word>(totalWords);
     scratchSpace = ownedSpace;
   }
@@ -203,7 +204,7 @@ InputStreamMessageReader::~InputStreamMessageReader() {
       try {
         inputStream.skip(allEnd - readPos);
       } catch (...) {
-        // TODO:  Devise some way to report secondary errors during unwind.
+        // TODO(someday):  Devise some way to report secondary errors during unwind.
       }
     } else {
       inputStream.skip(allEnd - readPos);

c++/src/capnproto/type-safety.h

@@ -987,9 +987,9 @@ namespace internal { class BitLabel; class ElementLabel; class WirePointer; }
 // also change symbol names, so it's important that the Cap'n proto library and any clients be
 // compiled with the same setting here.
 //
-// TODO:  Decide policy on this.  It may make sense to only use CAPNPROTO_DEBUG_TYPES when compiling
-//   Cap'n Proto's own tests, but disable it for all real builds, as clients may find this safety
-//   tiring.
+// TODO(soon):  Decide policy on this.  It may make sense to only use CAPNPROTO_DEBUG_TYPES when
+//   compiling Cap'n Proto's own tests, but disable it for all real builds, as clients may find
+//   this safety tiring.  Also, need to benchmark to verify there really is no perf hit.
 
 #endif
 

c++/src/capnproto/util.h

@@ -40,7 +40,7 @@ namespace capnproto {
 // =======================================================================================
 // Arrays
 
-// TODO:  Move Array here?
+// TODO(cleanup):  Move these elsewhere, maybe an array.h.
 
 template <typename T, size_t fixedSize>
 class FixedArray {