Type alias syntax & semantics #591
Conversation
|
It looks like you need to regenerate the trace info for the native build. See the instructions here: https://github.com/nasa/fpp/tree/main/compiler#building-native-binaries. |
|
Another issue I see is that the code generator is generating an empty TypesAc.cpp file. I think we should suppress the generation of this file. Does that require any changes to the CppDoc framework? |
|
@bocchino You may need to do the trace updates again, I can't install GraalVM for Java11 since the only version available from Homebrew is jdk23 and the SDKMAN/official installation method seems to only have 17, 21, and 23. We may also want to consider updating the Java version or putting the trace updates in CI |
| @@ -345,8 +365,11 @@ object Type { | |||
| } | |||
|
|
|||
| /** Check for type identity */ | |||
| def areIdentical(t1: Type, t2: Type): Boolean = { | |||
| def areIdentical(t1Aliased: Type, t2Aliased: Type): Boolean = { | |||
There was a problem hiding this comment.
This change doesn't conform to the spec. The spec says that an alias type is identical to itself, not to its underlying type. See here: https://nasa.github.io/fpp/fpp-spec.html#Type-Checking_Identical-Types.
| override def defAliasTypeAnnotatedNode(a: Analysis, node: Ast.Annotated[AstNode[Ast.DefAliasType]]) = { | ||
| val (_, node1, _) = node | ||
| val data = node1.data | ||
| for { |
There was a problem hiding this comment.
This for...yield can be simplified to a call to typeNameNode.
| @@ -136,7 +136,9 @@ case class CppWriterState( | |||
| for { | |||
| fileName <- sym match { | |||
| case _: Symbol.AbsType => | |||
| if isBuiltInType(name) then None else Some(name) | |||
| if isBuiltInType(name) then None else Some(name) | |||
| // TODO(tumbar) Compute the includes for alias types | |||
There was a problem hiding this comment.
Can we resolve this TODO before merging this PR? I think we just need to and add TypeAc to the filename pattern.
There was a problem hiding this comment.
Or is the issue that we're not generating code for alias types, so we can't generate the include header? If so, can the comment be clearer?
| @@ -16,6 +17,10 @@ case class TypeCppWriter( | |||
| override def absType(s: CppWriterState, t: Type.AbsType) = | |||
| s.writeSymbol(Symbol.AbsType(t.node)) | |||
|
|
|||
| override def aliasType(s: CppWriterState, t: Type.AliasType) = | |||
| // TODO(tumbar) Keep the aliased type name in codegen | |||
There was a problem hiding this comment.
Please clarify that we're doing it this way for now because we're not generating the type alias code.
| @@ -3,6 +3,6 @@ package fpp.compiler.util | |||
| /** The compiler version */ | |||
| object Version { | |||
|
|
|||
| val v = "[unknown version]" | |||
There was a problem hiding this comment.
Please revert this change. This is a glitch in the way the installer works -- if you abort the installation, it can corrupt this file.
| @@ -1,6 +1,7 @@ | |||
| #!/bin/sh -e | |||
|
|
|||
| fpp_to_cpp=../../../../../../bin/fpp-to-cpp | |||
| fprime_gcc=../../../../../../scripts/fprime-gcc | |||
Noted. The trace updates are supposed to be in CI, but they have never worked, and instead of wrangling CI I have just updated them manually. @LeStarch do you see any issue with updating the FPP Java environment from Java 11 to Java 17? We would need to do this in the GitHub actions. |
Part of #113. This finishes up some changes made in #549 for syntax & semantics implementation. It adds a test to generate a struct that references a type alias.
No C++ code is generated yet for type aliases. In model elements that use type aliases, the type aliases are converted to their underlying types before generating C++ code.