Fix compiler bugs. Now test.capnp compiles to an identical schema to the old…

Fix compiler bugs.  Now test.capnp compiles to an identical schema to the old Haskell compiler, minus intentional changes.

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

@@ -453,8 +453,8 @@ TextBlob genStructMember(schema::StructNode::Member::Reader member,
       int i = 0;
       return text(indent, member.getName(), " @", member.getOrdinal(),
                   " union", genAnnotations(member.getAnnotations(), scope),
-                  " {  # tag bits[", un.getDiscriminantOffset(), ", ",
-                  un.getDiscriminantOffset() + 16, ")\n",
+                  " {  # tag bits[", un.getDiscriminantOffset() * 16, ", ",
+                  un.getDiscriminantOffset() * 16 + 16, ")\n",
                   FOR_EACH(un.getMembers(), member) {
                     return genStructMember(member, scope, indent.next(), i++);
                   },

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

@@ -63,10 +63,10 @@ public:
   }
 };
 
-class CompilerMain {
+class CompilerMain final: public GlobalErrorReporter {
 public:
   explicit CompilerMain(kj::ProcessContext& context)
-      : context(context), loader(STDERR_FILENO) {}
+      : context(context), loader(*this) {}
 
   kj::MainFunc getMain() {
     return kj::MainBuilder(
@@ -111,6 +111,11 @@ public:
   }
 
   kj::MainBuilder::Validity generateOutput() {
+    if (hadErrors()) {
+      // Skip output if we had any errors.
+      return true;
+    }
+
     if (outputs.size() == 0) {
       return "no outputs specified";
     }
@@ -185,6 +190,30 @@ public:
     return true;
   }
 
+  void addError(kj::StringPtr file, SourcePos start, SourcePos end,
+                kj::StringPtr message) const override {
+    kj::String wholeMessage;
+    if (end.line == start.line) {
+      if (end.column == start.column) {
+        wholeMessage = kj::str(file, ":", start.line + 1, ":", start.column + 1,
+                               ": error: ", message, "\n");
+      } else {
+        wholeMessage = kj::str(file, ":", start.line + 1, ":", start.column + 1,
+                               "-", end.column + 1, ": error: ", message, "\n");
+      }
+    } else {
+      // The error spans multiple lines, so just report it on the first such line.
+      wholeMessage = kj::str(file, ":", start.line + 1, ": error: ", message, "\n");
+    }
+
+    context.error(wholeMessage);
+    __atomic_store_n(&hadErrors_, true, __ATOMIC_RELAXED);
+  }
+
+  bool hadErrors() const override {
+    return __atomic_load_n(&hadErrors_, __ATOMIC_RELAXED);
+  }
+
 private:
   kj::ProcessContext& context;
   ModuleLoader loader;
@@ -197,6 +226,8 @@ private:
     kj::StringPtr dir;
   };
   kj::Vector<OutputDirective> outputs;
+
+  mutable bool hadErrors_ = false;
 };
 
 }  // namespace compiler

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

@@ -82,7 +82,7 @@ public:
   // Resolve an arbitrary DeclName to a Node.
 
   kj::Maybe<Schema> getBootstrapSchema() const;
-  kj::Maybe<Schema> getFinalSchema() const;
+  kj::Maybe<schema::Node::Reader> getFinalSchema() const;
 
   void traverse(Compiler::Mode mode) const;
   // Get the final schema for this node, and also possibly traverse the node's children and
@@ -94,7 +94,7 @@ public:
   // implements NodeTranslator::Resolver -----------------------------
   kj::Maybe<ResolvedName> resolve(const DeclName::Reader& name) const override;
   kj::Maybe<Schema> resolveBootstrapSchema(uint64_t id) const override;
-  kj::Maybe<Schema> resolveFinalSchema(uint64_t id) const override;
+  kj::Maybe<schema::Node::Reader> resolveFinalSchema(uint64_t id) const override;
 
 private:
   const CompiledModule* module;  // null iff isBuiltin is true
@@ -249,8 +249,12 @@ public:
   const SchemaLoader& getFinalLoader() const { return finalLoader; }
   // Schema loader containing final versions of schemas.
 
-  const Workspace& getWorkspace() const { return *workspace.getWithoutLock(); }
-  // Temporary workspace that can be used to construct bootstrap objects.
+  const Workspace& getWorkspace() const { return workspace.getWithoutLock(); }
+  // Temporary workspace that can be used to construct bootstrap objects.  We assume that the
+  // caller already holds the workspace lock somewhere up the stack.
+
+  void clearWorkspace();
+  // Reset the temporary workspace.
 
 
   uint64_t addNode(uint64_t desiredId, Node& node) const;
@@ -271,10 +275,8 @@ private:
   SchemaLoader finalLoader;
   // The loader where we put final output of the compiler.
 
-  kj::MutexGuarded<const Workspace*> workspace;
-  // The entry points to the Compiler allocate a Workspace on the stack, lock this mutex, and set
-  // the pointer to point at said stack workspace.  The rest of the compiler can assume that this
-  // Workspace is active.
+  kj::MutexGuarded<Workspace> workspace;
+  // The temporary workspace.
 
   typedef std::unordered_map<const Module*, kj::Own<CompiledModule>> ModuleMap;
   kj::MutexGuarded<ModuleMap> modules;
@@ -477,9 +479,13 @@ const Compiler::Node::Content& Compiler::Node::getContent(Content::State minimum
             locked->translator->getBootstrapNode());
       })) {
         locked->bootstrapSchema = nullptr;
+        // Only bother to report validation failures if we think we haven't seen any errors.
+        // Otherwise we assume that the errors caused the validation failure.
+        if (!module->getErrorReporter().hadErrors()) {
           addError(kj::str("Internal compiler bug: Bootstrap schema failed validation:\n",
                            *exception));
         }
+      }
 
       // If the Workspace is destroyed while this Node is still in the BOOTSTRAP state,
       // revert it to the EXPANDED state, because the NodeTranslator is no longer valid in this
@@ -505,9 +511,14 @@ const Compiler::Node::Content& Compiler::Node::getContent(Content::State minimum
             locked->translator->finish());
       })) {
         locked->finalSchema = nullptr;
+
+        // Only bother to report validation failures if we think we haven't seen any errors.
+        // Otherwise we assume that the errors caused the validation failure.
+        if (!module->getErrorReporter().hadErrors()) {
           addError(kj::str("Internal compiler bug: Schema failed validation:\n",
                            *exception));
         }
+      }
 
       locked->advanceState(Content::FINISHED);
       // no break
@@ -627,8 +638,9 @@ kj::Maybe<Schema> Compiler::Node::getBootstrapSchema() const {
     return content.bootstrapSchema;
   }
 }
-kj::Maybe<Schema> Compiler::Node::getFinalSchema() const {
-  return getContent(Content::FINISHED).finalSchema;
+kj::Maybe<schema::Node::Reader> Compiler::Node::getFinalSchema() const {
+  return getContent(Content::FINISHED).finalSchema.map(
+      [](const Schema& schema) { return schema.getProto(); });
 }
 
 void Compiler::Node::traverse(Compiler::Mode mode) const {
@@ -660,7 +672,7 @@ kj::Maybe<Schema> Compiler::Node::resolveBootstrapSchema(uint64_t id) const {
   }
 }
 
-kj::Maybe<Schema> Compiler::Node::resolveFinalSchema(uint64_t id) const {
+kj::Maybe<schema::Node::Reader> Compiler::Node::resolveFinalSchema(uint64_t id) const {
   KJ_IF_MAYBE(node, module->getCompiler().findNode(id)) {
     return node->getFinalSchema();
   } else {
@@ -686,7 +698,7 @@ kj::Maybe<const Compiler::CompiledModule&> Compiler::CompiledModule::importRelat
 
 // =======================================================================================
 
-Compiler::Impl::Impl(): finalLoader(*this), workspace(nullptr) {
+Compiler::Impl::Impl(): finalLoader(*this), workspace(*this) {
   // Reflectively interpret the members of Declaration.body.  Any member prefixed by "builtin"
   // defines a builtin declaration visible in the global scope.
   StructSchema::Union declBodySchema =
@@ -703,6 +715,14 @@ Compiler::Impl::Impl(): finalLoader(*this), workspace(nullptr) {
 
 Compiler::Impl::~Impl() {}
 
+void Compiler::Impl::clearWorkspace() {
+  auto lock = workspace.lockExclusive();
+
+  // Make sure we reconstruct the workspace even if destroying it throws an exception.
+  KJ_DEFER(kj::ctor(*lock, *this));
+  kj::dtor(*lock);
+}
+
 const Compiler::CompiledModule& Compiler::Impl::add(const Module& parsedModule) const {
   auto locked = modules.lockExclusive();
 
@@ -756,11 +776,7 @@ kj::Maybe<const Compiler::Node&> Compiler::Impl::lookupBuiltin(kj::StringPtr nam
 }
 
 uint64_t Compiler::Impl::add(const Module& module, Mode mode) const {
-  Impl::Workspace workspace(*this);
-  auto lock = this->workspace.lockExclusive();
-  *lock = &workspace;
-  KJ_DEFER(*lock = nullptr);
-
+  auto lock = this->workspace.lockShared();
   auto& node = add(module).getRootNode();
   if (mode != LAZY) {
     node.traverse(mode);
@@ -771,10 +787,7 @@ uint64_t Compiler::Impl::add(const Module& module, Mode mode) const {
 void Compiler::Impl::load(const SchemaLoader& loader, uint64_t id) const {
   KJ_IF_MAYBE(node, findNode(id)) {
     if (&loader == &finalLoader) {
-      Workspace workspace(*this);
-      auto lock = this->workspace.lockExclusive();
-      *lock = &workspace;
-      KJ_DEFER(*lock = nullptr);
+      auto lock = this->workspace.lockShared();
       node->getFinalSchema();
     } else {
       // Must be the bootstrap loader.
@@ -796,5 +809,9 @@ const SchemaLoader& Compiler::getLoader() const {
   return impl->getFinalLoader();
 }
 
+void Compiler::clearWorkspace() {
+  impl->clearWorkspace();
+}
+
 }  // namespace compiler
 }  // namespace capnp

c++/src/capnp/compiler/compiler.h

@@ -86,6 +86,15 @@ public:
   // Get a SchemaLoader backed by this compiler.  Schema nodes will be lazily constructed as you
   // traverse them using this loader.
 
+  void clearWorkspace();
+  // The compiler builds a lot of temporary tables and data structures while it works.  It's
+  // useful to keep these around if more work is expected (especially if you are using lazy
+  // compilation and plan to look up Schema nodes that haven't already been seen), but once
+  // the SchemaLoader has everything you need, you can call clearWorkspace() to free up the
+  // temporary space.  Note that it's safe to call clearWorkspace() even if you do expect to
+  // compile more nodes in the future; it may simply lead to redundant work if the discarded
+  // structures are needed again.
+
 private:
   class Impl;
   kj::Own<Impl> impl;

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

@@ -26,11 +26,14 @@
 
 #include "../common.h"
 #include <kj/string.h>
+#include <kj/exception.h>
 
 namespace capnp {
 namespace compiler {
 
 class ErrorReporter {
+  // Callback for reporting errors within a particular file.
+
 public:
   virtual ~ErrorReporter() noexcept(false);
 
@@ -46,6 +49,33 @@ public:
 
     addError(decl.getStartByte(), decl.getEndByte(), message);
   }
+
+  virtual bool hadErrors() const = 0;
+  // Return true if any errors have been reported, globally.  The main use case for this callback
+  // is to inhibit the reporting of errors which may have been caused by previous errors, or to
+  // allow the compiler to bail out entirely if it gets confused and thinks this could be because
+  // of previous errors.
+};
+
+class GlobalErrorReporter {
+  // Callback for reporting errors in any file.
+
+public:
+  struct SourcePos {
+    uint byte;
+    uint line;
+    uint column;
+  };
+
+  virtual void addError(kj::StringPtr file, SourcePos start, SourcePos end,
+                        kj::StringPtr message) const = 0;
+  // Report an error at the given location in the given file.
+
+  virtual bool hadErrors() const = 0;
+  // Return true if any errors have been reported, globally.  The main use case for this callback
+  // is to inhibit the reporting of errors which may have been caused by previous errors, or to
+  // allow the compiler to bail out entirely if it gets confused and thinks this could be because
+  // of previous errors.
 };
 
 }  // namespace compiler

c++/src/capnp/compiler/lexer-test.c++

@@ -34,6 +34,11 @@ public:
   void addError(uint32_t startByte, uint32_t endByte, kj::StringPtr message) const override {
     ADD_FAILURE() << "Parse failed: (" << startByte << "-" << endByte << ") " << message.cStr();
   }
+
+  bool hadErrors() const override {
+    // Not used by lexer.
+    return false;
+  }
 };
 
 template <typename LexResult>

c++/src/capnp/compiler/module-loader.c++

@@ -193,7 +193,7 @@ kj::String catPath(kj::StringPtr base, kj::StringPtr add) {
 
 class ModuleLoader::Impl {
 public:
-  Impl(int errorFd): errorFd(errorFd) {}
+  Impl(const GlobalErrorReporter& errorReporter): errorReporter(errorReporter) {}
 
   void addImportPath(kj::String path) {
     searchPath.add(kj::heapString(kj::mv(path)));
@@ -201,10 +201,10 @@ public:
 
   kj::Maybe<const Module&> loadModule(kj::StringPtr localName, kj::StringPtr sourceName) const;
   kj::Maybe<const Module&> loadModuleFromSearchPath(kj::StringPtr sourceName) const;
-  void writeError(kj::StringPtr content) const;
+  const GlobalErrorReporter& getErrorReporter() const { return errorReporter; }
 
 private:
-  int errorFd;
+  const GlobalErrorReporter& errorReporter;
   kj::Vector<kj::String> searchPath;
   kj::MutexGuarded<std::map<kj::StringPtr, kj::Own<Module>>> modules;
 };
@@ -260,10 +260,21 @@ public:
           return space.construct();
         });
 
+    // TODO(someday):  This counts tabs as single characters.  Do we care?
     uint startLine = findLargestElementBefore(lines, startByte);
     uint startCol = startByte - lines[startLine];
-    loader.writeError(
-        kj::str(localName, ":", startLine + 1, ":", startCol + 1, ": error: ", message, "\n"));
+    uint endLine = findLargestElementBefore(lines, endByte);
+    uint endCol = endByte - lines[endLine];
+
+    loader.getErrorReporter().addError(
+        localName,
+        GlobalErrorReporter::SourcePos { startByte, startLine, startCol },
+        GlobalErrorReporter::SourcePos { endByte, endLine, endCol },
+        message);
+  }
+
+  bool hadErrors() const {
+    return loader.getErrorReporter().hadErrors();
   }
 
 private:
@@ -317,18 +328,10 @@ kj::Maybe<const Module&> ModuleLoader::Impl::loadModuleFromSearchPath(
   return nullptr;
 }
 
-void ModuleLoader::Impl::writeError(kj::StringPtr content) const {
-  const char* pos = content.begin();
-  while (pos < content.end()) {
-    ssize_t n;
-    KJ_SYSCALL(n = write(errorFd, pos, content.end() - pos));
-    pos += n;
-  }
-}
-
 // =======================================================================================
 
-ModuleLoader::ModuleLoader(int errorFd): impl(kj::heap<Impl>(errorFd)) {}
+ModuleLoader::ModuleLoader(const GlobalErrorReporter& errorReporter)
+    : impl(kj::heap<Impl>(errorReporter)) {}
 ModuleLoader::~ModuleLoader() {}
 
 void ModuleLoader::addImportPath(kj::String path) { impl->addImportPath(kj::mv(path)); }

c++/src/capnp/compiler/module-loader.h

@@ -25,6 +25,7 @@
 #define CAPNP_COMPILER_MODULE_LOADER_H_
 
 #include "compiler.h"
+#include "error-reporter.h"
 #include <kj/memory.h>
 #include <kj/array.h>
 #include <kj/string.h>
@@ -34,8 +35,8 @@ namespace compiler {
 
 class ModuleLoader {
 public:
-  explicit ModuleLoader(int errorFd);
-  // Create a ModuleLoader that writes error messages to the given file descriptor.
+  explicit ModuleLoader(const GlobalErrorReporter& errorReporter);
+  // Create a ModuleLoader that reports error messages to the given reporter.
 
   KJ_DISALLOW_COPY(ModuleLoader);
 

c++/src/capnp/compiler/node-translator.c++

@@ -357,74 +357,20 @@ public:
           if (location.tryExpandTo(group.parent, lgSize)) {
             isUsed = true;
             lgSizeUsed = lgSize;
-            return kj::implicitCast<uint>(0);
+            return location.offset << (location.lgSize - lgSize);
           } else {
             return nullptr;
           }
         } else {
           uint newSize = kj::max(lgSizeUsed, lgSize) + 1;
           if (tryExpandUsage(group, location, newSize)) {
-            return holes.assertHoleAndAllocate(lgSize);
-          } else {
-            return nullptr;
-          }
-        }
-      }
-
-      kj::Maybe<uint> tryAllocate(Group& group, Union::DataLocation& location, uint lgSize) {
-        if (isUsed) {
-          // We've already used some space in this location.  Try to allocate from a hole.
-          uint result;
-          KJ_IF_MAYBE(hole, holes.tryAllocate(lgSize)) {
-            result = *hole;
-          } else {
-            // Failure.  But perhaps we could expand the location to include a new hole which would
-            // be big enough for the value.
-            uint neededSizeUsed;
-            if (lgSize <= lgSizeUsed) {
-              // We are already at least as big as the desired size, so doubling should be good
-              // enough.  The new value will be located just past the end of our current used
-              // space.
-              neededSizeUsed = lgSizeUsed + 1;
-              result = 1 << (lgSizeUsed - lgSize);
-            } else {
-              // We are smaller than the desired size, so we'll have to grow to 2x the desired size.
-              // The new value will be at an offset of 1x its own size.
-              neededSizeUsed = lgSize + 1;
-              result = 1;
-            }
-
-            if (!tryExpandUsage(group, location, neededSizeUsed)) {
-              return nullptr;
-            }
-            holes.addHolesAtEnd(lgSize, result + 1, neededSizeUsed - 1);
-          }
-
-          // OK, we found space.  Adjust the offset according to the location's offset before
-          // returning.
+            uint result = holes.assertHoleAndAllocate(lgSize);
             uint locationOffset = location.offset << (location.lgSize - lgSize);
             return locationOffset + result;
-
           } else {
-          // We haven't used this location at all yet.
-
-          if (location.lgSize < lgSize) {
-            // Not enough space.  Try to expand the location.
-            if (!location.tryExpandTo(group.parent, lgSize)) {
-              // Couldn't expand.  This location is not viable.
             return nullptr;
           }
         }
-
-          // Either the location was already big enough, or we expanded it.
-          KJ_DASSERT(location.lgSize >= lgSize);
-
-          // Just mark the first part used for now.
-          lgSizeUsed = lgSize;
-
-          // Return the offset, adjusted to be appropriate for the size.
-          return location.offset << (location.lgSize - lgSize);
-        }
       }
 
       bool tryExpand(Group& group, Union::DataLocation& location,
@@ -869,7 +815,7 @@ public:
         dupDetector.check(member.decl.getId().getOrdinal());
       }
 
-      schema::StructNode::Member::Builder builder = member.parent->getMemberSchema(member.index);
+      schema::StructNode::Member::Builder builder = member.getSchema();
 
       builder.setName(member.decl.getName().getValue());
       builder.setOrdinal(entry.first);
@@ -966,7 +912,7 @@ public:
     for (auto member: lateUnions) {
       member->unionScope->addDiscriminant();  // if it hasn't happened already
       KJ_IF_MAYBE(offset, member->unionScope->discriminantOffset) {
-        member->parent->getMemberSchema(member->index).getBody().getUnionMember()
+        member->getSchema().getBody().getUnionMember()
             .setDiscriminantOffset(*offset);
       } else {
         KJ_FAIL_ASSERT("addDiscriminant() didn't set the offset?");
@@ -1018,12 +964,17 @@ private:
     uint childCount = 0;
     // Number of children this member has.
 
-    uint index = 0;
+    uint childInitializedCount = 0;
+    // Number of children whose `schema` member has been initialized.  This initialization happens
+    // while walking the fields in ordinal order.
 
     Declaration::Reader decl;
 
     List<schema::StructNode::Member>::Builder memberSchemas;
 
+    kj::Maybe<schema::StructNode::Member::Builder> schema;
+    // Schema for this member, if applicable.  Initialized when getSchema() is first called.
+
     union {
       StructLayout::StructOrGroup* fieldScope;
       // If this member is a field, the scope of that field.  This will be used to assign an
@@ -1046,6 +997,16 @@ private:
                       const Declaration::Reader& decl, StructLayout::Union& unionScope)
         : parent(&parent), codeOrder(codeOrder), decl(decl), unionScope(&unionScope) {}
 
+    schema::StructNode::Member::Builder getSchema() {
+      KJ_IF_MAYBE(result, schema) {
+        return *result;
+      } else {
+        auto builder = parent->getMemberSchema(parent->childInitializedCount++);
+        schema = builder;
+        return builder;
+      }
+    }
+
     schema::StructNode::Member::Builder getMemberSchema(uint childIndex) {
       // Get the schema builder for the child member at the given index.  This lazily/dynamically
       // builds the builder tree.
@@ -1058,11 +1019,11 @@ private:
             KJ_FAIL_ASSERT("Fields don't have members.");
             break;
           case Declaration::Body::UNION_DECL:
-            memberSchemas = parent->getMemberSchema(index).getBody()
+            memberSchemas = getSchema().getBody()
                 .initUnionMember().initMembers(childCount);
             break;
           case Declaration::Body::GROUP_DECL:
-            memberSchemas = parent->getMemberSchema(index).getBody()
+            memberSchemas = getSchema().getBody()
                 .initGroupMember().initMembers(childCount);
             break;
           default:
@@ -1120,7 +1081,7 @@ private:
       }
 
       if (memberInfo != nullptr) {
-        memberInfo->index = parent.childCount++;
+        parent.childCount++;
         membersByOrdinal.insert(std::make_pair(ordinal, memberInfo));
         minOrdinal = kj::min(minOrdinal, ordinal);
       }
@@ -1175,7 +1136,7 @@ private:
       }
 
       if (memberInfo != nullptr) {
-        memberInfo->index = parent.childCount++;
+        parent.childCount++;
         membersByOrdinal.insert(std::make_pair(ordinal, memberInfo));
         minOrdinal = kj::min(minOrdinal, ordinal);
       }
@@ -1329,6 +1290,11 @@ void NodeTranslator::compileDefaultDefaultValue(
 }
 
 class NodeTranslator::DynamicSlot {
+  // Acts like a pointer to a field or list element.  The target's value can be set or initialized.
+  // This is useful when recursively compiling values.
+  //
+  // TODO(someday):  The Dynamic API should support something like this directly.
+
 public:
   DynamicSlot(DynamicStruct::Builder structBuilder, StructSchema::Member member)
       : type(FIELD), structBuilder(structBuilder), member(member) {}
@@ -1713,11 +1679,11 @@ kj::Maybe<DynamicValue::Reader> NodeTranslator::readConstant(
     //
     // We need to be very careful not to query this Schema's dependencies because if it is
     // a final schema then this query could trigger a lazy load which would deadlock.
-    kj::Maybe<Schema> maybeConstSchema = isBootstrap ?
-        resolver.resolveBootstrapSchema(resolved->id) :
+    kj::Maybe<schema::Node::Reader> maybeConstSchema = isBootstrap ?
+        resolver.resolveBootstrapSchema(resolved->id).map([](Schema s) { return s.getProto(); }) :
         resolver.resolveFinalSchema(resolved->id);
     KJ_IF_MAYBE(constSchema, maybeConstSchema) {
-      auto constReader = constSchema->getProto().getBody().getConstNode();
+      auto constReader = constSchema->getBody().getConstNode();
       auto constValue = toDynamic(constReader.getValue()).get("body").as<DynamicUnion>().get();
 
       if (constValue.getType() == DynamicValue::OBJECT) {
@@ -1756,8 +1722,7 @@ kj::Maybe<DynamicValue::Reader> NodeTranslator::readConstant(
         // A fully unqualified identifier looks like it might refer to a constant visible in the
         // current scope, but if that's really what the user wanted, we want them to use a
         // qualified name to make it more obvious.  Report an error.
-        KJ_IF_MAYBE(scope, resolver.resolveBootstrapSchema(
-            constSchema->getProto().getScopeId())) {
+        KJ_IF_MAYBE(scope, resolver.resolveBootstrapSchema(constSchema->getScopeId())) {
           auto scopeReader = scope->getProto();
           kj::StringPtr parent;
           if (scopeReader.getBody().which() == schema::Node::Body::FILE_NODE) {

c++/src/capnp/compiler/node-translator.h

@@ -63,11 +63,11 @@ public:
     // traversing other schemas.  Returns null if the ID is recognized, but the corresponding
     // schema node failed to be built for reasons that were already reported.
 
-    virtual kj::Maybe<Schema> resolveFinalSchema(uint64_t id) const = 0;
-    // Get the final schema for the given ID.  A bootstrap schema is not acceptable.  It is NOT
-    // safe to traverse the schema's dependencies with Schema::getDependency() as doing so may
-    // trigger lazy loading callbacks that deadlock on the compiler mutex.  Instead, the caller
-    // should carefully look up dependencies through this Resolver.
+    virtual kj::Maybe<schema::Node::Reader> resolveFinalSchema(uint64_t id) const = 0;
+    // Get the final schema for the given ID.  A bootstrap schema is not acceptable.  A raw
+    // node reader is returned rather than a Schema object because using a Schema object built
+    // by the final schema loader could trigger lazy initialization of dependencies which could
+    // lead to a cycle and deadlock.
     //
     // Throws an exception if the id is not one that was found by calling resolve() or by
     // traversing other schemas.  Returns null if the ID is recognized, but the corresponding

c++/src/kj/common.h

@@ -393,6 +393,10 @@ private:  // internal interface used by friends only
       : isSet(true) {
     ctor(value, kj::mv(t));
   }
+  inline NullableValue(T& t)
+      : isSet(true) {
+    ctor(value, t);
+  }
   inline NullableValue(const T& t)
       : isSet(true) {
     ctor(value, t);
@@ -437,6 +441,19 @@ private:  // internal interface used by friends only
     return *this;
   }
 
+  inline NullableValue& operator=(NullableValue& other) {
+    if (&other != this) {
+      if (isSet) {
+        dtor(value);
+      }
+      isSet = other.isSet;
+      if (isSet) {
+        ctor(value, other.value);
+      }
+    }
+    return *this;
+  }
+
   inline NullableValue& operator=(const NullableValue& other) {
     if (&other != this) {
       if (isSet) {
@@ -488,6 +505,7 @@ class Maybe {
 public:
   Maybe(): ptr(nullptr) {}
   Maybe(T&& t) noexcept(noexcept(T(instance<T&&>()))): ptr(kj::mv(t)) {}
+  Maybe(T& t): ptr(t) {}
   Maybe(const T& t): ptr(t) {}
   Maybe(const T* t) noexcept: ptr(t) {}
   Maybe(Maybe&& other) noexcept(noexcept(T(instance<T&&>()))): ptr(kj::mv(other.ptr)) {}
@@ -509,6 +527,7 @@ public:
   Maybe(decltype(nullptr)) noexcept: ptr(nullptr) {}
 
   inline Maybe& operator=(Maybe&& other) { ptr = kj::mv(other.ptr); return *this; }
+  inline Maybe& operator=(Maybe& other) { ptr = other.ptr; return *this; }
   inline Maybe& operator=(const Maybe& other) { ptr = other.ptr; return *this; }
 
   inline bool operator==(decltype(nullptr)) const { return ptr == nullptr; }

c++/src/kj/main.c++

@@ -30,6 +30,7 @@
 #include <unistd.h>
 #include <errno.h>
 #include <limits.h>
+#include <sys/uio.h>
 
 namespace kj {
 
@@ -58,28 +59,67 @@ void TopLevelProcessContext::exit() {
   quick_exit(exitCode);
 }
 
-void TopLevelProcessContext::warning(StringPtr message) {
-  const char* pos = message.begin();
+static void writeLineToFd(int fd, StringPtr message) {
+  // Write the given message to the given file descriptor with a trailing newline iff the message
+  // is non-empty and doesn't already have a trailing newline.  We use writev() to do this in a
+  // single system call without any copying.
+
+  if (message.size() == 0) {
+    return;
+  }
+
+  // Unfortunately the writev interface requires non-const pointers even though it won't modify
+  // the data.
+  struct iovec vec[2];
+  vec[0].iov_base = const_cast<char*>(message.begin());
+  vec[0].iov_len = message.size();
+  vec[1].iov_base = const_cast<char*>("\n");
+  vec[1].iov_len = 1;
 
-  while (pos < message.end()) {
-    ssize_t n = write(STDERR_FILENO, pos, message.end() - pos);
+  struct iovec* pos = vec;
+
+  // Only use the second item in the vec if the message doesn't already end with \n.
+  uint count = message.endsWith("\n") ? 1 : 2;
+
+  for (;;) {
+    ssize_t n = writev(fd, pos, count);
     if (n < 0) {
       if (errno == EINTR) {
         continue;
+      } else {
+        // This function is meant for writing to stdout and stderr.  If writes fail on those FDs
+        // there's not a whole lot we can reasonably do, so just ignore it.
+        return;
+      }
     }
-      // Apparently we can't write to stderr.  Dunno what to do.
+
+    // Update chunks to discard what was successfully written.
+    for (;;) {
+      if (count == 0) {
+        // Done writing.
         return;
+      } else if (pos->iov_len <= implicitCast<size_t>(n)) {
+        // Wrote this entire chunk.
+        n -= pos->iov_len;
+        ++pos;
+        --count;
+      } else {
+        // Wrote only part of this chunk.  Adjust the pointer and then retry.
+        pos->iov_base = reinterpret_cast<byte*>(pos->iov_base) + n;
+        pos->iov_len -= n;
+        break;
       }
-    pos += n;
     }
-  if (message.size() > 0 && message[message.size() - 1] != '\n') {
-    write(STDERR_FILENO, "\n", 1);
   }
 }
 
+void TopLevelProcessContext::warning(StringPtr message) {
+  writeLineToFd(STDERR_FILENO, message);
+}
+
 void TopLevelProcessContext::error(StringPtr message) {
   hadErrors = true;
-  warning(message);
+  writeLineToFd(STDERR_FILENO, message);
 }
 
 void TopLevelProcessContext::exitError(StringPtr message) {
@@ -88,17 +128,7 @@ void TopLevelProcessContext::exitError(StringPtr message) {
 }
 
 void TopLevelProcessContext::exitInfo(StringPtr message) {
-  const char* pos = message.begin();
-
-  while (pos < message.end()) {
-    ssize_t n;
-    KJ_SYSCALL(n = write(STDOUT_FILENO, pos, message.end() - pos));
-    pos += n;
-  }
-  if (message.size() > 0 && message[message.size() - 1] != '\n') {
-    write(STDOUT_FILENO, "\n", 1);
-  }
-
+  writeLineToFd(STDOUT_FILENO, message);
   exit();
 }