openscad / openscad

OpenSCAD - The Programmers Solid 3D CAD Modeller
https://www.openscad.org
Other
7.05k stars 1.21k forks source link

Review use of undeclared parameters in module/function calls #539

Open MichaelAtOz opened 10 years ago

MichaelAtOz commented 10 years ago

Marius wanted an issue to look into undeclared parameters.

From the perspective of other formal languages this could be viewed as a side-effect bug.

But while 'use' has the different scope handling it is necessary. Write.scad for example, has a bunch of ('use <>' file-scoped) global default variables, and the only way to override them is passing them as unnamed parameters. (or update it to have all the variables as parameters)

Otherwise you would have to 'include <>' them, pollute the global scope, so you can then re-assign defaults in your main program. Which is fine for small scale development, and easier as you just need to change the default once, rather than including your unique value in every module call (if you wanted to change many defaults you would presumably create a wrapper module).

But, if the other namespace initiative is fruitful you could

include <some.scad> as x
x.default_var=whatever;
x.a_module();

What all this is really about is 'interfaces' where other languages go into classes, methods, public/static variables, import etc to allow separately developed code to interact. It also goes to scoping which has been oft discussed here.

I think that should be sufficient to trigger thought & discussion...

--- Want to back this issue? **[Post a bounty on it!](https://app.bountysource.com/issues/1070689-review-use-of-undeclared-parameters-in-module-function-calls?utm_campaign=plugin&utm_content=tracker%2F52063&utm_medium=issues&utm_source=github)** We accept bounties via [Bountysource](https://app.bountysource.com/?utm_campaign=plugin&utm_content=tracker%2F52063&utm_medium=issues&utm_source=github).
GilesBathgate commented 10 years ago

Don't forget that currently include includes the file as text. So as it stands I think what you suggested would not easily be able to create namespaces for the variables from the included script. I envisaged that import (as implemented for rapcad stl import) could be overloaded to allow script imports.

nophead commented 10 years ago
a = 1;

module x() echo(a);

module y() { x(); echo (a); }

y(a = 2);

Gives

ECHO: 1
ECHO: 2

So although it might seem useful I think it breaks too many rules of programming.

kintel commented 10 years ago

@nophead The problem is that it has been like this from the start, so changing it is likely to break a lot of stuff.

At some point we'll probably have to bit the bullet and make a breaking change, but I've been wanting to do that through a real language cleanup. Not sure if that will ever happen though, as time is scarce..

nophead commented 10 years ago

Is it documented somewhere? I can't imagine anybody using it otherwise.

On 7 November 2013 21:52, Marius Kintel notifications@github.com wrote:

@nophead https://github.com/nophead The problem is that it has been like this from the start, so changing it is likely to break a lot of stuff.

At some point we'll probably have to bit the bullet and make a breaking change, but I've been wanting to do that through a real language cleanup. Not sure if that will ever happen though, as time is scarce..

— Reply to this email directly or view it on GitHubhttps://github.com/openscad/openscad/issues/539#issuecomment-28010178 .

kintel commented 10 years ago

I don't think it's documented, but OpenSCAD power users tend to be very explorative in nature ;)

Perhaps I should start collecting statistics on marginal feature usage..

MichaelAtOz commented 10 years ago

Perhaps I should start collecting statistics on marginal feature usage..

Off topic, but it triggered by: Happened to notice Thing 7373 uses a library courtesy of Adrien Bower full of your .x .y .z feature, so it is in the wild.

You could consider deprecating your dot product, add three constants, so people could use vector maths, ie;

X=[1,0,0];
Y=[0,1,0]; // uppercase to lessen conflict with preexisting variables
z=[0,0,1]; // or maybe lowercase saves hitting extra shift keystroke??
v=[88,89,90];
echo(v*X,v*Y,v*z);

Such features can help standardise programming techniques and make things clearer than v[0].

Presuming use specified variables override constants it should not break existing programs.

MichaelAtOz commented 10 years ago

@nophead but:

a = 1;
module x() echo("x:",a);
module y(a) { x(); echo ("y:",a); }  // NOTE parameter 'a'
y(a = 2);

Also produces

ECHO: "x:", 1
ECHO: "y:", 2

So it is not the undeclared nature of the parameter. The same scope issue is at hand.

These scope discussions keep reoccurring, [probably because when I see something funny I raise it].

When I think/research such stuff I keep going to wikipedia to refresh my n-decade old Computer Science formal descriptive language. Perhaps it is time to try to define WHAT OpenSCAD is/is-meant-to-be/is-desired-to-be?

edit/ same with module y(a) { a=3; x(); echo ("y:",a); } x:1 /edit

edit/ module y(a) { a=3; x(a=a); echo ("y:",a); } x:3 /edit

nophead commented 10 years ago

That behaviour is to be expected though as most languages allow a parameter to override a global variable. You can look at the definition of y() and see that. Openscad is odd in that you can inject new variables into the scope of a function which sort of breaks encapsulation. You can't treat the function as a black box that takes some parameters and returns a result based on those so it isn't like any languages I have used before.

It does seem useful though, so perhaps it is OK as long as it documented somewhere.

On 11 November 2013 22:54, Michael notifications@github.com wrote:

@nophead https://github.com/nophead but:

a = 1; module x() echo("x:",a); module y(a) { x(); echo ("y:",a); } // NOTE parameter 'a' y(a = 2);

Also produces

ECHO: "x:", 1 ECHO: "y:", 2

So it is not the undeclared nature of the parameter. The same scope issue is at hand.

These scope discussions keep reoccurring, [probably because when I see something funny I raise it].

When I think/research such stuff I keep going to wikipedia to refresh my n-decade old Computer Science formal descriptive language. Perhaps it is time to try to define WHAT OpenSCAD is/is-meant-to-be/is-desired-to-be?

— Reply to this email directly or view it on GitHubhttps://github.com/openscad/openscad/issues/539#issuecomment-28249360 .

szabi commented 10 years ago

"Openscad is odd in that you can inject new variables into the scope of a function which sort of breaks encapsulation."

Yes, I had a case just yesterday where in a module call I mistyped a parameter name, and instead of raising an error, the program run (injecting the -- then unused -- mistyped parameter, but having the original parameter that I meant undef) -- yielding a wrong result.

Maybe it would be useful, if one wants to keep the feature, to introduce a new keyword. Let's say

module x(a, b, c) {...}

x(1,2,3); // OK
x(a=1, b=2, c=3); //OK
x(a=1, b=2, c=3, let d=10); //OK
x(a=1, b=2, c=3, d=10); // ==> Error

module y(a, b, c=undef) {...}
x(a=1, b=2); // ==> Error (parameter without default value not passed).
y(a=1, b=2); // OK

"At some point we'll probably have to bit the bullet and make a breaking change, but I've been wanting to do that through a real language cleanup. Not sure if that will ever happen though, as time is scarce.."

You don't have to break existing code. If as of the second revision of the language you introduce a pragma which is interpreted as comment by the current language version, e.g. //// SCAD_LANG v2 or /* LANG_VERSION: 2 */ as the first line (or whatever), this could trigger the new interpretation of the language, while for files missing that pragma, the old compiler would be used. But "a real language cleanup", which starts with thoughts about what's desired, then a syntax documentation, and finally implementation seems to be a good idea.

MichaelAtOz commented 10 years ago

If it is a real break, then you could change the file extension.

.oscad .scad2

MichaelPFrey commented 5 years ago

see also #2638

Note that my approach is to not change how openscad behaves, but to just issue a warning.