copilot icon indicating copy to clipboard operation
copilot copied to clipboard

Repeated declarations of functions in generated C code if a function is triggered twice in specification

Open antanas-kalkauskas-sensmetry opened this issue 2 years ago • 5 comments

If we have a Copilot specification where we have multiple conditions for triggering the same function (but perhaps we are providing different arguments), then in generated C code we would have multiple declarations of trigger functions and multiple definitions of helper functions and the generated code could not be compiled without modifications.

Example

Suppose we would like to trigger the switchOffPower(..) function if the current of one of the components is too large. We would also like to pass the value of the signal and the index of the component to the switchOffPower(..).

{-# LANGUAGE RebindableSyntax #-}

module RepeatedDeclarationsExample where
import Language.Copilot

import Copilot.Compile.C99

current1 :: Stream Float 
current1 = extern "current_signal1" Nothing 

current2 :: Stream Float 
current2 = extern "current_signal2" Nothing

spec = do
    trigger "switchOffPower" (current1 > 0.8) [arg current1, arg (constant 0 :: Stream Word8)]
    trigger "switchOffPower" (current2 > 0.8) [arg current2, arg (constant 1 :: Stream Word8)]

main :: IO ()
main = do
    reify spec >>= compile "repeated_declarations_example"

Then the repeated_declarations_example.h declares switchOffPower(..) twice:

extern float current_signal1;
extern float current_signal2;
void switchOffPower(float switchOffPower_arg0, uint8_t switchOffPower_arg1);
void switchOffPower(float switchOffPower_arg0, uint8_t switchOffPower_arg1);
void step(void);

And repeated_declarations_example.c contains multiple definitions of bool switchOffPower_guard(void), float switchOffPower_arg0(void), uint8_t switchOffPower_arg1(void) .

#include <stdint.h>
#include <stdbool.h>
#include <string.h>
#include <stdlib.h>
#include <math.h>

#include "repeated_declarations_example.h"

static float current_signal1_cpy;
static float current_signal2_cpy;

bool switchOffPower_guard(void) {
  return (current_signal1_cpy) > ((float)(0.800000011920929));
}

float switchOffPower_arg0(void) {
  return current_signal1_cpy;
}

uint8_t switchOffPower_arg1(void) {
  return (uint8_t)(0);
}

bool switchOffPower_guard(void) {
  return (current_signal2_cpy) > ((float)(0.800000011920929));
}

float switchOffPower_arg0(void) {
  return current_signal2_cpy;
}

uint8_t switchOffPower_arg1(void) {
  return (uint8_t)(1);
}

void step(void) {
  (current_signal1_cpy) = (current_signal1);
  (current_signal2_cpy) = (current_signal2);
  if ((switchOffPower_guard)()) {
    {(switchOffPower)(((switchOffPower_arg0)()), ((switchOffPower_arg1)()));}
  };
  if ((switchOffPower_guard)()) {
    {(switchOffPower)(((switchOffPower_arg0)()), ((switchOffPower_arg1)()));}
  };
}

Indeed, that is an error in the spec.

At present, we consider it an error whose detection is delegated on the C compiler.

If you consider the process of compilation as indivisible (Haskell -> running -> GCC), then the error is being caught. What happens is that it is not caught in the first stage, it is caught in the third (when you try to compile the C code).

ivanperez-keera avatar Feb 28 '22 11:02 ivanperez-keera

I take that back. In principle, it should be an error only if the arguments were different, but not if they are the same.

ivanperez-keera avatar Feb 28 '22 14:02 ivanperez-keera

I took a quick stab at this. Avoiding multiple declarations of the handler functions if they have the same arguments is easy to do. Avoiding multiple declarations of the arguments or argument generation functions is a bit trickier.

There are two potentially orthogonal improvements to add to Copilot: 1) avoid multiple declarations of the same generation functions (this connects to #297), and 2) use different names for arguments to the same handling function when they come from different triggers with the same handling function name.

ivanperez-keera avatar Aug 15 '23 16:08 ivanperez-keera

I tried this again and I have a crude implementation that seems to work. There's an architecturally cleaner way of structuring the code (that's way I mean by crude).

I don't want to not rush putting this in the Jan 7, 2024 release; I'd rather discuss it well before it's merged.

We'll talk about it next week during our next planning meeting. If we can, we'll add this to Copilot 3.19, to be released on March 7, 2024.

I'll keep you updated.

ivanperez-keera avatar Jan 03 '24 20:01 ivanperez-keera

We just had our planning meeting and this issue is likely to be included in the next release. You should see an update on this in a few days.

ivanperez-keera avatar Jan 08 '24 19:01 ivanperez-keera