IS_VALID_BCD and IS_VALID false datatype validation

[rarris]

PROGRAM mainProg
VAR
   W : UINT;
END_VAR
   IS_VALID(W);
   IS_VALID_BCD(W);
END_PROGRAM

this code is successfully compiled, no error but IS_VALID_BCD accepts only ANY_BIT datatype and IS_VALID only ANY_REAL

[yingmanwumen]

Hello, after printing the corresponding compiling contexts, I find that IS_VALID is annotated to IS_VALID__REAL, and IS_VALID_BCD is annotated to IS_VALID_BCD__BOOL! This is strange for me because I only see the definitions of IS_VALID_BCD__BYTE, IS_VALID_BCD__WORD, IS_VALID_BCD__DWORD and IS_VALID_BCD_LWORD under lib/stdlb/.

There should be something wrong when annotating or linking...

[yingmanwumen]

Hello, after printing the corresponding compiling contexts, I find that IS_VALID is annotated to IS_VALID__REAL, and IS_VALID_BCD is annotated to IS_VALID_BCD__BOOL! This is strange for me because I only see the definitions of IS_VALID_BCD__BYTE, IS_VALID_BCD__WORD, IS_VALID_BCD__DWORD and IS_VALID_BCD_LWORD under lib/stdlb/.

There should be something wrong when annotating or linking...

I've tried the following code:

FUNCTION TEST_ANY_BIT <T: ANY_BIT>
VAR_INPUT
  IN: T;
END_VAR
END_FUNCTION

PROGRAM TEST_ANY_BIT_PROG
VAR
  W: UINT;
END_VAR
TEST_ANY_BIT(W);
END_PROGRAM

And the compiler outputs:

error[E071]: An error occurred during linking: lld: error: undefined symbol: TEST_ANY_BIT__BOOL
>>> referenced by test2.st
>>>               /tmp/.tmpcEz0Bu/test2.st.o:(TEST_ANY_BIT_PROG)

Still troubleshooting......

[ghaith]

So the way we do generic is we look for the types you used in your code, and annotate the method you called with your types and look for it. Let's say you have the following code:

FUNCTION main
VAR
  x : BOOL;
END_VAR
  IS_VALID_BCD(x);
END_FUNCTON

We will look for a function called IS_VALID_BCD__BOOL which is in this case not defined in the standard functions library. The lookup only happens on types we support, so in this case the IS_VALID_BCD is only defined on the ANY_BIT generic type, meaning only types BYTE, WORD, DWORD, and LWORD are candidates for BCD (We have ignored BOOL here). Same applies for the IS_VALID method, it only applies to REAL and LREAL because it is answering the "Is this a valid floating point number" question. I'm assuming we could define the BOOL variant as a function that returns 0 or 1 since it's technically an ANY_BIT.

@yingmanwumen Is the bistable functions you used the one from our standard lib? I don't see any references to the IS_VALID_BCD__BOOL function used inside it.

@rarris I'm not sure what we should do here, should we support the IS_VALID_BCD for Integers? It does not really make sense but I know that codesys usually treats ANY_BIT and ANY_NUMBER the same.. I don't see a reason to change the IS_VALID behavior, because for anything other than a floating point number, the answer of this method is TRUE

[ghaith]

Hello, after printing the corresponding compiling contexts, I find that IS_VALID is annotated to IS_VALID__REAL, and IS_VALID_BCD is annotated to IS_VALID_BCD__BOOL! This is strange for me because I only see the definitions of IS_VALID_BCD__BYTE, IS_VALID_BCD__WORD, IS_VALID_BCD__DWORD and IS_VALID_BCD_LWORD under lib/stdlb/. There should be something wrong when annotating or linking...

I've tried the following code:

FUNCTION TEST_ANY_BIT <T: ANY_BIT>
VAR_INPUT
  IN: T;
END_VAR
END_FUNCTION

PROGRAM TEST_ANY_BIT_PROG
VAR
  W: UINT;
END_VAR
TEST_ANY_BIT(W);
END_PROGRAM

And the compiler outputs:

error[E071]: An error occurred during linking: lld: error: undefined symbol: TEST_ANY_BIT__BOOL
>>> referenced by test2.st
>>>               /tmp/.tmpcEz0Bu/test2.st.o:(TEST_ANY_BIT_PROG)

Still troubleshooting......

So for me the problem here is we are looking for a BOOL when we should not have compiled in the first place because ANY_BIT does not include UINT. In the normal case (If you use either WORD in your TEST_ANY_BIT_PROG or ANY_NUM in your TEST_ANY_BIT you will get further, we will then be looking for a function TEST_ANY_BIT__WORD or TEST_ANY_BIT__UINT depending on the signature. Which you would need to implement. We do not (yet) auto generate the generics we only expect them to be implemented.

[ghaith]

Knowing that the UINT is resolving as BOOL kind of explains why there's an undefined IS_VALID_BCD__BOOL although I still didn't find the place where we used a IS_VALID_BCD on a UINT. Maybe there are other cases where we resolve to BOOL

[yingmanwumen]

So the way we do generic is we look for the types you used in your code, and annotate the method you called with your types and look for it. Let's say you have the following code:

FUNCTION main
VAR
  x : BOOL;
END_VAR
  IS_VALID_BCD(x);
END_FUNCTON
```I don't see a reason to change the IS_VALID behavior, because for anything other than a floating point number, the answer of this method is TRUE

We will look for a function called `IS_VALID_BCD__BOOL` which is in this case not defined in the standard functions library. The lookup only happens on types we support, so in this case the `IS_VALID_BCD` is only defined on the `ANY_BIT` generic type, meaning only types `BYTE`, `WORD`, `DWORD`, and `LWORD` are candidates for BCD (We have ignored `BOOL` here). Same applies for the `IS_VALID` method, it only applies to `REAL` and `LREAL` because it is answering the "Is this a valid floating point number" question. I'm assuming we could define the `BOOL` variant as a function that returns `0` or `1` since it's technically an `ANY_BIT`.

@yingmanwumen Is the bistable functions you used the one from our standard lib? I don't see any references to the `IS_VALID_BCD__BOOL` function used inside it.

@rarris I'm not sure what we should do here, should we support the `IS_VALID_BCD` for Integers? It does not really make sense but I know that codesys usually treats `ANY_BIT` and `ANY_NUMBER` the same.. I don't see a reason to change the `IS_VALID` behavior, because for anything other than a floating point number, the answer of this method is `TRUE`

Hello, I tried to use the -c compiling flag to output an object file from a file called test.st, and plc produced that bistable.st.o, idk why. The whole command looks like:

./rusty/target/release/plc -i ./rusty/libs/stdlib/iec61131-st/*.st ./test.st -c 

And in my point of view, although BOOL looks similar to BIT, it's actually not a BIT. Just my own thought :)

It seems like the behavior of IS_VALID is normal because there's an implicit type casting from integer to float.

[yingmanwumen]

Knowing that the UINT is resolving as BOOL kind of explains why there's an undefined IS_VALID_BCD__BOOL although I still didn't find the place where we used a IS_VALID_BCD on a UINT. Maybe there are other cases where we resolve to BOOL

I'm digging into source codes and seeking for making contribution ^_^ There could be a further compiling hint when trying to pass an argument whose type is not implemented to a parameter with generic type, or we could stop compilation if we cannot find the corresponding signature.

For example, we may stop compilation and print a diagnostics when invoking IS_VALID_BCD(x) where x is a number for we cannot find IS_VALID_BCD__BOOL and BOOL is not a member of ANY_BIT currently.

[ghaith]

Hello, I tried to use the -c compiling flag to output an object file from a file called test.st, and plc produced that bistable.st.o, idk why. The whole command looks like:

./rusty/target/release/plc -i ./rusty/libs/stdlib/iec61131-st/*.st ./test.st -c 

Yes ok this clears it up, I think it would be best to wrap the blog with quotes that the compiler does the glob expansion instead of your terminal: ./rusty/target/release/plc -i "./rusty/libs/stdlib/iec61131-st/*.st" ./test.st -c what happened in your case is only the first match of the glob got the -i flag so you ended up with library functions being compiled. Which is not a big deal if you were not using them. The BOOL appearing as an undefined symbol is because we resolved UINT as BOOL for some reason.

And in my point of view, although BOOL looks similar to BIT, it's actually not a BIT. Just my own thought :)

Yes I kind of agree here. I'm assuming this is why we never added it.

It seems like the behavior of IS_VALID is normal because there's an implicit type casting from integer to float.

Indeed, so I assume the only thing to fix here is to make sure the generic conversion does not treat the unsigned integers as a BOOL so we end up in the type nature validation instead.

[ghaith]

Knowing that the UINT is resolving as BOOL kind of explains why there's an undefined IS_VALID_BCD__BOOL although I still didn't find the place where we used a IS_VALID_BCD on a UINT. Maybe there are other cases where we resolve to BOOL

I'm digging into source codes and seeking for making contribution ^_^ There could be a further compiling hint when trying to pass an argument whose type is not implemented to a parameter with generic type, or we could stop compilation if we cannot find the corresponding signature.

For example, we may stop compilation and print a diagnostics when invoking IS_VALID_BCD(x) where x is a number for we cannot find IS_VALID_BCD__BOOL and BOOL is not a member of ANY_BIT currently.

Contributions would be awesome thanks! Regarding the validation, I think it would be great. I thought we already had type nature validation, but it seems unsupported natures are being treated as the smallest possible type for the nature, (BOOL, USINT..) which then causes another validation to trigger. In your example, if you switch UINT with STRING you will get a conversion error.

FUNCTION TEST_ANY_BIT <T: ANY_BIT>
VAR_INPUT
  IN: T;
END_VAR
END_FUNCTION

PROGRAM TEST_ANY_BIT_PROG
VAR
  W: STRING; (* was UINT *)
END_VAR
TEST_ANY_BIT(W);
END_PROGRAM

[yingmanwumen]

Knowing that the UINT is resolving as BOOL kind of explains why there's an undefined IS_VALID_BCD__BOOL although I still didn't find the place where we used a IS_VALID_BCD on a UINT. Maybe there are other cases where we resolve to BOOL

I'm digging into source codes and seeking for making contribution ^_^ There could be a further compiling hint when trying to pass an argument whose type is not implemented to a parameter with generic type, or we could stop compilation if we cannot find the corresponding signature. For example, we may stop compilation and print a diagnostics when invoking IS_VALID_BCD(x) where x is a number for we cannot find IS_VALID_BCD__BOOL and BOOL is not a member of ANY_BIT currently.

Contributions would be awesome thanks! Regarding the validation, I think it would be great. I thought we already had type nature validation, but it seems unsupported natures are being treated as the smallest possible type for the nature, (BOOL, USINT..) which then causes another validation to trigger. In your example, if you switch UINT with STRING you will get a conversion error.

FUNCTION TEST_ANY_BIT <T: ANY_BIT>
VAR_INPUT
  IN: T;
END_VAR
END_FUNCTION

PROGRAM TEST_ANY_BIT_PROG
VAR
  W: STRING; (* was UINT *)
END_VAR
TEST_ANY_BIT(W);
END_PROGRAM

Hello, I tested the same code in Codesys and find that in CodeSys UINT cannot be converted to both ANY_REAL and ANY_BIT!

And I noticed that issue #1030 and #1044 are almost the same with this issue. All of these problems are caused by

https://github.com/PLC-lang/rusty/blob/f51b011bf3c2378baecda9eee014457390d7a10a/src/resolver/generics.rs#L460

, and I think it may be a typing mistake: the smallest type of ANY_BIT should be BYTE_TYPE instead of BOOL_TYPE!

Even if we use BYTE_TYPE here, the problem still exists: since UINT or any numberic type can be convert into BYTE (https://github.com/PLC-lang/rusty/blob/f51b011bf3c2378baecda9eee014457390d7a10a/src/typesystem.rs#L247C1-L250C14), IS_VALID_BCD(w) will still pass compilation without any warning or error (CodeSys tells you that the convertion from UINT to ANY_REAL or ANY_BYTE is unsupported, but it's ok to convert UINT to REAL or BYTE).

I wonder:

To be compatible with CodeSys, If we should follow the same behavior.

We could acheive it by reserving some messages of generic types for validator, because the corresponding generic type instance is determined after annotation(for example, IS_VALID_BCD__BOOL), and the validator cannot differ this instance from other non-generic type. To put it in another way, the validator isn't aware of generic types, so the implicit type casting to ANY_BYTE which is not allowed in CodeSys happens.

:)

[ghaith]

I would vote to add validators in that case. I'm not sure why we had the ANY_BIT include BOOL maybe it's part of the Norm, but i'll have to look. With our recent change to error reporting validator can be turned off if we want a different behavior but I think the Codesys behavior is correct in these cases. It might however end up strange in cases where we were expecting the implicit casting. Can't think of a case now but i'm sure it will pop up =) If you do end up adding a validator here, you would need to add a new error number and add it to the error codes in the plc_diagnostics crate, but don't worry about that, we'll tackle it when we get there.

[yingmanwumen]

It looks like I found the way to measure it. 🤠 Some outputs during try:

root@ymwm:~/rusty# ./target/debug/plc ../test2.st
error[E037]: Invalid assignment: cannot assign 'UINT' to 'ANY_BIT'
   ┌─ ../test2.st:11:1
   │
11 │ TEST_ANY_BIT(W);
   │ ^^^^^^^^^^^^ Invalid assignment: cannot assign 'UINT' to 'ANY_BIT'

Error: Compilation aborted due to critical errors.
Hint: You can use `plc explain <ErrorCode>` for more information

root@ymwm:~/rusty# cat ../test2.st
FUNCTION TEST_ANY_BIT  <T: ANY_BIT>
VAR_INPUT
  IN: T;
END_VAR
END_FUNCTION

PROGRAM TEST_ANY_BIT_PROG 
VAR
  W: UINT;
END_VAR
TEST_ANY_BIT(W);
END_PROGRAM

But to complete it, it will require more time hhhh. I think I can do it!