New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Do lazy evaluation in list/map index defaults #20
Comments
I have added a workaround in yast/ycp-killer#461, it should be removed after fixing this issue. |
@jreidinger Could you make With that implemented, I could translate this code:
as: @str = Ops.index(@m, ["a"]) { Foo() } This would ensure lazy evaluation of the default value. Translation of default values that are not functions would stay the same (they would be passed as the 3rd argument). |
yes, WIP, not sure if I finish it today |
Implemented - yast/yast-ruby-bindings@0dfcb4d |
One note to the fix: It only fixes the case when the default value is a call. All other cases are still evaluated eagerly. This means that e.g. complex default values including calls (e.g. a list or map) may still behave incorrectly. I don't look if such values are even allowed in YCP, and how they behave if so. Moreover, I didn't see any such default value in the code so far. As a result, I think this case is most likely irrelevant. |
Sorry, that argument is wrong. Yes, the majority of cases are simple values, but fixing only one specific case of evaluable values can result in subtle, well hidden bugs. It should be safe and equally simple to replace "use block if default is a call" with "use block if default is not a literal". To clarify, |
This change avoids subtle bugs when a there is a function hidden somewhere in the default value. Note that we still evaluate empty lists, maps, and terms non-lazily as there could be no function hidden inside them. This extends previous fix of #20.
@mvidner I added lazy evaluation for non-empty lists, maps and terms. It seems a reasonable compromise between generating too many blocks and too complex code. We can improve this further later if we'll have time. BTW, this change added 145 blocks to the translated YaST code, most of them containing UI terms (e.g. |
Correction: There were 145 |
The defaults for accessing YCPList and YCPMap values need to evaluated only when the requested index/key is missing.
If a function with side effect is used for returning the default then the Ruby code behaves differently to the original YCP.
YCP Example:
This logs:
The converted ruby code contains this
which causes the
Foo()
function evaluation before accessing the map and thus logs:This breaks testsuite in
yast2
inPortAliases
module, see lines https://github.com/yast/yast-yast2/blob/master/library/network/src/PortAliases.ycp#L216 and https://github.com/yast/yast-yast2/blob/master/library/network/src/PortAliases.ycp#L167 where the default handler function modifies the accessed map (yeah!).The text was updated successfully, but these errors were encountered: