rusty icon indicating copy to clipboard operation
rusty copied to clipboard

false STRING assignment leads to panicked

Open rarris opened this issue 1 year ago • 13 comments

the following code

PROGRAM mainProg
VAR
  x3 : STRING;
END_VAR
  x3 := 11;
END_PROGRAM

will generate the following error with panicked:

mainProg.st:5:1:{5:1-5:9}: warning[E037]: Invalid assignment: cannot assign 'DINT' to 'STRING' thread '' panicked at src\codegen\llvm_typesystem.rs:187:18: internal error: entered unreachable code: Cannot cast integer value to STRING note: run with RUST_BACKTRACE=1 environment variable to display a backtrace

rarris avatar Mar 20 '24 10:03 rarris

This seems to be a diagnostics severity problem. Invalid assignment should be an error, not a warning - then we also would not enter codegen and no panic would be happening.

E037 should default to Severity::Error - are you using a custom config?

mhasel avatar Mar 20 '24 10:03 mhasel

Closing this because by default it's an error as can be seen here; feel free to re-open if it's not the case @rarris

volsa avatar Mar 21 '24 15:03 volsa

here is about the panicked text, not the error itself we should solve/remove all the panicked errors output text!

@mhasel E037 should default to Severity::Error - are you using a custom config?

yes

rarris avatar Mar 22 '24 07:03 rarris

Reopening because we should not reach a panic state.

ghaith avatar Mar 22 '24 07:03 ghaith

This seems to be a diagnostics severity problem. Invalid assignment should be an error, not a warning - then we also would not enter codegen and no panic would be happening.

E037 should default to Severity::Error - are you using a custom config?

I still think we should not panic but exist gracefully although this is a bigger topic. Should we make some errors non-changeable like the E37 or the one for references? This used to be the case with the critical errors

ghaith avatar Mar 22 '24 07:03 ghaith

Should we make some errors non-changeable like the E37 or the one for references?

Big fan of this, any crucial error code should have an immutable severity imo

volsa avatar Mar 22 '24 08:03 volsa

how would the user be able to recognize which error codes are configurable and which are not? it feels strange for me when there is no recognizable difference for the user between the errors but if the id is put in the configuration as warning nothing happens. it would feel buggy then imo

tisis2 avatar Mar 22 '24 09:03 tisis2

this is a very good point @tisis2 we should have 2 error lists/categories if a error is norm defined error and codesys error, that cannot be configured!

rarris avatar Mar 22 '24 09:03 rarris

The way rust does this is they have 2 types of errors, hard errors that cannot be changed, these are the E ones, and then named errors that can be changed in severity. These are ones things like deprecation warnings or clippy issues, they call them lints. https://rustc-dev-guide.rust-lang.org/diagnostics.html We can try going that route.

ghaith avatar Mar 22 '24 09:03 ghaith

this is a very good point @tisis2 we should have 2 error lists/categories if a error is norm defined error and codesys error, that cannot be configured!

I would say if an error can lead to a panic, it's non configurable. Things like this error here or the reference error.

ghaith avatar Mar 22 '24 09:03 ghaith

But how do we show this? Do we make these errors have different codes? And give out an error if the error_config.json ignores one of them? We could introduce a "critical" section to the registry, and on loading a custom error config, if any of the critical errors gets a reduced severity we through an error and don't compile.

ghaith avatar Mar 22 '24 09:03 ghaith

maybe something like that... we could also reflect it in the error id and differ between CRIT... and E... for example.

but to be honest, i think currently all this is nice to have... if a user changes an error to a warning and gets a panic after that step, the user might recognize that it is about the configuration that was changed...

tisis2 avatar Mar 22 '24 09:03 tisis2

we can make a template where only at certain error codes can the severity be changed

for example:

{
    "not configurable": [
        E037,
        E....
        E....
     ],
    "warning": [
     ],
    "ignore": [
     ]
}

rarris avatar Mar 22 '24 09:03 rarris