hasFoo() should return false if foo is in a union and not active. (Previously, it threw.)

New method isFoo() exists for union fields only and returns true if foo is the active union field.

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

@@ -444,8 +444,12 @@ private:
   // -----------------------------------------------------------------
 
   struct DiscriminantChecks {
+    kj::String has;
     kj::String check;
     kj::String set;
+    kj::StringTree readerIsDecl;
+    kj::StringTree builderIsDecl;
+    kj::StringTree isDefs;
   };
 
   DiscriminantChecks makeDiscriminantChecks(kj::StringPtr scope,
@@ -453,16 +457,28 @@ private:
                                             StructSchema containingStruct) {
     auto discrimOffset = containingStruct.getProto().getStruct().getDiscriminantOffset();
 
+    kj::String titleCase = toTitleCase(memberName);
     kj::String upperCase = toUpperCase(memberName);
 
     return DiscriminantChecks {
+        kj::str(
+            "  if (which() != ", scope, upperCase, ") return false;\n"),
         kj::str(
             "  KJ_IREQUIRE(which() == ", scope, upperCase, ",\n"
             "              \"Must check which() before get()ing a union member.\");\n"),
         kj::str(
             "  _builder.setDataField<", scope, "Which>(\n"
             "      ", discrimOffset, " * ::capnp::ELEMENTS, ",
-                      scope, upperCase, ");\n")
+                      scope, upperCase, ");\n"),
+        kj::strTree("  inline bool is", titleCase, "() const;\n"),
+        kj::strTree("  inline bool is", titleCase, "();\n"),
+        kj::strTree(
+            "inline bool ", scope, "Reader::is", titleCase, "() const {\n"
+            "  return which() == ", scope, upperCase, ";\n"
+            "}\n"
+            "inline bool ", scope, "Builder::is", titleCase, "() {\n"
+            "  return which() == ", scope, upperCase, ";\n"
+            "}\n")
     };
   }
 
@@ -501,15 +517,54 @@ private:
         auto slots = getSortedSlots(schemaLoader.get(field.getProto().getGroup()).asStruct());
         return FieldText {
             kj::strTree(
+                kj::mv(unionDiscrim.readerIsDecl),
+                "  inline bool has", titleCase, "() const;\n"
                 "  inline ", titleCase, "::Reader get", titleCase, "() const;\n"
                 "\n"),
 
             kj::strTree(
+                kj::mv(unionDiscrim.builderIsDecl),
+                "  inline bool has", titleCase, "();\n"
                 "  inline ", titleCase, "::Builder get", titleCase, "();\n"
                 "  inline ", titleCase, "::Builder init", titleCase, "();\n"
                 "\n"),
 
             kj::strTree(
+                kj::mv(unionDiscrim.isDefs),
+                "inline bool ", scope, "Reader::has", titleCase, "() const {\n",
+                unionDiscrim.has,
+                "  return ",
+                kj::StringTree(KJ_MAP(slots, slot) {
+                  switch (sectionFor(slot.whichType)) {
+                    case Section::NONE:
+                      return kj::strTree();
+                    case Section::DATA:
+                      return kj::strTree(
+                          "_reader.getDataField<", maskType(slot.whichType), ">(",
+                              slot.offset, " * ::capnp::ELEMENTS) != 0");
+                    case Section::POINTERS:
+                      return kj::strTree(
+                          "!_reader.isPointerFieldNull(", slot.offset, " * ::capnp::POINTERS)");
+                  }
+                }, "\n      || "), ";\n"
+                "}\n"
+                "inline bool ", scope, "Builder::has", titleCase, "() {\n",
+                unionDiscrim.has,
+                "  return ",
+                kj::StringTree(KJ_MAP(slots, slot) {
+                  switch (sectionFor(slot.whichType)) {
+                    case Section::NONE:
+                      return kj::strTree();
+                    case Section::DATA:
+                      return kj::strTree(
+                          "_builder.getDataField<", maskType(slot.whichType), ">(",
+                              slot.offset, " * ::capnp::ELEMENTS) != 0");
+                    case Section::POINTERS:
+                      return kj::strTree(
+                          "!_builder.isPointerFieldNull(", slot.offset, " * ::capnp::POINTERS)");
+                  }
+                }, "\n      || "), ";\n"
+                "}\n"
                 "inline ", scope, titleCase, "::Reader ", scope, "Reader::get", titleCase, "() const {\n",
                 unionDiscrim.check,
                 "  return ", scope, titleCase, "::Reader(_reader);\n"
@@ -653,24 +708,27 @@ private:
     if (kind == FieldKind::PRIMITIVE) {
       return FieldText {
         kj::strTree(
+            kj::mv(unionDiscrim.readerIsDecl),
             "  inline bool has", titleCase, "() const;\n"
             "  inline ", type, " get", titleCase, "() const;\n"
             "\n"),
 
         kj::strTree(
+            kj::mv(unionDiscrim.builderIsDecl),
             "  inline bool has", titleCase, "();\n"
             "  inline ", type, " get", titleCase, "();\n"
             "  inline void set", titleCase, "(", type, " value", setterDefault, ");\n"
             "\n"),
 
         kj::strTree(
+            kj::mv(unionDiscrim.isDefs),
             "inline bool ", scope, "Reader::has", titleCase, "() const {\n",
-            unionDiscrim.check,
+            unionDiscrim.has,
             "  return _reader.hasDataField<", type, ">(", offset, " * ::capnp::ELEMENTS);\n",
             "}\n"
             "\n"
             "inline bool ", scope, "Builder::has", titleCase, "() {\n",
-            unionDiscrim.check,
+            unionDiscrim.has,
             "  return _builder.hasDataField<", type, ">(", offset, " * ::capnp::ELEMENTS);\n",
             "}\n"
             "inline ", type, " ", scope, "Reader::get", titleCase, "() const {\n",
@@ -699,6 +757,7 @@ private:
     } else if (kind == FieldKind::OBJECT) {
       return FieldText {
         kj::strTree(
+            kj::mv(unionDiscrim.readerIsDecl),
             "  inline bool has", titleCase, "() const;\n"
             "  template <typename T>\n"
             "  inline typename T::Reader get", titleCase, "() const;\n"
@@ -707,6 +766,7 @@ private:
             "\n"),
 
         kj::strTree(
+            kj::mv(unionDiscrim.builderIsDecl),
             "  inline bool has", titleCase, "();\n"
             "  template <typename T>\n"
             "  inline typename T::Builder get", titleCase, "();\n"
@@ -725,12 +785,13 @@ private:
             "\n"),
 
         kj::strTree(
+            kj::mv(unionDiscrim.isDefs),
             "inline bool ", scope, "Reader::has", titleCase, "() const {\n",
-            unionDiscrim.check,
+            unionDiscrim.has,
             "  return !_reader.isPointerFieldNull(", offset, " * ::capnp::POINTERS);\n"
             "}\n"
             "inline bool ", scope, "Builder::has", titleCase, "() {\n",
-            unionDiscrim.check,
+            unionDiscrim.has,
             "  return !_builder.isPointerFieldNull(", offset, " * ::capnp::POINTERS);\n"
             "}\n"
             "template <typename T>\n"
@@ -839,11 +900,13 @@ private:
 
       return FieldText {
         kj::strTree(
+            kj::mv(unionDiscrim.readerIsDecl),
             "  inline bool has", titleCase, "() const;\n"
             "  inline ", type, "::Reader get", titleCase, "() const;\n"
             "\n"),
 
         kj::strTree(
+            kj::mv(unionDiscrim.builderIsDecl),
             "  inline bool has", titleCase, "();\n"
             "  inline ", type, "::Builder get", titleCase, "();\n"
             "  inline void set", titleCase, "(", type, "::Reader value);\n",
@@ -861,12 +924,13 @@ private:
             "\n"),
 
         kj::strTree(
+            kj::mv(unionDiscrim.isDefs),
             "inline bool ", scope, "Reader::has", titleCase, "() const {\n",
-            unionDiscrim.check,
+            unionDiscrim.has,
             "  return !_reader.isPointerFieldNull(", offset, " * ::capnp::POINTERS);\n"
             "}\n"
             "inline bool ", scope, "Builder::has", titleCase, "() {\n",
-            unionDiscrim.check,
+            unionDiscrim.has,
             "  return !_builder.isPointerFieldNull(", offset, " * ::capnp::POINTERS);\n"
             "}\n"
             "inline ", type, "::Reader ", scope, "Reader::get", titleCase, "() const {\n",
@@ -1015,9 +1079,9 @@ private:
           namespace_, subScope, nested.getName(), schemaLoader.get(nested.getId())));
     };
 
-    if (proto.which() == schema::Node::STRUCT) {
+    if (proto.isStruct()) {
       for (auto field: proto.getStruct().getFields()) {
-        if (field.which() == schema::Field::GROUP) {
+        if (field.isGroup()) {
           nestedTexts.add(makeNodeText(
               namespace_, subScope, toTitleCase(field.getName()),
               schemaLoader.get(field.getGroup())));

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

@@ -255,12 +255,12 @@ private:
       case schema::Value::DATA:
         return kj::strTree(DynamicValue::Reader(value.getData()));
       case schema::Value::LIST: {
-        KJ_REQUIRE(type.which() == schema::Type::LIST, "type/value mismatch");
+        KJ_REQUIRE(type.isList(), "type/value mismatch");
         auto listValue = value.getList<DynamicList>(ListSchema::of(type.getList(), scope));
         return kj::strTree(listValue);
       }
       case schema::Value::ENUM: {
-        KJ_REQUIRE(type.which() == schema::Type::ENUM, "type/value mismatch");
+        KJ_REQUIRE(type.isEnum(), "type/value mismatch");
         auto enumNode = scope.getDependency(type.getEnum()).asEnum().getProto();
         auto enumerants = enumNode.getEnum().getEnumerants();
         KJ_REQUIRE(value.getEnum() < enumerants.size(),
@@ -268,7 +268,7 @@ private:
         return kj::strTree(enumerants[value.getEnum()].getName());
       }
       case schema::Value::STRUCT: {
-        KJ_REQUIRE(type.which() == schema::Type::STRUCT, "type/value mismatch");
+        KJ_REQUIRE(type.isStruct(), "type/value mismatch");
         auto structValue = value.getStruct<DynamicStruct>(
             scope.getDependency(type.getStruct()).asStruct());
         return kj::strTree(structValue);
@@ -288,7 +288,7 @@ private:
                                const char* prefix = " ", const char* suffix = "") {
     auto decl = schemaLoader.get(annotation.getId());
     auto proto = decl.getProto();
-    KJ_REQUIRE(proto.which() == schema::Node::ANNOTATION);
+    KJ_REQUIRE(proto.isAnnotation());
     auto annDecl = proto.getAnnotation();
 
     return kj::strTree(prefix, "$", nodeName(decl, scope), "(",
@@ -508,7 +508,7 @@ private:
 
   kj::StringTree genFile(Schema file) {
     auto proto = file.getProto();
-    KJ_REQUIRE(proto.which() == schema::Node::FILE, "Expected a file node.", (uint)proto.which());
+    KJ_REQUIRE(proto.isFile(), "Expected a file node.", (uint)proto.which());
 
     return kj::strTree(
       "# ", proto.getDisplayName(), "\n",

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

@@ -384,7 +384,7 @@ Compiler::Node::Node(kj::StringPtr name, Declaration::Which kind)
 
 uint64_t Compiler::Node::generateId(uint64_t parentId, kj::StringPtr declName,
                                     Declaration::Id::Reader declId) {
-  if (declId.which() == Declaration::Id::UID) {
+  if (declId.isUid()) {
     return declId.getUid().getValue();
   }
 
@@ -828,7 +828,7 @@ kj::Maybe<const Compiler::CompiledModule&> Compiler::CompiledModule::importRelat
 }
 
 static void findImports(DeclName::Reader name, std::set<kj::StringPtr>& output) {
-  if (name.getBase().which() == DeclName::Base::IMPORT_NAME) {
+  if (name.getBase().isImportName()) {
     output.insert(name.getBase().getImportName().getValue());
   }
 }
@@ -859,7 +859,7 @@ static void findImports(Declaration::Reader decl, std::set<kj::StringPtr>& outpu
           findImports(ann.getName(), output);
         }
       }
-      if (method.getReturnType().which() == Declaration::Method::ReturnType::EXPRESSION) {
+      if (method.getReturnType().isExpression()) {
         findImports(method.getReturnType().getExpression(), output);
       }
       break;

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

@@ -609,7 +609,7 @@ void NodeTranslator::DuplicateNameDetector::check(
       auto nameText = name.getValue();
       auto insertResult = names.insert(std::make_pair(nameText, name));
       if (!insertResult.second) {
-        if (nameText.size() == 0 && decl.which() == Declaration::UNION) {
+        if (nameText.size() == 0 && decl.isUnion()) {
           errorReporter.addErrorOn(
               name, kj::str("An unnamed union is already defined in this scope."));
           errorReporter.addErrorOn(
@@ -754,7 +754,7 @@ void NodeTranslator::compileEnum(Void decl,
 
   uint codeOrder = 0;
   for (auto member: members) {
-    if (member.which() == Declaration::ENUMERANT) {
+    if (member.isEnumerant()) {
       enumerants.insert(
           std::make_pair(member.getId().getOrdinal().getValue(),
                          std::make_pair(codeOrder++, member)));
@@ -799,7 +799,7 @@ public:
     for (auto& entry: membersByOrdinal) {
       MemberInfo& member = *entry.second;
 
-      if (member.decl.getId().which() == Declaration::Id::ORDINAL) {
+      if (member.decl.getId().isOrdinal()) {
         dupDetector.check(member.decl.getId().getOrdinal());
       }
 
@@ -1159,7 +1159,7 @@ private:
           }
           memberInfo->unionScope = &unionLayout;
           traverseUnion(member.getNestedDecls(), *memberInfo, unionLayout, *subCodeOrder);
-          if (member.getId().which() == Declaration::Id::ORDINAL) {
+          if (member.getId().isOrdinal()) {
             ordinal = member.getId().getOrdinal().getValue();
           }
           break;
@@ -1271,7 +1271,7 @@ bool NodeTranslator::compileType(TypeExpression::Reader source, schema::Type::Bu
           return false;
         }
 
-        if (elementType.which() == schema::Type::OBJECT) {
+        if (elementType.isObject()) {
           errorReporter.addErrorOn(source, "'List(Object)' is not supported.");
           // Seeing List(Object) later can mess things up, so change the type to Void.
           elementType.setVoid();
@@ -1455,9 +1455,9 @@ private:
 
   static kj::Maybe<uint64_t> enumIdForField(StructSchema::Field field) {
     schema::Field::Reader proto = field.getProto();
-    if (proto.which() == schema::Field::NON_GROUP) {
+    if (proto.isNonGroup()) {
       auto type = proto.getNonGroup().getType();
-      if (type.which() == schema::Type::ENUM) {
+      if (type.isEnum()) {
         return type.getEnum();
       }
     }
@@ -1560,7 +1560,7 @@ void NodeTranslator::compileValueInner(
   switch (src.which()) {
     case ValueExpression::NAME: {
       auto name = src.getName();
-      bool isBare = name.getBase().which() == DeclName::Base::RELATIVE_NAME &&
+      bool isBare = name.getBase().isRelativeName() &&
                     name.getMemberPath().size() == 0;
       bool wasSet = false;
       if (isBare) {
@@ -1715,7 +1715,7 @@ kj::Maybe<DynamicValue::Reader> NodeTranslator::readConstant(
         }
       }
 
-      if (name.getBase().which() == DeclName::Base::RELATIVE_NAME &&
+      if (name.getBase().isRelativeName() &&
           name.getMemberPath().size() == 0) {
         // 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
@@ -1723,7 +1723,7 @@ kj::Maybe<DynamicValue::Reader> NodeTranslator::readConstant(
         KJ_IF_MAYBE(scope, resolver.resolveBootstrapSchema(constSchema->getScopeId())) {
           auto scopeReader = scope->getProto();
           kj::StringPtr parent;
-          if (scopeReader.which() == schema::Node::FILE) {
+          if (scopeReader.isFile()) {
             parent = "";
           } else {
             parent = scopeReader.getDisplayName().slice(scopeReader.getDisplayNamePrefixLength());
@@ -1817,7 +1817,7 @@ Orphan<List<schema::Annotation>> NodeTranslator::compileAnnotationApplications(
           switch (value.which()) {
             case Declaration::AnnotationApplication::Value::NONE:
               // No value, i.e. void.
-              if (node.getType().which() == schema::Type::VOID) {
+              if (node.getType().isVoid()) {
                 annotationBuilder.getValue().setVoid();
               } else {
                 errorReporter.addErrorOn(name, kj::str(

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

@@ -110,7 +110,7 @@ void parseFile(List<Statement>::Reader statements, ParsedFile::Builder result,
       Declaration::Builder builder = decl->get();
       switch (builder.which()) {
         case Declaration::NAKED_ID:
-          if (fileDecl.getId().which() == Declaration::Id::UID) {
+          if (fileDecl.getId().isUid()) {
             errorReporter.addError(builder.getStartByte(), builder.getEndByte(),
                                    "File can only have one ID.");
           } else {

c++/src/capnp/dynamic.c++

@@ -400,10 +400,6 @@ bool DynamicStruct::Reader::has(StructSchema::Field field) const {
     if (discrim != proto.getDiscriminantValue()) {
       // Field is not active in the union.
       return false;
-    } else if (discrim != 0) {
-      // Field is active and is not the default active field, therefore it is notable regardless
-      // of the value.
-      return true;
     }
   }
 
@@ -615,8 +611,7 @@ DynamicValue::Builder DynamicStruct::Builder::init(StructSchema::Field field) {
     case schema::Field::NON_GROUP: {
       auto nonGroup = proto.getNonGroup();
       auto type = nonGroup.getType();
-      KJ_REQUIRE(type.which() == schema::Type::STRUCT,
-              "init() without a size is only valid for struct fields.");
+      KJ_REQUIRE(type.isStruct(), "init() without a size is only valid for struct fields.");
       auto subSchema = schema.getDependency(type.getStruct()).asStruct();
       return DynamicStruct::Builder(subSchema,
           builder.initStructField(nonGroup.getOffset() * POINTERS,
@@ -751,8 +746,7 @@ WirePointerCount DynamicStruct::Builder::verifyIsObject(StructSchema::Field fiel
   switch (proto.which()) {
     case schema::Field::NON_GROUP: {
       auto nonGroup = proto.getNonGroup();
-      KJ_REQUIRE(nonGroup.getType().which() == schema::Type::OBJECT,
-                 "Expected an Object.");
+      KJ_REQUIRE(nonGroup.getType().isObject(), "Expected an Object.");
       return nonGroup.getOffset() * POINTERS;
     }
 

c++/src/capnp/dynamic.h

@@ -191,9 +191,9 @@ public:
   bool has(StructSchema::Field field) const;
   // Tests whether the given field is set to its default value.  For pointer values, this does
   // not actually traverse the value comparing it with the default, but simply returns true if the
-  // pointer is non-null.  For members of unions, has() returns whether the field is currently
-  // active and the union as a whole is non-default -- so, the only time has() will return false
-  // for an active union field is if it is the default active field and it has its default value.
+  // pointer is non-null.  For members of unions, has() returns false if the union member is not
+  // active, but does not necessarily return true if the member is active (depends on the field's
+  // value).
 
   kj::Maybe<StructSchema::Field> which() const;
   // If the struct contains an (unnamed) union, and the currently-active field within that union

c++/src/capnp/encoding-test.c++

@@ -375,6 +375,10 @@ TEST(Encoding, UnnamedUnion) {
   root.setBar(321);
   EXPECT_EQ(test::TestUnnamedUnion::BAR, root.which());
   EXPECT_EQ(test::TestUnnamedUnion::BAR, root.asReader().which());
+  EXPECT_FALSE(root.hasFoo());
+  EXPECT_TRUE(root.hasBar());
+  EXPECT_FALSE(root.asReader().hasFoo());
+  EXPECT_TRUE(root.asReader().hasBar());
   EXPECT_EQ(321, root.getBar());
   EXPECT_EQ(321, root.asReader().getBar());
   EXPECT_DEBUG_ANY_THROW(root.getFoo());
@@ -383,6 +387,10 @@ TEST(Encoding, UnnamedUnion) {
   root.setFoo(123);
   EXPECT_EQ(test::TestUnnamedUnion::FOO, root.which());
   EXPECT_EQ(test::TestUnnamedUnion::FOO, root.asReader().which());
+  EXPECT_TRUE(root.hasFoo());
+  EXPECT_FALSE(root.hasBar());
+  EXPECT_TRUE(root.asReader().hasFoo());
+  EXPECT_FALSE(root.asReader().hasBar());
   EXPECT_EQ(123, root.getFoo());
   EXPECT_EQ(123, root.asReader().getFoo());
   EXPECT_DEBUG_ANY_THROW(root.getBar());
@@ -438,6 +446,26 @@ TEST(Encoding, InterleavedGroups) {
   MallocMessageBuilder builder;
   auto root = builder.initRoot<test::TestInterleavedGroups>();
 
+  EXPECT_FALSE(root.hasGroup1());
+  EXPECT_FALSE(root.hasGroup2());
+  EXPECT_FALSE(root.asReader().hasGroup1());
+  EXPECT_FALSE(root.asReader().hasGroup2());
+
+  root.getGroup1().setBar(1);
+
+  EXPECT_TRUE(root.hasGroup1());
+  EXPECT_FALSE(root.hasGroup2());
+  EXPECT_TRUE(root.asReader().hasGroup1());
+  EXPECT_FALSE(root.asReader().hasGroup2());
+
+  // Merely setting the union to a non-default field should also make "has" light up.
+  root.getGroup2().initCorge();
+
+  EXPECT_TRUE(root.hasGroup1());
+  EXPECT_TRUE(root.hasGroup2());
+  EXPECT_TRUE(root.asReader().hasGroup1());
+  EXPECT_TRUE(root.asReader().hasGroup2());
+
   // Init both groups to different values.
   {
     auto group = root.getGroup1();
@@ -463,6 +491,11 @@ TEST(Encoding, InterleavedGroups) {
     group.setWaldo("odlaw");
   }
 
+  EXPECT_TRUE(root.hasGroup1());
+  EXPECT_TRUE(root.hasGroup2());
+  EXPECT_TRUE(root.asReader().hasGroup1());
+  EXPECT_TRUE(root.asReader().hasGroup2());
+
   // Check group1 is still set correctly.
   {
     auto group = root.asReader().getGroup1();
@@ -486,6 +519,11 @@ TEST(Encoding, InterleavedGroups) {
     EXPECT_FALSE(group.hasWaldo());
   }
 
+  EXPECT_FALSE(root.hasGroup1());
+  EXPECT_TRUE(root.hasGroup2());
+  EXPECT_FALSE(root.asReader().hasGroup1());
+  EXPECT_TRUE(root.asReader().hasGroup2());
+
   // Group 2 should not have been touched.
   {
     auto group = root.asReader().getGroup2();

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

@@ -179,14 +179,13 @@ Schema loadUnderAlternateTypeId(SchemaLoader& loader, uint64_t id) {
   auto root = schemaBuilder.getRoot<schema::Node>();
   root.setId(id);
 
-  if (root.which() == schema::Node::STRUCT) {
+  if (root.isStruct()) {
     // If the struct contains any self-referential members, change their type IDs as well.
     auto fields = root.getStruct().getFields();
     for (auto field: fields) {
-      if (field.which() == schema::Field::NON_GROUP) {
+      if (field.isNonGroup()) {
         auto type = field.getNonGroup().getType();
-        if (type.which() == schema::Type::STRUCT &&
-            type.getStruct() == typeId<T>()) {
+        if (type.isStruct() && type.getStruct() == typeId<T>()) {
           type.setStruct(id);
         }
       }

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

@@ -289,7 +289,7 @@ private:
       sawCodeOrder[field.getCodeOrder()] = true;
 
       auto ordinal = field.getOrdinal();
-      if (ordinal.which() == schema::Field::Ordinal::EXPLICIT) {
+      if (ordinal.isExplicit()) {
         VALIDATE_SCHEMA(ordinal.getExplicit() >= nextOrdinal,
                         "fields were not ordered by ordinal");
         nextOrdinal = ordinal.getExplicit() + 1;
@@ -785,25 +785,25 @@ private:
                           UpgradeToStructMode upgradeToStructMode) {
     if (replacement.which() != type.which()) {
       // Check for allowed "upgrade" to Data or Object.
-      if (replacement.which() == schema::Type::DATA && canUpgradeToData(type)) {
+      if (replacement.isData() && canUpgradeToData(type)) {
         replacementIsNewer();
         return;
-      } else if (type.which() == schema::Type::DATA && canUpgradeToData(replacement)) {
+      } else if (type.isData() && canUpgradeToData(replacement)) {
         replacementIsOlder();
         return;
-      } else if (replacement.which() == schema::Type::OBJECT && canUpgradeToObject(type)) {
+      } else if (replacement.isObject() && canUpgradeToObject(type)) {
         replacementIsNewer();
         return;
-      } else if (type.which() == schema::Type::OBJECT && canUpgradeToObject(replacement)) {
+      } else if (type.isObject() && canUpgradeToObject(replacement)) {
         replacementIsOlder();
         return;
       }
 
       if (upgradeToStructMode == ALLOW_UPGRADE_TO_STRUCT) {
-        if (type.which() == schema::Type::STRUCT) {
+        if (type.isStruct()) {
           checkUpgradeToStruct(replacement, type.getStruct());
           return;
-        } else if (replacement.which() == schema::Type::STRUCT) {
+        } else if (replacement.isStruct()) {
           checkUpgradeToStruct(type, replacement.getStruct());
           return;
         }
@@ -939,9 +939,9 @@ private:
   }
 
   bool canUpgradeToData(const schema::Type::Reader& type) {
-    if (type.which() == schema::Type::TEXT) {
+    if (type.isText()) {
       return true;
-    } else if (type.which() == schema::Type::LIST) {
+    } else if (type.isList()) {
       switch (type.getList().which()) {
         case schema::Type::INT8:
         case schema::Type::UINT8:
@@ -1238,7 +1238,7 @@ kj::ArrayPtr<word> SchemaLoader::Impl::makeUncheckedNode(schema::Node::Reader no
 
 kj::ArrayPtr<word> SchemaLoader::Impl::makeUncheckedNodeEnforcingSizeRequirements(
     schema::Node::Reader node) {
-  if (node.which() == schema::Node::STRUCT) {
+  if (node.isStruct()) {
     auto iter = structSizeRequirements.find(node.getId());
     if (iter != structSizeRequirements.end()) {
       auto requirement = iter->second;

c++/src/capnp/schema.c++

@@ -60,23 +60,20 @@ Schema Schema::getDependency(uint64_t id) const {
 }
 
 StructSchema Schema::asStruct() const {
-  KJ_REQUIRE(getProto().which() == schema::Node::STRUCT,
-          "Tried to use non-struct schema as a struct.",
-          getProto().getDisplayName());
+  KJ_REQUIRE(getProto().isStruct(), "Tried to use non-struct schema as a struct.",
+             getProto().getDisplayName());
   return StructSchema(raw);
 }
 
 EnumSchema Schema::asEnum() const {
-  KJ_REQUIRE(getProto().which() == schema::Node::ENUM,
-          "Tried to use non-enum schema as an enum.",
-          getProto().getDisplayName());
+  KJ_REQUIRE(getProto().isEnum(), "Tried to use non-enum schema as an enum.",
+             getProto().getDisplayName());
   return EnumSchema(raw);
 }
 
 InterfaceSchema Schema::asInterface() const {
-  KJ_REQUIRE(getProto().which() == schema::Node::INTERFACE,
-          "Tried to use non-interface schema as an interface.",
-          getProto().getDisplayName());
+  KJ_REQUIRE(getProto().isInterface(), "Tried to use non-interface schema as an interface.",
+             getProto().getDisplayName());
   return InterfaceSchema(raw);
 }
 

c++/src/capnp/stringify-test.c++

@@ -343,24 +343,45 @@ TEST(Stringify, Unions) {
   MallocMessageBuilder builder;
   auto root = builder.initRoot<TestUnion>();
 
-  root.getUnion0().setU0f0s16(321);
+  root.getUnion0().setU0f0s16(123);
   root.getUnion1().setU1f0sp("foo");
   root.getUnion2().setU2f0s1(true);
   root.getUnion3().setU3f0s64(123456789012345678ll);
 
   EXPECT_EQ("("
-      "union0 = (u0f0s16 = 321), "
+      "union0 = (u0f0s16 = 123), "
       "union1 = (u1f0sp = \"foo\"), "
       "union2 = (u2f0s1 = true), "
       "union3 = (u3f0s64 = 123456789012345678))",
       kj::str(root));
 
-  EXPECT_EQ("(u0f0s16 = 321)", kj::str(root.getUnion0()));
+  EXPECT_EQ("(u0f0s16 = 123)", kj::str(root.getUnion0()));
   EXPECT_EQ("(u1f0sp = \"foo\")", kj::str(root.getUnion1()));
   EXPECT_EQ("(u2f0s1 = true)", kj::str(root.getUnion2()));
   EXPECT_EQ("(u3f0s64 = 123456789012345678)", kj::str(root.getUnion3()));
 }
 
+TEST(Stringify, UnionDefaults) {
+  MallocMessageBuilder builder;
+  auto root = builder.initRoot<TestUnion>();
+
+  root.getUnion0().setU0f0s16(0);     // Non-default field has default value.
+  root.getUnion1().setU1f0sp("foo");  // Non-default field has non-default value.
+  root.getUnion2().setU2f0s1(false);  // Default field has default value.
+  root.getUnion3().setU3f0s1(true);   // Default field has non-default value.
+
+  EXPECT_EQ("("
+      "union0 = (u0f0s16 = 0), "
+      "union1 = (u1f0sp = \"foo\"), "
+      "union3 = (u3f0s1 = true))",
+      kj::str(root));
+
+  EXPECT_EQ("(u0f0s16 = 0)", kj::str(root.getUnion0()));
+  EXPECT_EQ("(u1f0sp = \"foo\")", kj::str(root.getUnion1()));
+  EXPECT_EQ("()", kj::str(root.getUnion2()));
+  EXPECT_EQ("(u3f0s1 = true)", kj::str(root.getUnion3()));
+}
+
 TEST(Stringify, UnnamedUnions) {
   MallocMessageBuilder builder;
   auto root = builder.initRoot<test::TestUnnamedUnion>();

c++/src/capnp/stringify.c++

@@ -198,15 +198,22 @@ static kj::StringTree print(const DynamicValue::Reader& value,
       // We try to write the union field, if any, in proper order with the rest.
       auto which = structValue.which();
 
+      kj::StringTree unionValue;
+      KJ_IF_MAYBE(field, which) {
+        // Even if the union field has its default value, if it is not the default field of the
+        // union then we have to print it anyway.
+        auto fieldProto = field->getProto();
+        if (fieldProto.getDiscriminantValue() != 0 || structValue.has(*field)) {
+          unionValue = kj::strTree(
+              fieldProto.getName(), " = ",
+              print(structValue.get(*field), whichFieldType(*field), indent.next(), PREFIXED));
+        }
+      }
+
       for (auto field: nonUnionFields) {
         KJ_IF_MAYBE(unionField, which) {
           if (unionField->getIndex() < field.getIndex()) {
-            if (structValue.has(*unionField)) {
-              printedFields.add(kj::strTree(
-                  unionField->getProto().getName(), " = ",
-                  print(structValue.get(*unionField), whichFieldType(*unionField),
-                        indent.next(), PREFIXED)));
-            }
+            printedFields.add(kj::mv(unionValue));
             which = nullptr;
           }
         }
@@ -216,12 +223,9 @@ static kj::StringTree print(const DynamicValue::Reader& value,
               print(structValue.get(field), whichFieldType(field), indent.next(), PREFIXED)));
         }
       }
-      KJ_IF_MAYBE(field, which) {
-        if (structValue.has(*field)) {
-          printedFields.add(kj::strTree(
-              field->getProto().getName(), " = ",
-              print(structValue.get(*field), whichFieldType(*field), indent.next(), PREFIXED)));
-        }
+      if (which != nullptr) {
+        // Union value is last.
+        printedFields.add(kj::mv(unionValue));
       }
 
       if (mode == PARENTHESIZED) {