Merge branch 'develop-fix-trigger-arg-arrays'. Close #431.

**Description**

Copilot's C99 backend generates incorrect code. In the generated code,
the functions that generate values passed as arguments to trigger
functions may return pointers to stack-allocated arrays, so the memory
may be used for something else (or freed).

Instead, the structure of the implementation should guarantee that the
arrays are always allocated.

**Type**

- Bug: generated C code may be incorrect.

**Additional context**

- Bug #386 was initially opened for the same reason, but only addressed
  the `_gen` functions generated by Copilot. Other functions generated
  by Copilot also return pointers to arrays and can fail for the same
  reason.

**Requester**

- Ryan Scott (Galois)

**Method to check presence of bug**

The following Dockerfile, and accompanying Copilot and C code, check
that passing a stream of arrays as an argument to a trigger, when
compiled with and without optimizations, produces the same results, in
which case it prints the message Success. The docker image produces a
diff of the two outputs if not:

```
--- Dockerfile-verify-431
FROM ubuntu:trusty

RUN apt-get update

RUN apt-get install --yes software-properties-common
RUN add-apt-repository ppa:hvr/ghc
RUN apt-get update

RUN apt-get install --yes ghc-8.6.5 cabal-install-2.4
RUN apt-get install --yes libz-dev

ENV PATH=/opt/ghc/8.6.5/bin:/opt/cabal/2.4/bin:$PWD/.cabal-sandbox/bin:$PATH

RUN cabal update
RUN cabal v1-sandbox init
RUN cabal v1-install alex happy
RUN apt-get install --yes git

ADD Array.hs /tmp/Array.hs
ADD array_driver.c /tmp/array_driver.c

SHELL ["/bin/bash", "-c"]
CMD git clone $REPO && cd $NAME && git checkout $COMMIT && cd .. \
  && cabal v1-install $NAME/copilot**/ \
  && cabal v1-exec -- runhaskell /tmp/Array.hs \
  && gcc -O0 /tmp/array_driver.c array.c -I. -o main-no-op && ./main-no-op > no-optimizations \
  && gcc -O /tmp/array_driver.c array.c -I. -o main-op && ./main-op > with-optimizations \
  && diff no-optimizations with-optimizations \
  && echo "Success"

--- Array.hs
{-# LANGUAGE DataKinds #-}
module Main where

import           Copilot.Compile.C99
import           Language.Copilot
import qualified Prelude             as P

spec :: Spec
spec = trigger "f" true [arg stream]
  where
    stream :: Stream (Array 2 Int16)
    stream = constant (array [3,4])

main :: IO ()
main = reify spec >>= compile "array"

--- array_driver.c
#include <stdint.h>
#include <stdio.h>

#include "array_types.h"
#include "array.h"

void f (int16_t f_arg0[(2)]) {
  printf("([%d,%d])\n", f_arg0[0], f_arg0[1]);
}

int main(void) {
  int i = 0;

  printf("f:\n");

  for (i=0; i<5; i++) {
    step();
  }
  return 0;
}
```


Command (substitute variables based on new path after merge):
```
$ docker run -e "REPO=https://github.com/Copilot-Language/copilot" -e "NAME=copilot" -e "COMMIT=<HASH>" -it copilot-verify-431
```

**Expected result**

Running the docker image above prints the message success, indicating
that the output structs produced with and without optimizations enabled
are the same.

**Solution implemented**

Modify generated C99 backend so that the output array in a function
producing values passed to triggers as arguments, as well as any they
call that also produces arrays, take the returned array as input. That
way, all memory is preallocated and there's no need for mallocs either
(which we want to avoid).

Identify factorization opportunity for different functions that call
generators and pattern match based on the type of the expression being
generated.

**Further notes**

None.

Author: ivanperez-keera

copilot-c99/CHANGELOG

@@ -1,6 +1,7 @@
-2023-12-17
+2023-12-19
         * Change return type of main generated for tests. (#468)
         * Print constants in tests using portable suffixes. (#471).
+        * Pass output arrays as arguments to trigger argument functions. (#431)
 
 2023-11-07
         * Version bump (3.17). (#466)

copilot-c99/src/Copilot/Compile/C99/CodeGen.hs

@@ -163,17 +163,24 @@ mkStep cSettings streams triggers exts =
     (triggerDeclns, triggerStmts) =
       unzip $ map mkTriggerCheck triggers
 
+    -- Update the value of a variable with the result of calling a function that
+    -- generates the next value in a stream expression. If the type of the
+    -- variable is an array, then we cannot perform a direct C assignment, so
+    -- we instead pass the variable as an output array to the function.
+    updateVar :: C.Ident -> C.Ident -> Type a -> C.Expr
+    updateVar varName genName (Array _) =
+      C.Funcall (C.Ident genName) [C.Ident varName]
+    updateVar varName genName _ =
+      C.AssignOp C.Assign (C.Ident varName) (C.Funcall (C.Ident genName) [])
+
     -- Write code to update global stream buffers and index.
     mkUpdateGlobals :: Stream -> (C.Decln, C.Stmt, C.Stmt, C.Stmt)
     mkUpdateGlobals (Stream sId buff _expr ty) =
       (tmpDecln, tmpAssign, bufferUpdate, indexUpdate)
         where
           tmpDecln = C.VarDecln Nothing cTy tmpVar Nothing
 
-          tmpAssign = case ty of
-            Array _ -> C.Expr $ C.Funcall (C.Ident $ generatorName sId)
-                                          [ C.Ident tmpVar ]
-            _       -> C.Expr $ C.Ident tmpVar C..= val
+          tmpAssign = C.Expr $ updateVar tmpVar (generatorName sId) ty
 
           bufferUpdate = case ty of
             Array _ -> C.Expr $ memcpy dest (C.Ident tmpVar) size
@@ -193,7 +200,6 @@ mkStep cSettings streams triggers exts =
           tmpVar   = streamName sId ++ "_tmp"
           buffVar  = C.Ident $ streamName sId
           indexVar = C.Ident $ indexName sId
-          val      = C.Funcall (C.Ident $ generatorName sId) []
           cTy      = transType ty
 
     -- Make code that copies an external variable to its local one.
@@ -250,22 +256,8 @@ mkStep cSettings streams triggers exts =
         aTmpDeclns = zipWith declare args aTempNames
           where
             declare :: UExpr -> C.Ident -> C.Decln
-            declare arg tmpVar =
-              C.VarDecln Nothing (tempType arg) tmpVar Nothing
-
-            -- Type of the temporary variable used to store values of the type
-            -- of a given expression.
-            tempType :: UExpr -> C.Type
-            tempType (UExpr { uExprType = ty }) =
-              case ty of
-                -- If a temporary variable is being used to store an array,
-                -- declare the type of the temporary variable as a pointer, not
-                -- an array. The problem with declaring it as an array is that
-                -- the `arg` function will return a pointer, not an array, and
-                -- C doesn't make it easy to cast directly from an array to a
-                -- pointer.
-                Array ty' -> C.Ptr $ transType ty'
-                _         -> transType ty
+            declare (UExpr { uExprType = ty }) tmpVar =
+              C.VarDecln Nothing (transType ty) tmpVar Nothing
 
         triggerCheckStmt :: C.Stmt
         triggerCheckStmt = C.If guard' fireTrigger
@@ -283,13 +275,14 @@ mkStep cSettings streams triggers exts =
               where
                 -- List of assignments of values of temporary variables.
                 argAssigns :: [C.Expr]
-                argAssigns = zipWith assign aTempNames args'
+                argAssigns = zipWith3 assign aTempNames aArgNames args
 
-                assign :: C.Ident -> C.Expr -> C.Expr
-                assign aTempName = C.AssignOp C.Assign (C.Ident aTempName)
+                assign :: C.Ident -> C.Ident -> UExpr -> C.Expr
+                assign aTempName aArgName (UExpr { uExprType = ty }) =
+                  updateVar aTempName aArgName ty
 
-                args'         = take (length args) (map argCall (argNames name))
-                argCall name' = C.Funcall (C.Ident name') []
+                aArgNames :: [C.Ident]
+                aArgNames = take (length args) (argNames name)
 
                 -- Build an expression to pass a temporary variable as argument
                 -- to a trigger handler.

copilot-c99/src/Copilot/Compile/C99/Compile.hs

@@ -119,9 +119,8 @@ compileC cSettings spec = C.TransUnit declns funs
         accessDecln (Stream sId buff _ ty) = mkAccessDecln sId ty buff
 
         streamGen :: Stream -> C.FunDef
-        streamGen (Stream sId _ expr ty@(Array _)) =
-          mkGenFunArray (generatorName sId) (generatorOutputArgName sId) expr ty
-        streamGen (Stream sId _ expr ty) = mkGenFun (generatorName sId) expr ty
+        streamGen (Stream sId _ expr ty) =
+          exprGen (generatorName sId) (generatorOutputArgName sId) expr ty
 
         triggerGen :: Trigger -> [C.FunDef]
         triggerGen (Trigger name guard args) = guardDef : argDefs
@@ -130,7 +129,19 @@ compileC cSettings spec = C.TransUnit declns funs
             argDefs  = zipWith argGen (argNames name) args
 
             argGen :: String -> UExpr -> C.FunDef
-            argGen argName (UExpr ty expr) = mkGenFun argName expr ty
+            argGen argName (UExpr ty expr) =
+              exprGen argName (argName ++ "_output") expr ty
+
+        -- Create a function that calculates the current value generated by an
+        -- expression `expr` of type `ty`. The generator treats arrays
+        -- specially, and the function takes an output array as a parameter.
+        -- The second identifier `outputArrName` is not used if `expr` is not an
+        -- array.
+        exprGen :: C.Ident -> C.Ident -> Expr a -> Type a -> C.FunDef
+        exprGen funName outputArrName expr ty@(Array _) =
+          mkGenFunArray funName outputArrName expr ty
+        exprGen funName _ expr ty =
+          mkGenFun funName expr ty
 
 -- | Generate the .h file from a 'Spec'.
 compileH :: CSettings -> Spec -> C.TransUnit