search

jubnzv / iec-checker

Static analysis of IEC 61131-3 programs
GNU Lesser General Public License v3.0
64 stars 12 forks source link

"FUNCTION_BLOCK PRIVATE MAIN_POU" 3:11 ParseError is Thrown #4

Closed engineerjoe440 closed 3 years ago

engineerjoe440 commented 3 years ago

Hi there!

First of all, let me say that this project looks remarkable! I'm very impressed, and I think I'm going to try to put this project to work if at all possible during my regular development.

My problem arose when I was trying the online demo with the "Elevator" example. I added the keyword "PRIVATE" to the FB declaration header as shown here:

image

and after running the linter, I saw a ParseError. I would have expected the linter to accept the PRIVATE keyword specifier, is this unsupported, or is there something else that is causing trouble for my test?

Thank you!!!

jubnzv commented 3 years ago

Hi, Joe, thank you for reporting this.

I'd like to ask you, what IDE/compiler do you use? Does it allow the use of such syntax?

As far as I know, the access specifiers like PRIVATE can only be used with the class methods, not with function blocks. What is the point of using PRIVATE in your example?

engineerjoe440 commented 3 years ago

Ah! You know.... That's a good point! I guess I wasn't thinking about the limitations of which objects could use the PRIVATE declaration.

Having only just started looking at this project, I'm curious if there are ways that more unique error messages could be developed? That way dummies such as myself wouldn't be so dumbfounded at first :P I have no idea what sort of complexity that would require, but I would imagine that more information in the error messages would always be more useful!

Thanks again for your work on this project, this looks fantastic!

jubnzv commented 3 years ago

I'm curious if there are ways that more unique error messages could be developed?

Yes, we can modify the parser to get better error messages, but I'm not sure we need this. The point is that this tool is a standalone static analyzer, so we expect that the input code was recognized as valid by some compiler. If iec-checker returns ParserError, it means that the problem is in this tool, not in the user code. So these errors only exists for the developers of the static analyzer to fix the parser.

I also want to note that in some places I intentionally violate the IEC61131-3 standard in the parser to make this tool compatible with some compilers that don't follow the standard. For example, I allow to use VAR_GLOBAL in any POU, not only in CONFIGURATION and RESOURCE entries. And in the future, if we refine the number of supported compilers, the number of violations of the standard in the parser will increase.

So I'm not in a hurry to add more readable error messages yet. Usually I can understand the meaning of ParserError based on the source code of the user.

jubnzv commented 3 years ago

Feel free to open new issues, if something doesn't work. I am primarily focused on supporting the matiec compiler and the compiler being developed in our company, so the parser may get stuck on some input data.

engineerjoe440 commented 3 years ago

Thanks again! I'm going to do some exploratory work with this tool, and I'll report any issues I run into!