rusty icon indicating copy to clipboard operation
rusty copied to clipboard

show warning for BOOL-INT downcast

Open rarris opened this issue 1 year ago • 4 comments

the following code

PROGRAM mainProg
VAR
  x1 : BOOL;
  x2 : INT;
END_VAR
  x1 := 20; 
  x2 := TRUE;
END_PROGRAM

will generate no warning, no error!

It will be nice to have minimum a warning and later to adjust easier the error_config file for codesys where this a error is.

rarris avatar Mar 20 '24 09:03 rarris

I don't really see x2 := TRUE as a big issue, it'll just have the value 1. Your other example will also retain it's value, so no downcast is happening. Any non-zero value will evaluate to TRUE.

And if you rewrite your assignment to x1 := x2, in other words INT to BOOL, the compiler will warn about the downcast.

The only problem I can see here is if you assign a negative number to a boolean, which will underflow u8 and then reassign it into a signed variable. E.g.:


{external}
FUNCTION printf : DINT
VAR_INPUT {ref}
  format : STRING;
END_VAR
VAR_INPUT
  args : ...;
END_VAR
END_FUNCTION

FUNCTION main : DINT 
VAR
  x1 : BOOL;
  x2 : INT;
END_VAR
  x1 := -2; // will underflow u8 and have value 253

  IF x1 THEN
    printf('evaluates to true, has value %d$N', x1);
  END_IF;

  x2 := x1; // will still have value 253
  printf('value is %d', x2);
END_FUNCTION

So maybe we could warn about the implicit conversion to unsigned when assigning a signed integer to a boolean.

mhasel avatar Mar 20 '24 10:03 mhasel

x1 := 20 just feels wrong to me tbh, I'd be in favour of at least emitting a info / warning here.

volsa avatar Mar 20 '24 10:03 volsa

x1 := 20 just feels wrong to me tbh, I'd be in favour of at least emitting a info / warning here.

This is basically the same idea as #1148, just in a different context/a literal in this case. But disallowing literal integers to booleans seems sensible. This would make it an assignment validation rather than a downcast validation, though.

mhasel avatar Mar 20 '24 10:03 mhasel

I have tried a similar example with clang:

#include <stdbool.h>
#include <stdio.h>

int main() {
  bool x = 20;
  char y = x;
  char z = !x;

  printf("%d\n", x); // 1
  printf("%d\n", y); // 1
  printf("%d\n", z); // 0
  return 0;
}

We have a function expression_generator::to_i1 which should achieve this behaviour, it generates IR similar to clang. Here is a smaller example plus its IR in both ST and C:

#include <stdbool.h>
int main() {
  bool x = 20;
  char y = !x;
  return 0;
}
define dso_local i32 @main() #0 {
  %1 = alloca i32, align 4
  %2 = alloca i8, align 1
  %3 = alloca i8, align 1
  store i32 0, i32* %1, align 4
  store i8 1, i8* %2, align 1
  %4 = load i8, i8* %2, align 1
  %5 = trunc i8 %4 to i1
  %6 = xor i1 %5, true
  %7 = zext i1 %6 to i32
  %8 = trunc i32 %7 to i8
  store i8 %8, i8* %3, align 1
  ret i32 0
}
FUNCTION main: DINT
VAR
    b: BOOL;
    u: USINT;
END_VAR
    b := 20;
    u := NOT b;
END_FUNCTION
define i32 @main() {
entry:
  %main = alloca i32, align 4
  %b = alloca i8, align 1
  %u = alloca i8, align 1
  store i8 0, i8* %b, align 1
  store i8 0, i8* %u, align 1
  store i32 0, i32* %main, align 4
  store i8 20, i8* %b, align 1
  %load_b = load i8, i8* %b, align 1
  %0 = icmp ne i8 %load_b, 0
  %tmpVar = xor i1 %0, true
  %1 = zext i1 %tmpVar to i8
  store i8 %1, i8* %u, align 1
  %main_ret = load i32, i32* %main, align 4
  ret i32 %main_ret
}

Guess we'd just have to make sure to call it not just for boolean expressions, but also assignments etc.. That would make our booleans actually of size i1 and BOOL x1 := 20 a downcast. Might also be worth it to check out the trunc instruction which clang is calling when assigning booleans.

mhasel avatar Mar 20 '24 15:03 mhasel