Remove ast-node type caching, fix bugs, refactor.

packages/adl/compiler/binder.ts

@@ -1,7 +1,6 @@
 import { DiagnosticError, formatDiagnostic } from "./diagnostics.js";
 import { visitChildren } from "./parser.js";
 import { ADLSourceFile, Program } from "./program.js";
-import { createSourceFile } from "./scanner.js";
 import {
   NamespaceStatementNode,
   ModelStatementNode,
@@ -10,6 +9,7 @@ import {
   TemplateParameterDeclarationNode,
   SourceLocation,
   Sym,
+  Declaration,
 } from "./types.js";
 
 export class SymbolTable extends Map<string, Sym> {
@@ -83,7 +83,7 @@ export function createBinder(): Binder {
         bindModelStatement(<any>node);
         break;
       case SyntaxKind.NamespaceStatement:
-        bindInterfaceStatement(<any>node);
+        bindNamespaceStatement(<any>node);
         break;
       case SyntaxKind.TemplateParameterDeclaration:
         bindTemplateParameterDeclaration(<any>node);
@@ -115,22 +115,13 @@ export function createBinder(): Binder {
   }
 
   function bindModelStatement(node: ModelStatementNode) {
-    currentFile.symbols.set(node.id.sv, {
-      kind: "type",
-      node: node,
-      name: node.id.sv,
-    });
-
+    declareSymbol(currentFile.symbols, node);
     // initialize locals for type parameters.
     node.locals = new SymbolTable();
   }
 
-  function bindInterfaceStatement(statement: NamespaceStatementNode) {
-    currentFile.symbols.set(statement.id.sv, {
-      kind: "type",
-      node: statement,
-      name: statement.id.sv,
-    });
+  function bindNamespaceStatement(statement: NamespaceStatementNode) {
+    declareSymbol(currentFile.symbols, statement);
   }
 
   function reportDuplicateSymbols(globalSymbols: SymbolTable) {
@@ -175,3 +166,17 @@ function hasScope(node: Node) {
       return false;
   }
 }
+
+function createTypeSymbol(node: Node, name: string): TypeSymbol {
+  return {
+    kind: "type",
+    node,
+    name,
+  };
+}
+
+function declareSymbol(table: SymbolTable, node: Declaration) {
+  const symbol = createTypeSymbol(node, node.id.sv);
+  node.symbol = symbol;
+  table.set(node.id.sv, symbol);
+}

packages/adl/compiler/checker.ts

@@ -23,7 +23,7 @@ import {
   StringLiteralNode,
   StringLiteralType,
   SyntaxKind,
-  TemplateApplicationNode,
+  TypeReferenceNode,
   TemplateParameterDeclarationNode,
   TupleExpressionNode,
   TupleType,
@@ -31,6 +31,9 @@ import {
   UnionExpressionNode,
   UnionType,
   ReferenceExpression,
+  DecoratorSymbol,
+  TypeSymbol,
+  SymbolLinks,
 } from "./types.js";
 
 /**
@@ -72,6 +75,8 @@ export class MultiKeyMap<T> {
 export function createChecker(program: Program) {
   let templateInstantiation: Array<Type> = [];
   let instantiatingTemplate: Node | undefined;
+  let currentSymbolId = 0;
+  const symbolLinks = new Map<number, SymbolLinks>();
 
   const seq = 0;
 
@@ -84,9 +89,6 @@ export function createChecker(program: Program) {
   };
 
   function getTypeForNode(node: Node): Type {
-    const cached = program.typeCache.get([node, ...templateInstantiation.values()]);
-    if (cached) return cached;
-
     switch (node.kind) {
       case SyntaxKind.ModelExpression:
         return checkModel(node);
@@ -98,9 +100,6 @@ export function createChecker(program: Program) {
         return checkNamespace(node);
       case SyntaxKind.NamespaceProperty:
         return checkNamespaceProperty(node);
-      case SyntaxKind.Identifier:
-        // decorator bindings presently return an empty binding
-        return <any>checkIdentifier(node);
       case SyntaxKind.NumericLiteral:
         return checkNumericLiteral(node);
       case SyntaxKind.BooleanLiteral:
@@ -115,8 +114,8 @@ export function createChecker(program: Program) {
         return checkUnionExpression(node);
       case SyntaxKind.IntersectionExpression:
         return checkIntersectionExpression(node);
-      case SyntaxKind.TemplateApplication:
-        return checkTemplateApplication(node);
+      case SyntaxKind.TypeReference:
+        return checkTypeReference(node);
       case SyntaxKind.TemplateParameterDeclaration:
         return checkTemplateParameterDeclaration(node);
     }
@@ -172,11 +171,66 @@ export function createChecker(program: Program) {
     });
   }
 
-  function checkTemplateApplication(node: TemplateApplicationNode): Type {
+  function checkTypeReference(node: TypeReferenceNode): Type {
+    // todo: support member expressions
+    const sym = resolveTypeReference(node.target as IdentifierNode);
+    if (sym.kind === "decorator") {
+      throwDiagnostic("Can't put a decorator in a type", node);
+    }
+
+    const symbolLinks = getSymbolLinks(sym);
     const args = node.arguments.map(getTypeForNode);
-    const targetType = getTypeForNode(node.target);
-    // todo: check proper target
-    return instantiateTemplate(<ModelStatementNode>targetType.node, args);
+
+    if (sym.node.kind === SyntaxKind.ModelStatement && !sym.node.assignment) {
+      // model statement, possibly templated
+      if (sym.node.templateParameters.length === 0) {
+        if (args.length > 0) {
+          throwDiagnostic("Can't pass template arguments to model that is not templated", node);
+        }
+
+        if (symbolLinks.declaredType) {
+          return symbolLinks.declaredType;
+        }
+
+        return checkModelStatement(sym.node);
+      } else {
+        // model is templated, lets instantiate.
+
+        if (!symbolLinks.declaredType) {
+          // we haven't checked the declared type yet, so do so.
+          checkModelStatement(sym.node);
+        }
+        if (sym.node.templateParameters!.length > node.arguments.length) {
+          throwDiagnostic("Too few template arguments provided.", node);
+        }
+
+        if (sym.node.templateParameters!.length < node.arguments.length) {
+          throwDiagnostic("Too many template arguments provided.", node);
+        }
+
+        return instantiateTemplate(sym.node, args);
+      }
+    }
+    // some other kind of reference
+
+    if (args.length > 0) {
+      throwDiagnostic("Can't pass template arguments to non-templated type", node);
+    }
+
+    if (sym.node.kind === SyntaxKind.TemplateParameterDeclaration) {
+      const type = checkTemplateParameterDeclaration(sym.node);
+      // TODO: could cache this probably.
+      return type;
+    }
+    // types for non-templated types
+    if (symbolLinks.type) {
+      return symbolLinks.type;
+    }
+
+    const type = getTypeForNode(sym.node);
+    symbolLinks.type = type;
+
+    return type;
   }
 
   /**
@@ -185,15 +239,14 @@ export function createChecker(program: Program) {
    * parameters to access type type arguments.
    *
    * This will fall over if the same template is ever being instantiated
-   * twice at the same time.
+   * twice at the same time, or if template parameters from more than one template
+   * are ever in scope at once.
    */
   function instantiateTemplate(templateNode: ModelStatementNode, args: Array<Type>): ModelType {
-    if (templateNode.templateParameters!.length < args.length) {
-      throwDiagnostic("Too few template arguments provided.", templateNode);
-    }
-
-    if (templateNode.templateParameters!.length > args.length) {
-      throwDiagnostic("Too many template arguments provided.", templateNode);
+    const symbolLinks = getSymbolLinks(templateNode.symbol!);
+    const cached = symbolLinks.instantiations!.get(args) as ModelType;
+    if (cached) {
+      return cached;
     }
 
     const oldTis = templateInstantiation;
@@ -202,6 +255,9 @@ export function createChecker(program: Program) {
     instantiatingTemplate = templateNode;
     // this cast is invalid once we support templatized `model =`.
     const type = <ModelType>getTypeForNode(templateNode);
+
+    symbolLinks.instantiations!.set(args, type);
+
     type.templateNode = templateNode;
     templateInstantiation = oldTis;
     instantiatingTemplate = oldTemplate;
@@ -242,13 +298,10 @@ export function createChecker(program: Program) {
           );
         }
 
-        const newPropType = createType(
-          {
-            ...prop,
-            sourceProperty: prop,
-          },
-          true
-        );
+        const newPropType = createType({
+          ...prop,
+          sourceProperty: prop,
+        });
 
         properties.set(prop.name, newPropType);
       }
@@ -286,6 +339,9 @@ export function createChecker(program: Program) {
       type.properties.set(prop.id.sv, checkNamespaceProperty(prop));
     }
 
+    const links = getSymbolLinks(node.symbol!);
+    links.type = type;
+
     return type;
   }
 
@@ -307,16 +363,28 @@ export function createChecker(program: Program) {
     });
   }
 
-  function checkIdentifier(node: IdentifierNode) {
-    const binding = resolveIdentifier(node);
-    if (binding.kind === "decorator") {
-      return {};
-    } else {
-      return getTypeForNode(binding.node);
+  function getSymbolLinks(s: TypeSymbol): SymbolLinks {
+    const id = getSymbolId(s);
+
+    if (symbolLinks.has(id)) {
+      return symbolLinks.get(id)!;
     }
+
+    const links = {};
+    symbolLinks.set(id, links);
+
+    return links;
   }
 
-  function resolveIdentifier(node: IdentifierNode) {
+  function getSymbolId(s: TypeSymbol) {
+    if (s.id === undefined) {
+      s.id = currentSymbolId++;
+    }
+
+    return s.id;
+  }
+
+  function resolveIdentifier(node: IdentifierNode): DecoratorSymbol | TypeSymbol {
     let scope: Node | undefined = node.parent;
     let binding;
 
@@ -339,6 +407,11 @@ export function createChecker(program: Program) {
     return binding;
   }
 
+  function resolveTypeReference(node: IdentifierNode): DecoratorSymbol | TypeSymbol {
+    // TODO: Support for member expressions
+    return resolveIdentifier(node);
+  }
+
   function checkStringLiteral(str: StringLiteralNode): StringLiteralType {
     return getLiteralType(str);
   }
@@ -364,65 +437,113 @@ export function createChecker(program: Program) {
   }
 
   function checkModel(node: ModelExpressionNode | ModelStatementNode) {
-    if (node.properties) {
-      const properties = new Map();
-      const baseModels =
-        node.kind === SyntaxKind.ModelExpression ? [] : checkClassHeritage(node.heritage);
-
-      for (const prop of node.properties) {
-        if ("id" in prop) {
-          const propType = <ModelTypeProperty>getTypeForNode(prop);
-          properties.set(propType.name, propType);
-        } else {
-          // spread property
-          const newProperties = checkSpreadProperty(prop.target);
-
-          for (const newProp of newProperties) {
-            if (properties.has(newProp.name)) {
-              throwDiagnostic(`Model already has a property named ${newProp.name}`, node);
-            }
-
-            properties.set(newProp.name, newProp);
-          }
-        }
+    if (node.kind === SyntaxKind.ModelStatement) {
+      if (node.properties) {
+        return checkModelStatement(node);
+      } else {
+        return checkModelEquals(node);
       }
-
-      return createType({
-        kind: "Model",
-        name: node.kind === SyntaxKind.ModelStatement ? node.id.sv : "",
-        node: node,
-        properties,
-        baseModels: baseModels,
-      });
     } else {
-      // model =
-      // this will likely have to change, as right now `model =` is really just
-      // alias and so disappears. That means you can't easily rename symbols.
-      const assignmentType = getTypeForNode((<ModelStatementNode>node).assignment!);
-
-      if (assignmentType.kind === "Model") {
-        const type: ModelType = createType({
-          ...(<ModelType>assignmentType),
-          node: node,
-          name: (<ModelStatementNode>node).id.sv,
-          assignmentType,
-        });
-
-        return type;
-      }
-
-      return assignmentType;
+      return checkModelExpression(node);
     }
   }
 
-  function checkClassHeritage(heritage: ReferenceExpression[]): ModelType[] {
-    return heritage.flatMap((heritageRef) => {
-      const heritageType = getTypeForNode(heritageRef);
+  function checkModelStatement(node: ModelStatementNode) {
+    const links = getSymbolLinks(node.symbol!);
+    const instantiatingThisTemplate = instantiatingTemplate === node;
 
-      if (heritageType.kind === "TemplateParameter") {
-        // don't need to track heritage for template parameters.
-        return [];
+    if (links.declaredType && !instantiatingThisTemplate) {
+      // we're not instantiating this model and we've already checked it
+      return links.declaredType;
+    }
+
+    const baseModels = checkClassHeritage(node.heritage);
+    const properties = checkModelProperties(node);
+    const type: ModelType = {
+      kind: "Model",
+      name: node.id.sv,
+      node: node,
+      properties,
+      baseModels: baseModels,
+    };
+
+    if (
+      instantiatingThisTemplate &&
+      templateInstantiation.every((t) => t.kind !== "TemplateParameter")
+    ) {
+      createType(type);
+    }
+
+    if (!instantiatingThisTemplate) {
+      links.declaredType = type;
+      links.instantiations = new MultiKeyMap();
+    }
+
+    return type;
+  }
+
+  function checkModelExpression(node: ModelExpressionNode) {
+    const properties = checkModelProperties(node);
+    const type: ModelType = createType({
+      kind: "Model",
+      name: "",
+      node: node,
+      properties,
+      baseModels: [],
+    });
+
+    return type;
+  }
+
+  function checkModelProperties(node: ModelExpressionNode | ModelStatementNode) {
+    const properties = new Map();
+    for (const prop of node.properties!) {
+      if ("id" in prop) {
+        const propType = <ModelTypeProperty>getTypeForNode(prop);
+        properties.set(propType.name, propType);
+      } else {
+        // spread property
+        const newProperties = checkSpreadProperty(prop.target);
+
+        for (const newProp of newProperties) {
+          if (properties.has(newProp.name)) {
+            throwDiagnostic(`Model already has a property named ${newProp.name}`, node);
+          }
+
+          properties.set(newProp.name, newProp);
+        }
       }
+    }
+
+    return properties;
+  }
+
+  function checkModelEquals(node: ModelStatementNode) {
+    // model =
+    // this will likely have to change, as right now `model =` is really just
+    // alias and so disappears. That means you can't easily rename symbols.
+    const assignmentType = getTypeForNode((<ModelStatementNode>node).assignment!);
+
+    if (assignmentType.kind === "Model") {
+      const type: ModelType = createType({
+        ...(<ModelType>assignmentType),
+        node: node,
+        name: (<ModelStatementNode>node).id.sv,
+        assignmentType,
+      });
+
+      return type;
+    }
+
+    const links = getSymbolLinks(node.symbol!);
+    links.type = assignmentType;
+
+    return assignmentType;
+  }
+
+  function checkClassHeritage(heritage: ReferenceExpression[]): ModelType[] {
+    return heritage.map((heritageRef) => {
+      const heritageType = getTypeForNode(heritageRef);
 
       if (heritageType.kind !== "Model") {
         throwDiagnostic("Models must extend other models.", heritageRef);
@@ -443,13 +564,10 @@ export function createChecker(program: Program) {
 
       // copy each property
       for (const prop of walkPropertiesInherited(targetType)) {
-        const newProp = createType(
-          {
-            ...prop,
-            sourceProperty: prop,
-          },
-          true
-        );
+        const newProp = createType({
+          ...prop,
+          sourceProperty: prop,
+        });
         props.push(newProp);
       }
     }
@@ -490,18 +608,12 @@ export function createChecker(program: Program) {
       });
     }
   }
+
   // the types here aren't ideal and could probably be refactored.
-  function createType<T extends Type>(typeDef: T, skipCache = false): T {
-    if (!skipCache) {
-      (<any>typeDef).seq = program.typeCache.set([typeDef.node, ...templateInstantiation], typeDef);
-    }
+  function createType<T extends Type>(typeDef: T): T {
     (<any>typeDef).templateArguments = templateInstantiation;
 
-    // only run decorators on fully instantiated types.
-    if (templateInstantiation.every((i) => i.kind !== "TemplateParameter")) {
-      program.executeDecorators(typeDef);
-    }
-
+    program.executeDecorators(typeDef);
     return typeDef;
   }
 

packages/adl/compiler/parser.ts

@@ -379,24 +379,20 @@ export function parse(code: string | Types.SourceFile) {
 
   function parseReferenceExpression(): Types.ReferenceExpression {
     const pos = tokenPos();
-    const expr = parseIdentifierOrMemberExpression();
+    const target = parseIdentifierOrMemberExpression();
 
-    if (token() !== Token.LessThan) {
-      return expr;
+    let args: Types.Expression[] = [];
+    if (parseOptional(Token.LessThan)) {
+      args = parseExpressionList();
+      parseExpected(Token.GreaterThan);
     }
 
-    parseExpected(Token.LessThan);
-    const args = parseExpressionList();
-    parseExpected(Token.GreaterThan);
 
-    return finishNode(
-      {
-        kind: Types.SyntaxKind.TemplateApplication,
-        target: expr,
-        arguments: args,
-      },
-      pos
-    );
+    return finishNode({
+      kind: Types.SyntaxKind.TypeReference,
+      target,
+      arguments: args,
+    }, pos);
   }
 
   function parseReferenceExpressionList(): Types.ReferenceExpression[] {
@@ -773,8 +769,9 @@ export function visitChildren<T>(node: Types.Node, cb: NodeCb<T>): T | undefined
       );
     case Types.SyntaxKind.NamedImport:
       return visitNode(cb, node.id);
-    case Types.SyntaxKind.TemplateApplication:
-      return visitNode(cb, node.target) || visitEach(cb, node.arguments);
+    case Types.SyntaxKind.TypeReference:
+      return visitNode(cb, node.target) ||
+        visitEach(cb, node.arguments);
     case Types.SyntaxKind.TupleExpression:
       return visitEach(cb, node.values);
     case Types.SyntaxKind.UnionExpression:

packages/adl/compiler/types.ts

@@ -1,4 +1,5 @@
 import { SymbolTable } from "./binder";
+import { MultiKeyMap } from "./checker";
 
 /**
  * Type System types
@@ -114,8 +115,19 @@ export interface TypeSymbol {
   kind: "type";
   node: Node;
   name: string;
+  id?: number;
 }
 
+export interface SymbolLinks {
+  type?: Type;
+
+  // for types which can be instantiated, we split `type` into declaredType and
+  // a map of instantiations.
+  declaredType?: Type;
+  instantiations?: MultiKeyMap<Type>; 
+}
+
+
 /**
  * AST types
  */
@@ -139,8 +151,8 @@ export enum SyntaxKind {
   StringLiteral,
   NumericLiteral,
   BooleanLiteral,
-  TemplateApplication,
-  TemplateParameterDeclaration,
+  TypeReference,
+  TemplateParameterDeclaration
 }
 
 export interface BaseNode extends TextRange {
@@ -160,6 +172,7 @@ export type Node =
   | Statement
   | Expression;
 
+
 export interface ADLScriptNode extends BaseNode {
   kind: SyntaxKind.ADLScript;
   statements: Array<Statement>;
@@ -168,6 +181,14 @@ export interface ADLScriptNode extends BaseNode {
 
 export type Statement = ImportStatementNode | ModelStatementNode | NamespaceStatementNode;
 
+export interface DeclarationNode {
+  symbol?: TypeSymbol; // tracks the symbol assigned to this declaration
+}
+
+export type Declaration =
+  | ModelStatementNode
+  | NamespaceStatementNode;
+
 export interface ImportStatementNode extends BaseNode {
   kind: SyntaxKind.ImportStatement;
   id: IdentifierNode;
@@ -197,13 +218,16 @@ export type Expression =
   | TupleExpressionNode
   | UnionExpressionNode
   | IntersectionExpressionNode
-  | TemplateApplicationNode
+  | TypeReferenceNode
   | IdentifierNode
   | StringLiteralNode
   | NumericLiteralNode
   | BooleanLiteralNode;
 
-export type ReferenceExpression = TemplateApplicationNode | MemberExpressionNode | IdentifierNode;
+export type ReferenceExpression =
+  | TypeReferenceNode
+  | MemberExpressionNode
+  | IdentifierNode;
 
 export interface MemberExpressionNode extends BaseNode {
   kind: SyntaxKind.MemberExpression;
@@ -211,7 +235,7 @@ export interface MemberExpressionNode extends BaseNode {
   base: MemberExpressionNode | IdentifierNode;
 }
 
-export interface NamespaceStatementNode extends BaseNode {
+export interface NamespaceStatementNode extends BaseNode, DeclarationNode {
   kind: SyntaxKind.NamespaceStatement;
   id: IdentifierNode;
   parameters?: ModelExpressionNode;
@@ -227,7 +251,8 @@ export interface NamespacePropertyNode extends BaseNode {
   decorators: Array<DecoratorExpressionNode>;
 }
 
-export interface ModelStatementNode extends BaseNode {
+
+export interface ModelStatementNode extends BaseNode, DeclarationNode {
   kind: SyntaxKind.ModelStatement;
   id: IdentifierNode;
   properties?: Array<ModelPropertyNode | ModelSpreadPropertyNode>;
@@ -293,8 +318,8 @@ export interface IntersectionExpressionNode extends BaseNode {
   options: Array<Expression>;
 }
 
-export interface TemplateApplicationNode extends BaseNode {
-  kind: SyntaxKind.TemplateApplication;
+export interface TypeReferenceNode extends BaseNode {
+  kind: SyntaxKind.TypeReference;
   target: Expression;
   arguments: Array<Expression>;
 }