Various tweaks.

c++/src/capnp/compiler/capnpc2.c++

@@ -33,7 +33,7 @@
 
 class CoutErrorReporter: public capnp::compiler::ErrorReporter {
 public:
-  void addError(uint32_t startByte, uint32_t endByte, kj::String message) override {
+  void addError(uint32_t startByte, uint32_t endByte, kj::StringPtr message) override {
     std::cout << "input:" << startByte << "-" << endByte << ": " << message.cStr() << std::endl;
   }
 };

c++/src/capnp/compiler/error-reporter.h

@@ -34,7 +34,7 @@ class ErrorReporter {
 public:
   virtual ~ErrorReporter() noexcept(false);
 
-  virtual void addError(uint32_t startByte, uint32_t endByte, kj::String message) = 0;
+  virtual void addError(uint32_t startByte, uint32_t endByte, kj::StringPtr message) = 0;
   // Report an error at the given location in the input text.  `startByte` and `endByte` indicate
   // the span of text that is erroneous.  They may be equal, in which case the parser was only
   // able to identify where the error begins, not where it ends.

c++/src/capnp/compiler/parser.c++

@@ -66,7 +66,7 @@ void parseFile(List<Statement>::Reader statements, ParsedFile::Builder result,
         case Declaration::Body::NAKED_ID:
           if (fileDecl.getId().which() == Declaration::Id::UID) {
             errorReporter.addError(builder.getStartByte(), builder.getEndByte(),
-                                   kj::str("File can only have one ID."));
+                                   "File can only have one ID.");
           } else {
             fileDecl.getId().adoptUid(body.disownNakedId());
             if (builder.hasDocComment()) {
@@ -219,20 +219,17 @@ public:
         if (best < item.end()) {
           // Report error from the point where parsing failed to the end of the item.
           errorReporter.addError(
-              best->getStartByte(), (item.end() - 1)->getEndByte(),
-              kj::str("Parse error."));
+              best->getStartByte(), (item.end() - 1)->getEndByte(), "Parse error.");
         } else if (item.size() > 0) {
           // The item is non-empty and the parser consumed all of it before failing.  Report an
           // error for the whole thing.
           errorReporter.addError(
-              item.begin()->getStartByte(), (item.end() - 1)->getEndByte(),
-              kj::str("Parse error."));
+              item.begin()->getStartByte(), (item.end() - 1)->getEndByte(), "Parse error.");
         } else {
           // The item has no content.
           // TODO(cleanup):  We don't actually know the item's location, so we can only report
           //   an error across the whole list.  Fix this.
-          errorReporter.addError(items.startByte, items.endByte,
-              kj::str("Parse error: Empty list item."));
+          errorReporter.addError(items.startByte, items.endByte, "Parse error: Empty list item.");
         }
       }
     }
@@ -412,7 +409,7 @@ CapnpParser::CapnpParser(Orphanage orphanageParam, ErrorReporter& errorReporterP
             } else {
               auto fieldValue = field->get().getValue();
               errorReporter.addError(fieldValue.getStartByte(), fieldValue.getEndByte(),
-                                     kj::str("Missing field name."));
+                                     "Missing field name.");
             }
           }
         }
@@ -511,7 +508,7 @@ CapnpParser::CapnpParser(Orphanage orphanageParam, ErrorReporter& errorReporterP
                 } else {
                   auto fieldValue = field->get().getValue();
                   errorReporter.addError(fieldValue.getStartByte(), fieldValue.getEndByte(),
-                                         kj::str("Missing field name."));
+                                         "Missing field name.");
                 }
               }
             }
@@ -541,7 +538,7 @@ CapnpParser::CapnpParser(Orphanage orphanageParam, ErrorReporter& errorReporterP
       [this](Located<uint64_t>&& value) {
         if (value.value < (1ull << 63)) {
           errorReporter.addError(value.startByte, value.endByte,
-              kj::str("Invalid ID.  Please generate a new one with 'capnpc -i'."));
+              "Invalid ID.  Please generate a new one with 'capnpc -i'.");
         }
         return value.asProto<LocatedInteger>(orphanage);
       }));
@@ -551,7 +548,7 @@ CapnpParser::CapnpParser(Orphanage orphanageParam, ErrorReporter& errorReporterP
       [this](Located<uint64_t>&& value) {
         if (value.value >= 65536) {
           errorReporter.addError(value.startByte, value.endByte,
-              kj::str("Ordinals cannot be greater than 65535."));
+              "Ordinals cannot be greater than 65535.");
         }
         return value.asProto<LocatedInteger>(orphanage);
       }));
@@ -778,7 +775,7 @@ CapnpParser::CapnpParser(Orphanage orphanageParam, ErrorReporter& errorReporterP
               // Set all.
               if (targets.value.size() > 1) {
                 errorReporter.addError(target->startByte, target->endByte,
-                    kj::str("Wildcard should not be specified together with other targets."));
+                    "Wildcard should not be specified together with other targets.");
               }
 
               for (auto member: dynamicBuilder.getSchema().getMembers()) {
@@ -790,7 +787,7 @@ CapnpParser::CapnpParser(Orphanage orphanageParam, ErrorReporter& errorReporterP
               if (target->value.size() == 0 || target->value.size() >= 32 ||
                   target->value[0] < 'a' || target->value[0] > 'z') {
                 errorReporter.addError(target->startByte, target->endByte,
-                    kj::str("Not a valid annotation target."));
+                                       "Not a valid annotation target.");
               } else {
                 char buffer[64];
                 strcpy(buffer, "targets");
@@ -799,12 +796,12 @@ CapnpParser::CapnpParser(Orphanage orphanageParam, ErrorReporter& errorReporterP
                 KJ_IF_MAYBE(member, dynamicBuilder.getSchema().findMemberByName(buffer)) {
                   if (dynamicBuilder.get(*member).as<bool>()) {
                     errorReporter.addError(target->startByte, target->endByte,
-                        kj::str("Duplicate target specification."));
+                                           "Duplicate target specification.");
                   }
                   dynamicBuilder.set(*member, true);
                 } else {
                   errorReporter.addError(target->startByte, target->endByte,
-                      kj::str("Not a valid annotation target."));
+                                         "Not a valid annotation target.");
                 }
               }
             }
@@ -866,7 +863,7 @@ kj::Maybe<Orphan<Declaration>> CapnpParser::parseStatement(
       case Statement::Block::NONE:
         if (output->memberParser != nullptr) {
           errorReporter.addError(statement.getStartByte(), statement.getEndByte(),
-              kj::str("This statement should end with a semicolon, not a block."));
+              "This statement should end with a semicolon, not a block.");
         }
         break;
 
@@ -882,7 +879,7 @@ kj::Maybe<Orphan<Declaration>> CapnpParser::parseStatement(
           builder.adoptNestedDecls(arrayToList(orphanage, members.releaseAsArray()));
         } else {
           errorReporter.addError(statement.getStartByte(), statement.getEndByte(),
-              kj::str("This statement should end with a block, not a semicolon."));
+              "This statement should end with a block, not a semicolon.");
         }
         break;
     }
@@ -902,7 +899,7 @@ kj::Maybe<Orphan<Declaration>> CapnpParser::parseStatement(
       bestByte = 0;
     }
 
-    errorReporter.addError(bestByte, bestByte, kj::str("Parse error."));
+    errorReporter.addError(bestByte, bestByte, "Parse error.");
     return nullptr;
   }
 }

c++/src/capnp/schema-loader-test.c++

@@ -263,7 +263,7 @@ public:
     EXPECT_EQ(node.getId(), id);
     EXPECT_FALSE(loaded);
     loaded = true;
-    loader.loadIfNew(node);
+    loader.loadOnce(node);
   }
 
 private:

c++/src/capnp/schema-loader.c++

@@ -1213,7 +1213,7 @@ Schema SchemaLoader::load(const schema::Node::Reader& reader) {
   return Schema(impl.lock()->get()->load(reader, false));
 }
 
-Schema SchemaLoader::loadIfNew(const schema::Node::Reader& reader) const {
+Schema SchemaLoader::loadOnce(const schema::Node::Reader& reader) const {
   auto locked = impl.lock();
   auto getResult = locked->get()->tryGet(reader.getId());
   if (getResult.schema == nullptr || getResult.schema->lazyInitializer != nullptr) {

c++/src/capnp/schema-loader.h

@@ -36,7 +36,7 @@ public:
   public:
     virtual void load(const SchemaLoader& loader, uint64_t id) const = 0;
     // Request that the schema node with the given ID be loaded into the given SchemaLoader.  If
-    // the callback is able to find a schema for this ID, it should invoke `loadIfNew()` on
+    // the callback is able to find a schema for this ID, it should invoke `loadOnce()` on
     // `loader` to load it.  If no such node exists, it should simply do nothing and return.
     //
     // The callback is allowed to load schema nodes other than the one requested, e.g. because it
@@ -101,10 +101,12 @@ public:
   // Also note that unknown types are not considered invalid.  Instead, the dynamic API returns
   // a DynamicValue with type UNKNOWN for these.
 
-  Schema loadIfNew(const schema::Node::Reader& reader) const;
+  Schema loadOnce(const schema::Node::Reader& reader) const;
   // Like `load()` but does nothing if a schema with the same ID is already loaded.  In contrast,
-  // `load()` would attempt to compare the schemas and take the newer one.  `loadIfNew()` is safe
-  // to call even while concurrently using schemas from this loader.
+  // `load()` would attempt to compare the schemas and take the newer one.  `loadOnce()` is safe
+  // to call even while concurrently using schemas from this loader.  It should be considered an
+  // error to call `loadOnce()` with two non-identical schemas that share the same ID, although
+  // this error may or may not actually be detected by the implementation.
 
   template <typename T>
   void loadCompiledTypeAndDependencies();

c++/src/kj/common.h

@@ -732,8 +732,7 @@ public:
   inline ~Deferred() { if (!canceled) func(); }
   KJ_DISALLOW_COPY(Deferred);
 
-  // This move constructor is optimized away by the compiler in practice.  It is only here for
-  // technical correctness.
+  // This move constructor is usually optimized away by the compiler.
   inline Deferred(Deferred&& other): func(kj::mv(other.func)) {
     other.canceled = true;
   }
@@ -742,14 +741,22 @@ private:
   bool canceled;
 };
 
+}  // namespace _ (private)
+
 template <typename Func>
-Deferred<Decay<Func>> defer(Func&& func) {
-  return Deferred<Decay<Func>>(kj::fwd<Func>(func));
-}
+_::Deferred<Decay<Func>> defer(Func&& func) {
+  // Returns an object which will invoke the given functor in its destructor.  The object is not
+  // copyable but is movable with the semantics you'd expect.  Since the return type is private,
+  // you need to assign to an `auto` variable.
+  //
+  // The KJ_DEFER macro provides slightly more convenient syntax for the common case where you
+  // want some code to run at function exit.
 
-}  // namespace _ (private)
+  return _::Deferred<Decay<Func>>(kj::fwd<Func>(func));
+}
 
-#define KJ_DEFER(code) auto KJ_UNIQUE_NAME(_kjDefer) = ::kj::_::defer([&](){code;})
+#define KJ_DEFER(code) auto KJ_UNIQUE_NAME(_kjDefer) = ::kj::defer([&](){code;})
+// Run the given code when the function exits, whether by return or exception.
 
 }  // namespace kj
 

c++/src/kj/mutex.h

@@ -153,6 +153,11 @@ public:
   // Lock the value for read-only access.  Multiple read-only locks can be taken concurrently, as
   // long as there are no writers.
 
+  inline const T& getWithoutLock() const { return value; }
+  inline T& getWithoutLock() { return value; }
+  // Escape hatch for cases where some external factor guarantees that it's safe to get the
+  // value.  You should treat these like const_cast -- be highly suspicious of any use.
+
 private:
   mutable _::Mutex mutex;
   mutable T value;