Template semantics by Kronos3 · Pull Request #1006 · nasa/fpp

Kronos3 · 2026-05-27T16:05:47Z

This PR includes the implementation of templates. I'm opening this PR so it's easier to track work and look at diffs, it is not ready for review/merge yet.

…mantics

…emantics

…mbols for nested template expansion

…-semantics and fix it up

Kronos3 · 2026-07-09T03:24:24Z

@bocchino I've dug into the convertValueToType conversion issue that we discussed today and I'd like to encode my findings so we can find a path forward. It's quite a long block of text, sorry!

Test case

array B = [2] U32

module template M(constant b: B) {
  constant c = [1, b]
}

expand M(constant [2, 3])

Error Diagnosis

The error: cannot convert value 1 to type array of Integer. This error occurs at the point where FPP tries to promote the primitive integer 1 to the commonType(1, typeof(b)

The answer was actually encoded exactly in this error message though the message itself was misleading. See this snippet below:

fpp/compiler/lib/src/main/scala/analysis/Semantics/Type.scala

Lines 347 to 350 in cc7fec0

    
           override def toString = size match { 
        
             case Some(n) => "[" ++ n.toString ++ "] " ++ eltType.toString 
        
             case None => "array of " ++ eltType.toString 
        
           }

When an anonymous array size is known, the name of the array is [size] EltType and when it's unknown (None) it's array of EltType. When the integer promoted, the array length must exist which is the reason why this conversion fails:

fpp/compiler/lib/src/main/scala/analysis/Semantics/Value.scala

Lines 65 to 70 in cc7fec0

    
           if (this.getType.isPromotableToArray) 
        
             for { 
        
               size <- anonArray.size 
        
               elt <- this.convertToType(anonArray.eltType) 
        
             } 
        
             yield Value.AnonArray(List.fill(size)(elt))

Note: This is not a simple case of FinalizeType needing to be run because there is no actual type to finalize. This is an anonymous array type that needs to be finalized which is not possible since the syntax should implicitly provide a size. See below for the cause.

Cause

As an initial note, the [unknown] Integer type that FPP attempts to promote 1 to is coming from the commonType computed from the CheckExprTypes pass earlier in the execution.

Let's read the FPP spec on commonType:

I believe the issue is with step 6. We are stripping away the concrete array type for an anonymous array with unknown size. This was ok before because there was never a case where this case was hit. There was never a case in which a value of concrete array type could be constructed and that value be commonType-ed with another type. Templates constant parameters constructing values with any concrete type.

As a side note: I think anonymous arrays of None size should never be possible at any point in the analysis. Anonymous arrays stem from array literals which should always be trivially determined since you can just count the number of elements in the syntax. It may be worth keeping this in the semantics as a mechanism to allow concrete Arrays to wrap AnonArray which would allow None sizes.

A side-note to my side note: Anonymous arrays of None size will be a valid thing when we have [0, ...] ellipsis operators.

Fix

We need to change the semantics of commonType to say:

// (Array, Array) case

Otherwise if T1 and T2 are array types, throw an error (I'm fairly sure there is there no valid commonType solution here, but maybe you can think of something?)

// (Array, AnonArray)
// (AnonArray, Array)

Otherwise if one of T1 or T2 is an array type A equivalent to [n1] T' and the other is an anonymous array type [n2] T'':

a. If T'' is convertible to T' and n1 == n2, let T be A
b. Otherwise the attempted resolution is invalid. Throw an error

// (Array, promotableToArray(T))
// (promotableToArray(T), Array)

Otherwise if one of T1 or T2 is an array type A equivalent to [n] T' and the other is a type convertible to an anonymous array type T'':

a. If T'' is convertible to T', let T be A
b. Otherwise the attempted resolution is invalid. Throw an error

... continue onward with the same rules

Implementation note!

commonType is computed before concrete array sizes are known, I'm not sure if these rules can actually be implemented as stated above at the moment?

I'm mainly referring to Step 7a where we check n1 == n2. We could theoretically defer that check later when we attempt to convert the value to the concrete type.

bocchino · 2026-07-09T05:03:06Z

I believe the issue is with step 6.

I think it's trying to apply step 8 (common type of Integer and [n] U32) at a point where n is unknown. I don't think we need to revise the spec. I think we need to revise the implementation. One way to do it is to accumulate constraints at the point of type checking where the sizes aren't known, and check the constraints later after we know the sizes.

bocchino · 2026-07-09T05:04:30Z

Otherwise if T1 and T2 are array types, throw an error (I'm fairly sure there is there no valid commonType solution here, but maybe you can think of something?)

I think the spec is correct here. For example, if A and B are both array types with shape [2] U32, then the common type should be the shape (anonymous array). The common type should be the least type to which both types are assignable.

bocchino · 2026-07-09T05:41:58Z

One way to do it is to accumulate constraints at the point of type checking where the sizes aren't known, and check the constraints later after we know the sizes.

Actually I suspect what may be happening is this:

Before adding template parameters, we could compute partial type info (missing array sizes) and that was enough to compute values; then we use the values to finalize the types and finish the type checking.
With template parameters, the partial type info is not sufficient to compute the values under the current rules -- that's what seems to be causing the crash. Specifically, we are trying to convert the value 1: Integer to the partial type "array of unknown size" and the system can't do that. We never needed to do that before, but now we do.

So a solution may be to relax the value conversion rules to allow the temporary formation of array values with unknown sizes, analogously to how we temporarily form array types with unknown sizes. That should provide enough scalar value info to finalize the array values and the array types.

…-semantics

Disallow template composition

Kronos3 · 2026-08-05T22:34:01Z

+        expr.data match {
+          case Ast.ExprIdent(e) => Right(List(AstNode.create(e, expr.id)))
+          case Ast.ExprDot(e, id) => for (left <- exprToIdentList(e)) yield left :+ id
+          // TODO(tumbar) Make error messages specific to the parameter type


Kronos3 · 2026-08-05T22:38:38Z

+                nestedScope <- nestedScope.put(NameGroup.Value)(param.getUnqualifiedName, param)
+                nestedScope <- nestedScope.put(NameGroup.Type)(param.getUnqualifiedName, param)
+                nestedScope <- nestedScope.put(NameGroup.PortInterfaceInstance)(param.getUnqualifiedName, param)


Don't add to all name-groups

Kronos3 · 2026-08-05T22:40:45Z

        val eltType = a.typeMap(node.id) match {
          case Type.AnonArray(_, eltType) => eltType
+          case Type.Array(_, Type.AnonArray(_, eltType), _, _) => eltType
          case _ => throw InternalError("element type of array expression should be AnonArray")


Kronos3 · 2026-08-05T22:42:35Z

    override def toString = size match {
      case Some(n) => "[" ++ n.toString ++ "] " ++ eltType.toString
-      case None => "array of " ++ eltType.toString
+      case None => "[unknown] " ++ eltType.toString


revert this back

Kronos3 · 2026-08-05T22:52:27Z

-  includingLoc: Option[Location] = None /* Location where this location is included */
+  /* Location where this location is included */
+  includeLoc: Option[Location] = None,
+  /* Location where this location is included */


Kronos3 · 2026-08-05T22:52:51Z

  pos: Position, /* The position */
-  includingLoc: Option[Location] = None /* Location where this location is included */
+  /* Location where this location is included */
+  includeLoc: Option[Location] = None,


Make into includingLoc and expandingLoc

bocchino · 2026-08-06T16:27:06Z

+        // Enum symbol: if this is in scope, then we are in
        // the enum definition, so it already has a type
        case Symbol.EnumConstant(node) => Right(a)
+        // Template parameter symbol: we are already inside the template expansion


Suggested change

// Template parameter symbol: we are already inside the template expansion

// Template constant argument symbol: we are already inside the template expansion

bocchino · 2026-08-06T16:28:27Z

+    val scope = expansion.scope
+
+    // We do use-analysis on the scope of the definition + param scope
+    // This is a bit unorthadox but we need to build a new nested scope from scratch


Suggested change

// This is a bit unorthadox but we need to build a new nested scope from scratch

// This is a bit unorthodox but we need to build a new nested scope from scratch

bocchino · 2026-08-06T16:29:14Z

+    }
+  }
+
+  // FIXME(tumbar) This is probably not needed


Let's fix this in this PR or open an issue to track it.

bocchino · 2026-08-06T16:29:55Z

+  defNode: Ast.Annotated[AstNode[Ast.DefModuleTemplate]],
+  /** The AST node expanding the template */
+  expansion: Ast.Annotated[AstNode[Ast.SpecTemplateExpand]],
+  /** Concrete parameters given to this template during expansion */


Suggested change

/** Concrete parameters given to this template during expansion */

/** Arguments bound to the parameters of this template during expansion */

bocchino · 2026-08-06T16:30:26Z

+  expansion: Ast.Annotated[AstNode[Ast.SpecTemplateExpand]],
+  /** Concrete parameters given to this template during expansion */
+  params: Map[String, TemplateArgSymbol],
+  /** Scope where parameter symbols are entered */


Suggested change

/** Scope where parameter symbols are entered */

/** Scope where template argument symbols are entered */

bocchino · 2026-08-06T16:34:25Z

+        expr.data match {
+          case Ast.ExprIdent(e) => Right(List(AstNode.create(e, expr.id)))
+          case Ast.ExprDot(e, id) => for (left <- exprToIdentList(e)) yield left :+ id
+          // TODO(tumbar) Make error messages specific to the parameter type


Let's fix this TODO here or open an issue to track it.

bocchino · 2026-08-06T16:34:59Z

      QualIdent.NodeList.split(nodeList) match {
        case (Nil, name) => QualIdent.Unqualified(name.data)
        case (qualifier, name) => {
+          // TODO(tumbar) Don't duplicate node ids


Let's fix this TODO here or open an issue to track it.

Kronos3 added 18 commits November 6, 2025 16:25

Template expansion is working

a3e012e

Iteratively expand templates

e10090e

Working template semantics

78e9a17

Progress dump

c7a89f1

Reworked struct conversion to include defaults

735fca2

Merge remote-tracking branch 'origin/templates-spec' into template-se…

581b62b

…mantics

Started on interface parameters

cd0606c

Merge remote-tracking branch 'origin/template-syntax' into template-s…

4cd0a25

…emantics

Fix fpp writer

0aa2eb1

Update check tests

c8657b5

Implemented deep/recursive template expansion/ast cloning

490c841

Duplicate nodes on expand

5039fb8

Cleanup tests

00de398

Cleanup a fpp-check test

6bf37b8

Flatten template interface instance symbols and fix enter template sy…

06b7a1e

…mbols for nested template expansion

Clean up error message

ea79ebe

Add more test cases

62fc2e9

Merge remote-tracking branch 'origin/feature/templates' into template…

5fdf152

…-semantics and fix it up

Kronos3 force-pushed the template-semantics branch from 7528e98 to 5fdf152 Compare June 21, 2026 15:34

Fix some implied uses checks from merge

6950e36

Kronos3 marked this pull request as ready for review June 21, 2026 15:56

Kronos3 added 2 commits June 21, 2026 10:55

Fix up some topology resolution issues

6e94509

Fix up interface instance resolution with template args

477c807

Kronos3 mentioned this pull request Jul 9, 2026

Revise Common Type Specification For Concrete Arrays #1034

Closed

Kronos3 added 2 commits July 31, 2026 08:16

Add a pass to resolve unknown anonymous arrays

e7b5fa3

Merge remote-tracking branch 'origin/feature/templates' into template…

5aa36d6

…-semantics

Kronos3 added 8 commits July 31, 2026 09:25

Fix spectest

0340627

Fix another test

142cb06

Fix sizeof() expand

b1d19da

Disallow nested template definitions, fix up alias tests, regen refs

aaced7a

Fix up syntax tests

af4d99c

Fix format

2b59ab3

Clean up change

1a2f12b

Disallow template composition

8ddb827

Kronos3 mentioned this pull request Aug 1, 2026

Disallow template composition #1053

Merged

Merge pull request #1053 from nasa/template-semantics-remove-iter

9efc6c7

Disallow template composition

Kronos3 commented Aug 5, 2026

View reviewed changes

bocchino reviewed Aug 6, 2026

View reviewed changes

	// Template parameter symbol: we are already inside the template expansion
	// Template constant argument symbol: we are already inside the template expansion

	// This is a bit unorthadox but we need to build a new nested scope from scratch
	// This is a bit unorthodox but we need to build a new nested scope from scratch

	/** Concrete parameters given to this template during expansion */
	/** Arguments bound to the parameters of this template during expansion */

	/** Scope where parameter symbols are entered */
	/** Scope where template argument symbols are entered */

Uh oh!

Conversation

Kronos3 commented May 27, 2026

Uh oh!

Kronos3 commented Jul 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Test case

Error Diagnosis

Cause

Fix

Implementation note!

Uh oh!

bocchino commented Jul 9, 2026

Uh oh!

bocchino commented Jul 9, 2026

Uh oh!

bocchino commented Jul 9, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Kronos3 Aug 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bocchino Aug 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Kronos3 commented Jul 9, 2026 •

edited

Loading

Kronos3 Aug 5, 2026 •

edited

Loading

bocchino Aug 6, 2026 •

edited

Loading