Fix annotations and constants: Factor out type definitions, and add definitions of annotations and constants.

The TypeDefinition is no longer overloaded for Annotation and Constant nodes, and the generator is happy again.

capnpc-csharp/Generator/CodeGenerator.cs

@@ -104,7 +104,7 @@
                 yield return _interfaceGen.MakePipeliningSupport(def);
             }
 
-            if (def.NestedTypes.Count > 0)
+            if (def.NestedTypes.Any())
             {
                 var ns = ClassDeclaration(
                     _names.MakeTypeName(def, NameUsage.Namespace).ToString())

capnpc-csharp/Model/Annotation.cs

@@ -0,0 +1,19 @@
+
+namespace CapnpC.Model
+{
+    class Annotation : IDefinition
+    {
+        public ulong Id { get; }
+        public TypeTag Tag { get => TypeTag.Annotation; }
+        public IHasNestedDefinitions DeclaringElement { get; }
+
+        public Type Type { get; set; }
+
+        public Annotation(ulong id, IHasNestedDefinitions parent)
+        {
+            Id = id;
+            DeclaringElement = parent;
+            parent.NestedDefinitions.Add(this);
+        }
+    }
+}

capnpc-csharp/Model/Constant.cs

@@ -0,0 +1,19 @@
+
+namespace CapnpC.Model
+{
+    class Constant : IDefinition
+    {
+        public ulong Id { get; }
+        public TypeTag Tag { get => TypeTag.Const; }
+        public IHasNestedDefinitions DeclaringElement { get; }
+
+        public Value Value { get; set; } 
+
+        public Constant(ulong id, IHasNestedDefinitions parent)
+        {
+            Id = id;
+            DeclaringElement = parent;
+            parent.NestedDefinitions.Add(this);
+        }
+    }
+}

capnpc-csharp/Model/DefinitionManager.cs

@@ -0,0 +1,67 @@
+using System;
+using System.Collections.Generic;
+using System.Linq;
+
+namespace CapnpC.Model
+{
+    class DefinitionManager
+    {
+        readonly Dictionary<ulong, IDefinition> _id2def = new Dictionary<ulong, IDefinition>();
+
+        public GenFile CreateFile(ulong id)
+            => CreateId<GenFile>(id, () => new GenFile(id));
+        public GenFile GetExistingFile(ulong id)
+            => GetId<GenFile>(id, TypeTag.File);
+
+        public TypeDefinition CreateTypeDef(ulong id, TypeTag tag, IHasNestedDefinitions decl) 
+            => CreateId<TypeDefinition>(id, () => new TypeDefinition(tag, id, decl));
+        public TypeDefinition GetExistingTypeDef(ulong id, TypeTag tag)
+        {
+            var def = GetId<TypeDefinition>(id, tag);
+            if (def.Tag == TypeTag.Unknown) def.Tag = tag;
+            return def;
+        }
+
+        public Annotation CreateAnnotation(ulong id, IHasNestedDefinitions decl)
+            => CreateId<Annotation>(id, () => new Annotation(id, decl));
+        public Annotation GetExistingAnnotation(ulong id)
+            => GetId<Annotation>(id, TypeTag.Annotation);
+
+        public Constant CreateConstant(ulong id, IHasNestedDefinitions decl)
+            => CreateId<Constant>(id, () => new Constant(id, decl));
+        public Constant GetExistingConstant(ulong id)
+            => GetId<Constant>(id, TypeTag.Const);
+
+        public IDefinition GetExistingDef(ulong id, TypeTag tag)
+            => GetId<IDefinition>(id, tag);
+
+        public IEnumerable<GenFile> Files
+        {
+            get => _id2def.Values.Where(d => d.Tag == TypeTag.File).Select(f => f as GenFile);
+        }
+
+        T CreateId<T>(ulong id, Func<IDefinition> creator) where T : class, IDefinition
+        {
+            if (_id2def.TryGetValue(id, out var d))
+            {
+                throw new ArgumentException(nameof(id), $"Attempting to redefine {d.Tag.ToString()} {id.StrId()} (as {nameof(T)}).");
+            }
+            var def = creator();
+            _id2def.Add(id, def);
+            return def as T;
+        }
+
+        T GetId<T>(ulong id, TypeTag tag) where T : IDefinition
+        {
+            if (!_id2def.TryGetValue(id, out var anyDef))
+            {
+                throw new ArgumentOutOfRangeException($"Attempting to retrieve nonexistent node {id.StrId()}.");
+            }
+            if (!(anyDef is T def) || (tag != TypeTag.Unknown && def.Tag != tag))
+            {
+                throw new ArgumentOutOfRangeException($"Attempting to retrieve {tag.ToString()} {id.StrId()}, but found {anyDef.Tag.ToString()} instead.");
+            }
+            return def;
+        }
+    }
+}

capnpc-csharp/Model/GenFile.cs

@@ -1,15 +1,23 @@
-using System;
-using System.Collections.Generic;
-using System.Text;
+using System.Collections.Generic;
 
 namespace CapnpC.Model
 {
-    class GenFile: IHasNestedDefinitions
+    class GenFile: IDefinition, IHasNestedDefinitions
     {
+        public ulong Id { get;  }
+        public TypeTag Tag { get => TypeTag.File;  }
+        public IHasNestedDefinitions DeclaringElement { get; }
+
         public string Name { get; set; }
         public string[] Namespace { get; set; }
 
-        public List<TypeDefinition> NestedTypes { get; } = new List<TypeDefinition>();
-        public List<Value> Constants { get; } = new List<Value>();
+        public IEnumerable<TypeDefinition> NestedTypes { get => this.GetNestedTypes(); }
+        public ICollection<IDefinition> NestedDefinitions { get; } = new List<IDefinition>();
+        public ICollection<Constant> Constants { get; } = new List<Constant>();
+
+        public GenFile(ulong id)
+        {
+            Id = id;
+        }
     }
 }

capnpc-csharp/Model/IDefinition.cs

@@ -0,0 +1,10 @@
+
+namespace CapnpC.Model
+{
+    interface IDefinition
+    {
+        ulong Id { get;  }
+        TypeTag Tag { get; }
+        IHasNestedDefinitions DeclaringElement { get; }
+    }
+}

capnpc-csharp/Model/IHasNestedDefinitions.cs

@@ -1,10 +1,18 @@
 using System.Collections.Generic;
+using System.Linq;
 
 namespace CapnpC.Model
 {
     interface IHasNestedDefinitions
     {
-        List<TypeDefinition> NestedTypes { get; }
-        List<Value> Constants { get; }
+        IEnumerable<TypeDefinition> NestedTypes { get; }
+        ICollection<IDefinition> NestedDefinitions { get; }
+        ICollection<Constant> Constants { get; }
+    }
+
+    static partial class Extensions
+    {
+        public static IEnumerable<TypeDefinition> GetNestedTypes(this IHasNestedDefinitions def)
+            => def.NestedDefinitions.Select(d => d as TypeDefinition).Where(d => d != null);
     }
 }

capnpc-csharp/Model/SchemaModel.cs

@@ -10,8 +10,7 @@ namespace CapnpC.Model
     {
         readonly Schema.CodeGeneratorRequest.Reader _request;
         readonly List<GenFile> _generatedFiles = new List<GenFile>();
-        Dictionary<ulong, IHasNestedDefinitions> _allDefinitions = new Dictionary<ulong, IHasNestedDefinitions>();
-        readonly TypeDefinitionManager _typeDefMgr = new TypeDefinitionManager();
+        readonly DefinitionManager _typeDefMgr = new DefinitionManager();
 
         readonly Dictionary<ulong, Schema.Node.Reader> _id2node = new Dictionary<ulong, Schema.Node.Reader>();
 
@@ -52,8 +51,9 @@ namespace CapnpC.Model
                 _id2node[node.Id] = node;
             }
 
-            BuildPass1();
-            BuildPass2();
+            var requestedFiles = _request.RequestedFiles.ToDictionary(req => req.Id);
+            BuildPass1(requestedFiles);
+            BuildPass2(requestedFiles);
         }
 
         // First pass: create type definitions for each node.
@@ -65,13 +65,12 @@ namespace CapnpC.Model
             public IHasNestedDefinitions parent;
         }
 
-        void BuildPass1()
+        void BuildPass1(Dictionary<ulong, Schema.CodeGeneratorRequest.RequestedFile.Reader> requestedFiles)
         {
             Pass1State state = new Pass1State()
             {
                 unprocessedNodes = new HashSet<ulong>(_id2node.Keys)
             };
-            var requestedFiles = _request.RequestedFiles.ToDictionary(req => req.Id);
             foreach (var node in _id2node.Values.Where(n => n.IsFile))
             {
                 GenFile file;
@@ -86,7 +85,6 @@ namespace CapnpC.Model
                 {
                     file = (GenFile)ProcessNodePass1(node.Id, node.DisplayName, state);
                 }
-                _allDefinitions.Add(node.Id, file);
             }
             if (state.unprocessedNodes.Count != 0)
             {
@@ -94,30 +92,14 @@ namespace CapnpC.Model
             }
         }
 
-        TypeDefinition CreateTypeDef(Schema.Node.Reader node, IHasNestedDefinitions parent)
+        IDefinition ProcessNodePass1(ulong id, string name, Pass1State state)
         {
-            var kind = node.GetKind();
-            var def = _typeDefMgr.Create(node.Id, kind.GetTypeTag());
-            def.DeclaringElement = parent;
-            if (kind == NodeKind.Group)
-                ((TypeDefinition)parent).NestedGroups.Add(def);
-            else
-                parent.NestedTypes.Add(def);
-            return def;
-        }
-
-        IHasNestedDefinitions ProcessNodePass1(ulong id, string name, Pass1State state)
-        {
-            if (!_id2node.TryGetValue(id, out var node))
-            {
-                if (!state.isGenerated) return null;
-                throw new InvalidSchemaException($"The node {id.StrId()} was necessary for backend codegen but is missing.");
-            }
-
+            if (!(IdToNode(id, state.isGenerated) is Schema.Node.Reader node))
+                return null;
             if (!state.unprocessedNodes.Remove(id))
-                throw new InvalidSchemaException($"The node \"{node.DisplayName}\" {node.StrId()} has been declared recursively.");
+                return null;
 
-            GenFile file = null;
+            IDefinition def = null;
             bool processNestedNodes = false;
             bool processFields = false;
             bool processInterfaceMethods = false;
@@ -125,14 +107,16 @@ namespace CapnpC.Model
             switch (node.GetKind())
             {
                 case NodeKind.Annotation:
+                    return _typeDefMgr.CreateAnnotation(id, state.parent);
                 case NodeKind.Const:
-                    // A dummy TypeDefinition is created to node hierarchy
-                    break;
+                    return _typeDefMgr.CreateConstant(id, state.parent);
                 case NodeKind.File:
                     if (state.parent != null)
                         throw new InvalidSchemaException("Did not expect file nodes to appear as nested nodes");
-                    file = new GenFile();
+                    var file = _typeDefMgr.CreateFile(id);
                     file.Namespace = GetNamespaceAnnotation(node);
+                    file.Name = name;
+                    def = file;
                     processNestedNodes = true;
                     break;
                 case NodeKind.Enum:
@@ -151,17 +135,13 @@ namespace CapnpC.Model
                     throw new InvalidSchemaException($"Don't know how to process node {node.StrId()} \"{node.DisplayName}\"");
             }
 
-            TypeDefinition def = null;
-            if (file != null)
+            if (def == null)
             {
-                state.parent = file;
-                file.Name = name;
-            }
-            else
-            {
-                state.parent = def = CreateTypeDef(node, state.parent);
-                def.Name = name;
+                var typeDef = _typeDefMgr.CreateTypeDef(id, node.GetTypeTag(), state.parent);
+                typeDef.Name = name;
+                def = typeDef;
             }
+            state.parent = def as IHasNestedDefinitions;
             
             if (processNestedNodes && node.NestedNodes != null)
                 foreach (var nested in node.NestedNodes)
@@ -186,7 +166,7 @@ namespace CapnpC.Model
                     pnode = IdToNode(method.ResultStructType);
                     if (pnode.ScopeId == 0) ProcessNodePass1(pnode.Id, null, state); // Anonymous generated type
                 }
-            return state.parent;
+            return def;
         }
 
         string[] GetNamespaceAnnotation(Schema.Node.Reader fileNode)
@@ -210,14 +190,13 @@ namespace CapnpC.Model
             public HashSet<ulong> processedNodes;
         }
 
-        void BuildPass2()
+        void BuildPass2(Dictionary<ulong, Schema.CodeGeneratorRequest.RequestedFile.Reader> requestedFiles)
         {
-            var files = _allDefinitions.Select(d => (Id: d.Key, File: d.Value as GenFile)).Where(d => d.File != null);
             var state = new Pass2State() { processedNodes = new HashSet<ulong>() };
-            foreach (var file in files)
+            foreach (var file in _typeDefMgr.Files)
             {
                 var node = IdToNode(file.Id);
-                state.isGenerated = _request.RequestedFiles.Where(req => req.Id == file.Id).Any();
+                state.isGenerated = requestedFiles.ContainsKey(file.Id);
                 ProcessNestedNodes(node.NestedNodes, state);
             }
         }
@@ -234,7 +213,7 @@ namespace CapnpC.Model
         {
             foreach (var scopeReader in brandReader.Scopes)
             {
-                var whatToBind = ProcessNode(scopeReader.ScopeId, state);
+                var whatToBind = ProcessTypeDef(scopeReader.ScopeId, state);
                 int index = 0;
 
                 switch (0)
@@ -290,7 +269,7 @@ namespace CapnpC.Model
                             return Types.FromParameter(
                                 new GenericParameter()
                                 {
-                                    DeclaringEntity = ProcessNode(typeReader.AnyPointer_Parameter_ScopeId, state),
+                                    DeclaringEntity = ProcessTypeDef(typeReader.AnyPointer_Parameter_ScopeId, state),
                                     Index = typeReader.AnyPointer_Parameter_ParameterIndex
                                 });
 
@@ -336,7 +315,7 @@ namespace CapnpC.Model
                     return Types.F64;
 
                 case 0 when typeReader.IsEnum:
-                    return Types.FromDefinition(ProcessNode(typeReader.Enum_TypeId, state, TypeTag.Enum));
+                    return Types.FromDefinition(ProcessTypeDef(typeReader.Enum_TypeId, state, TypeTag.Enum));
 
                 case 0 when typeReader.IsFloat32:
                     return Types.F32;
@@ -354,7 +333,7 @@ namespace CapnpC.Model
                     return Types.S8;
 
                 case 0 when typeReader.IsInterface:
-                    result = Types.FromDefinition(ProcessNode(typeReader.Interface_TypeId, state, TypeTag.Interface));
+                    result = Types.FromDefinition(ProcessTypeDef(typeReader.Interface_TypeId, state, TypeTag.Interface));
                     ProcessBrand(typeReader.Interface_Brand, result, state);
                     return result;
 
@@ -362,7 +341,7 @@ namespace CapnpC.Model
                     return Types.List(ProcessType(typeReader.List_ElementType, state));
 
                 case 0 when typeReader.IsStruct:
-                    result = Types.FromDefinition(ProcessNode(typeReader.Struct_TypeId, state, TypeTag.Struct));
+                    result = Types.FromDefinition(ProcessTypeDef(typeReader.Struct_TypeId, state, TypeTag.Struct));
                     ProcessBrand(typeReader.Struct_Brand, result, state);
                     return result;
 
@@ -520,7 +499,7 @@ namespace CapnpC.Model
                 switch (0)
                 {
                     case 0 when fieldReader.IsGroup:
-                        var def = ProcessNode(fieldReader.Group_TypeId, state, TypeTag.Group);
+                        var def = ProcessTypeDef(fieldReader.Group_TypeId, state, TypeTag.Group);
                         field.Type = Types.FromDefinition(def);
                         break;
 
@@ -642,7 +621,7 @@ namespace CapnpC.Model
                 throw new InvalidSchemaException("Expected a struct");
             }
 
-            var def = ProcessNode(reader.Id, state, TypeTag.Struct);
+            var def = ProcessTypeDef(reader.Id, state, TypeTag.Struct);
 
             if (reader.ScopeId == 0)
             {
@@ -665,7 +644,7 @@ namespace CapnpC.Model
         {
             foreach (var superClassReader in ifaceReader.Interface_Superclasses)
             {
-                var superClass = ProcessNode(superClassReader.Id, state, TypeTag.Interface);
+                var superClass = ProcessTypeDef(superClassReader.Id, state, TypeTag.Interface);
                 def.Superclasses.Add(Types.FromDefinition(superClass));
             }
 
@@ -693,36 +672,47 @@ namespace CapnpC.Model
             return def;
         }
 
-        Value ProcessConst(Schema.Node.Reader constReader, Pass2State state)
+        Constant ProcessConst(Schema.Node.Reader constReader, Constant @const, Pass2State state)
         {
             var value = ProcessValue(constReader.Const_Value);
             value.Type = ProcessType(constReader.Const_Type, state);
-            return value;
+            @const.Value = value;
+            return @const;
         }
 
-        TypeDefinition ProcessNode(ulong id, Pass2State state, TypeTag tag = default)
+        TypeDefinition ProcessTypeDef(ulong id, Pass2State state, TypeTag tag = default)
+        {
+            var def = ProcessNode(id, state, tag);
+            var typeDef = def as TypeDefinition;
+            if (def == null)
+                throw new ArgumentException(
+                    $"Expected node {id.StrId()} to be a TypeDefinition but got {def.GetType().Name} instead.",
+                    nameof(id));
+            return typeDef;
+        }
+
+        IDefinition ProcessNode(ulong id, Pass2State state, TypeTag tag = default)
         {
             if (!(IdToNode(id, state.isGenerated) is Schema.Node.Reader node)) return null;
             var kind = node.GetKind();
             if (tag == TypeTag.Unknown) tag = kind.GetTypeTag();
-            var def = _typeDefMgr.GetExisting(id, tag);
+            var def = _typeDefMgr.GetExistingDef(id, tag);
             if (state.processedNodes.Contains(id)) return def;
             state.processedNodes.Add(id);
 
-            switch (kind)
+            switch (def)
             {
-                case NodeKind.Annotation:
+                case Annotation annotation:
+                    return annotation;
+                case Constant constant:
+                    def.DeclaringElement.Constants.Add(ProcessConst(node, constant, state));
                     return def;
-                case NodeKind.Const:
-                    def.DeclaringElement.Constants.Add(ProcessConst(node, state));
-                    return def;
-                case NodeKind.Enum:
-                    return ProcessEnum(node, def, state);
-                case NodeKind.Interface:
-                    return ProcessInterface(node, def, state);
-                case NodeKind.Struct:
-                case NodeKind.Group:
-                    return ProcessStruct(node, def, state);
+                case TypeDefinition typeDef when kind == NodeKind.Enum:
+                    return ProcessEnum(node, typeDef, state);
+                case TypeDefinition typeDef when kind == NodeKind.Interface:
+                    return ProcessInterface(node, typeDef, state);
+                case TypeDefinition typeDef when kind == NodeKind.Struct || kind == NodeKind.Group:
+                    return ProcessStruct(node, typeDef, state);
                 default:
                     throw new InvalidProgramException($"An unexpected node {node.StrId()} was found during the 2nd schema model building pass.");
             }

capnpc-csharp/Model/TypeDefinition.cs

@@ -2,7 +2,7 @@
 using System.Linq;
 namespace CapnpC.Model
 {
-    class TypeDefinition : AbstractType, IHasNestedDefinitions, IHasGenericParameters
+    class TypeDefinition : AbstractType, IDefinition, IHasNestedDefinitions, IHasGenericParameters
     {
         public class DiscriminationInfo
         {
@@ -16,23 +16,30 @@ namespace CapnpC.Model
             public uint TagOffset { get; }
         }
 
-        public TypeDefinition(TypeTag tag, ulong id)
+        public TypeDefinition(TypeTag tag, ulong id, IHasNestedDefinitions parent)
         {
             Tag = tag;
             Id = id;
+            DeclaringElement = parent;
+            if (tag == TypeTag.Group)
+                ((TypeDefinition)parent).NestedGroups.Add(this);
+            else
+                parent.NestedDefinitions.Add(this);
         }
 
         public ulong Id { get; }
-        public IHasNestedDefinitions DeclaringElement { get; set; }
+        public IHasNestedDefinitions DeclaringElement { get; }
+
         public Method UsingMethod { get; set; }
         public string Name { get; set; }
         public SpecialName SpecialName { get; set; }
         public DiscriminationInfo UnionInfo { get; set; }
         public new List<Field> Fields => base.Fields;
         public List<Enumerant> Enumerants { get; } = new List<Enumerant>();
-        public List<TypeDefinition> NestedTypes { get; } = new List<TypeDefinition>();
+        public ICollection<IDefinition> NestedDefinitions { get; } = new List<IDefinition>();
+        public IEnumerable<TypeDefinition> NestedTypes { get => this.GetNestedTypes(); }
         public List<TypeDefinition> NestedGroups { get; } = new List<TypeDefinition>();
-        public List<Value> Constants { get; } = new List<Value>();
+        public ICollection<Constant> Constants { get; } = new List<Constant>();
         public List<Method> Methods { get; } = new List<Method>();
         public List<Type> Superclasses { get; } = new List<Type>();
         public List<string> GenericParameters { get; } = new List<string>();

capnpc-csharp/Model/TypeDefinitionManager.cs

@@ -1,40 +0,0 @@
-using System;
-using System.Collections.Generic;
-
-namespace CapnpC.Model
-{
-    class TypeDefinitionManager
-    {
-        readonly Dictionary<ulong, TypeDefinition> _id2def =
-            new Dictionary<ulong, TypeDefinition>();
-
-        public TypeDefinition Create(ulong id, TypeTag tag)
-        {
-            if (_id2def.ContainsKey(id))
-            {
-                throw new ArgumentException(nameof(id), $"Attempting to redefine {tag.ToString()} {id.StrId()}.");
-            }
-
-            var def = new TypeDefinition(tag, id);
-            _id2def.Add(id, def);
-            return def;
-        }
-
-        public TypeDefinition GetExisting(ulong id, TypeTag tag)
-        {
-            if (_id2def.TryGetValue(id, out var def))
-            {
-                if (def.Tag == TypeTag.Unknown)
-                {
-                    def.Tag = tag;
-                }
-                else if (tag != TypeTag.Unknown && def.Tag != tag)
-                {
-                    throw new ArgumentOutOfRangeException(nameof(tag), "Type tag does not match existing type");
-                }
-                return def;
-            }
-            throw new ArgumentOutOfRangeException($"Attempting to retrieve nonexistend node {id.StrId()}.");
-        }
-    }
-}

capnpc-csharp/Model/TypeTag.cs

@@ -28,6 +28,9 @@
         Group,
         Interface,
         Enum,
-        AnyEnum
+        AnyEnum,
+        Const,
+        Annotation,
+        File
     }
 }