PLC-lang / rusty

Structured Text Parser and LLVM Frontend
GNU Lesser General Public License v3.0
203 stars 52 forks source link

Methods cannot resolve non class functions #402

Closed ghaith closed 1 year ago

ghaith commented 2 years ago

Describe the bug Given an external function and a class method, the validator will always report a reference issue with the function if called from that method

To Reproduce

FUNCTION foo : DINT
END_FUNCTION

CLASS baz 
METHOD x : DINT
    foo();
END_METHOD
END_CLASS
r2k-in-the-vortex commented 1 year ago

Someone probably already fixed this, unable to reproduce.

{external} 
FUNCTION puts : DINT
VAR_INPUT {ref}
    text : STRING;
END_VAR
END_FUNCTION

{external} 
FUNCTION sprintf : DINT
VAR_INPUT {ref}
    str     : STRING;
    format  : STRING;
END_VAR
VAR_INPUT
    i      : DINT;
END_VAR
END_FUNCTION

FUNCTION DINT_TO_STRING : STRING
    VAR_INPUT
        i      : DINT;
    END_VAR
    sprintf(DINT_TO_STRING, '%d', i);
END_FUNCTION

FUNCTION foo : DINT
    foo := 42;
END_FUNCTION

CLASS baz 
METHOD x : DINT
    x := foo();
END_METHOD
END_CLASS

FUNCTION main : DINT
    VAR
        Variable    : baz; (* Comment *)
        x           : DINT;
        test        : STRING;
    END_VAR

    x := Variable.x();
    test := DINT_TO_STRING(x);
    puts(test);
    puts('it works just fine');
END_FUNCTION

produces

vscode ➜ /workspaces/rusty $ target/release/rustyc examples/bug.st --linker=cc && ./bug
42
it works just fine
riederm commented 1 year ago

thx @r2k-in-the-vortex for your efforts, probably you are right. we halted the implementation for classes for the moment because we first want to fully support the pre 2013 extensions of the iec61131-3 standard.

But it looks like we're very close to feature completeness (for <2013) so we will tackle all the class issues soon.

i'll look at it tomorrow, maybe I can find the commit that fixed this and link it, otherwise we can just close this.

r2k-in-the-vortex commented 1 year ago

If root of the problem is unknown and so is the fix, does it require a testcase to be added so the problem wouldn't sneak back in sometime in future?

ghaith commented 1 year ago

@r2k-in-the-vortex Yes I think we should have a test case covering it even if we are not yet officially supporting methods, I think @riederm can upload a commit if he is verifying this today, or I can add a test otherwise.