`$variable` assignment no longer seems to work. about openscad HOT 13 OPEN

Probably the same as #3781 / #3881.

from openscad.

I don't have a strong opinion, and I'm happy to change to a different way of doing it if it's proportionally similar in ease-of-use. However, I imagine this regression will break a lot of existing code too. So compatibility might be an important consideration.

from openscad.

This case is not a bug, but is actually a straightforward textbook case where you ended up fixing your code to be correct. You were using $radial_error, a dynamic scope variable (meaning it gets passed in by the caller), in a lexical scope (meaning it is syntactically outside of where they are defined) of the functions and modules that needed it. The lexical scope variable (the ones without $) that you switched this to is the correct approach for the lexical scope pick-up that you are trying to do. The behavior you were previously using worked due to a bug that mangled these scope definitions in one limited case (the tops of files being use<>d) but not others, but this was fixed during a code refactor clean-up a few years back.

from openscad.

I understand dynamic scope variables, so I guess my question is, are default arguments evaluated according to lexical scope, or at the time of the method call? The latter would seem to be the behaviour I was depending on previously.

With my recent change, I suppose I can no longer change this value globally from a different file. Is that correct? In other words, I think I lost some functionality.

from openscad.

Default arguments are evaluated at the time of the call. What was happening in the previous bug, was that the top scope of the use<> file was being dynamically re-evaluated at each call of each function and each module, as if the execution jumps to the top of the use<>d file, runs through that, and then down to whatever function or module you were calling, each time.

Because dynamic scope is being used properly now, actually you gained the ability to pass in $radial_error, whereas your previous code in the old version of the program would have ignored any value you tried to define globally outside, overwriting it with your lexical scope definition of a dynamic scope variable, perhaps contrary to your intuition. (Maybe you only intended for this to be possible before, but didn't try it, but it appears to me that structure of code you had before fully blocks external control of the values under the old bug.)

To get dynamic behavior with a default set in a single location, try using is_undef like this:

function radial_error() = is_undef($radial_error) ? 0.1 : $radial_error;

Or you can write similar is_undef routines in the parameter lists, inside of let in functions, or inside of modules if you want.

from openscad.

That makes sense.

The only thing I'd suggest here, is that if we can detect the previous usage, we should issue a warning and ideally link to some kind of documentation which explains how to fix it.

from openscad.

Detection is likely impossible without false positive triggers, because people often put other code (e.g., test code) in use<>d files for usage when the file is loaded directly, but that is intended to be ignored when it is use<>d, as this is the specification of how use<> is supposed to work. From the manual: "use <filename> imports modules and functions, but does not execute any commands other than those definitions.". So that makes it not specifically an error to have these other elements in the file for a different usage purpose.

from openscad.

Not sure it can be called a bug when the official OpenSCAD examples use it. For example, the second one uses $ variables at file scope.

from openscad.

What was happening in the previous bug, was that the top scope of the use<> file was being dynamically re-evaluated at each call of each function and each module, as if the execution jumps to the top of the use<>d file, runs through that, and then down to whatever function or module you were calling, each time.

That is still the case as you can define a normal variable at file scope and use it in a function or module. And $variables are still evaluated at file scope and can be used by file scope expressions. The change is they can't be seen directly in functions and modules, but only when the file is used. They work if it is the top level file. And they can be used indirectly in functions and modules if you copy them to a normal variable at file scope.

from openscad.

@rcolyer wrote:

What was happening in the previous bug, was that the top scope of the use<> file was being dynamically re-evaluated at each call of each function and each module, as if the execution jumps to the top of the use<>d file, runs through that, and then down to whatever function or module you were calling, each time.

But... it still does that now. So we have the worst of all worlds: the overhead (and arguably incorrectness) of evaluating those assignments, and incompatibility with previous behavior.

from openscad.

Yes proper fix would be to put the files cope search back but only after the stack search and then fix the performance problem by tagging expressions that don't need re-evaluation because they are true constants, i.e. don't use $variables or depend on other expressions that use $variables or call functions that use $variables. Probably 99.9% of my file scope expressions are constant but the odd one isn't.

from openscad.

Analyzing expressions to determine whether they are constant is not at all trivial. Constant-ness would need to percolate up through arbitrary levels of expressions and function calls.

from openscad.

Not trivial but not overly complex either. Even if regards all function calls as none constant it would save millions of re-evaluations in my code and probably speed it up from 4 minutes to a few seconds.

from openscad.