Bugfix/nw23001440/omitted parameter on program call

[foresti-smeup]

Description

Manage program call with omitted entry parameters (for example, call a program with 3 entries passing only 2 values)
Rules:

• No error if called program don't use omitted entry
• Throw exception if called program try to use the omitted entry var

In the previous version, calling a program with omitted parameters always threw a missing parameter exception, even if the called program made no use of the undefined variable.

Related to:

• MULANGT90-T10_A60-P04

Checklist:

• There are tests regarding this feature
• The code follows the Kotlin conventions (run ./gradlew ktlintCheck)
• The code passes all tests (run ./gradlew check)
• There is a specific documentation in the docs directory

[lanarimarco]

You are exploiting VoidValue, which was born to handle rpg functions that returns nothing to handle this issue, this choice forced you to introduce many changes. In addition you are adding in SymbolTable something that it should not be in SymbolTable.

Wasn't there a cleaner approach?

[foresti-smeup]

You are exploiting VoidValue, which was born to handle rpg functions that returns nothing to handle this issue, this choice forced you to introduce many changes. In addition you are adding in SymbolTable something that it should not be in SymbolTable.

At first I created a new NullValue class to manage variables without value assignment. Then I preferred to use the VoidValue class because it was already present and it seemed not used in any part of the code (its methods were mostly marked as TODO). If you prefer, I can restore the NullValue class and leave the VoidValue class unchanged.

For the SymbolTable issue, I also don't like the solution of throwing an exception when reading a value but I haven't found any alternative way to understand whether a variable is used or not in the called program.
Surely there is a cleaner solution but I haven't found it.

[lanarimarco]

You are exploiting VoidValue, which was born to handle rpg functions that returns nothing to handle this issue, this choice forced you to introduce many changes. In addition you are adding in SymbolTable something that it should not be in SymbolTable.

At first I created a new NullValue class to manage variables without value assignment. Then I preferred to use the VoidValue class because it was already present and it seemed not used in any part of the code (its methods were mostly marked as TODO). If you prefer, I can restore the NullValue class and leave the VoidValue class unchanged.

For the SymbolTable issue, I also don't like the solution of throwing an exception when reading a value but I haven't found any alternative way to understand whether a variable is used or not in the called program. Surely there is a cleaner solution but I haven't found it.

I'll think about it and let you know by tomorrow.

[lanarimarco]

Now your implementation is clearer, because I realized that the only way to manage this behavior is at runtime.
But maybe you are using VoidValue not properly.
You have implemented some methods in a logically wrong way, as mentioned in my comments.
Try to follow my suggestions, if you are not able to do it, it could be necessary define a new value, for instance NullValue.

[lanarimarco]

But VoidValue is not a NullValue. Il giorno gio 25 gen 2024 alle ore 11:37 Dario Foresti < ***@***.***> ha scritto:

…

***@***.**** commented on this pull request. ------------------------------ In rpgJavaInterpreter-core/src/main/kotlin/com/smeup/rpgparser/interpreter/values.kt <#405 (comment)>: > } override fun assignableTo(expectedType: Type): Boolean { - TODO("Not yet implemented") Depends, if we consider VoidValue as a null object we can assign it to every variable, it's a method to set to null a typed variable. Perhaps the right solution is to return to NullValue and leave VoidValue unchanged because VoidValue is an empty value, not a null one. — Reply to this email directly, view it on GitHub <#405 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AJR622R6XM6HYVX4YZFTTUDYQIYW7AVCNFSM6AAAAABCB54K66VHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMYTQNBTGM3TGMZWGM> . You are receiving this because your review was requested.Message ID: ***@***.***>

-- [image: smeup] <https://www.smeup.com> Marco Lanari R&D Department - Developer Office: 0521940611 <00390521940611> www.smeup.com *SMEUP LAB SRL* Via Carra, 8 - 43122 Parma (PR)

[lanarimarco]

I don't understand why the NullValue check is not in SymbolTable.get(DataDefinition).
Since that NullValue is something like a red semaphore that denies the variable access, you should implement this in SymbolTable.get(DataDefinition).

[lanarimarco]

I have tried to reimplement as I thought was better (SymbolTable.get) but it would have been a disaster, than, I have to admit that your idea is as best as we can do.

[lanarimarco]

Ok

[foresti-smeup]

I also try this solution with modified SymbolTable.get:

override operator fun get(data: AbstractDataDefinition): Value {
        val value =  when (data.scope) {
            Scope.Program -> (programSymbolTable as SymbolTable).getLocal(data)
            Scope.Local -> getLocal(data)
            Scope.Static -> TODO()
        }
        if (value is NullValue) {
            throw IllegalArgumentException("Void value for ${data.name}")
        }
        return value
    }

but also in my test the result was a half disaster.

[lanarimarco]

Not half but complete... :-) Il giorno ven 26 gen 2024 alle ore 17:57 Dario Foresti < ***@***.***> ha scritto:

…

I also try this solution with modified SymbolTable.get: override operator fun get(data: AbstractDataDefinition): Value { val value = when (data.scope) { Scope.Program -> (programSymbolTable as SymbolTable).getLocal(data) Scope.Local -> getLocal(data) Scope.Static -> TODO() } if (value is NullValue) { throw IllegalArgumentException("Void value for ${data.name}") } return value } but also in my test the result was a half disaster. — Reply to this email directly, view it on GitHub <#405 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AJR622VPLB4BX34GZB5UNPLYQPN6NAVCNFSM6AAAAABCB54K66VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSMJSGM4DAMRZGY> . You are receiving this because you modified the open/close state.Message ID: ***@***.***>

-- [image: smeup] <https://www.smeup.com> Marco Lanari R&D Department - Developer Office: 0521940611 <00390521940611> www.smeup.com *SMEUP LAB SRL* Via Carra, 8 - 43122 Parma (PR)