Carpet

Carpet

2M Downloads

defined functions in apps are not visible in libraries they import.

gnembon opened this issue ยท 10 comments

commented

its not a bug by itself, but lambda functions do work and that's inconsistent and not mentioned in the docs.
consider an example
lib.scl

callme(fun, arg) -> call(fun, arg);

app.sc

import('lib', 'callme');

foo(x) -> x*x;

test() -> callme(_(x) -> call('foo', x) , 5); // works
test() -> callme('foo', 5); // doesn't
commented

ok, so right now functions are searched in their module and imports of that module, so 'callme' is visible in module 'app' cause it imported it, but 'foo' isn not visible in 'app' since its not imported.

Because function discovery fails the program when a function is not found, it won't be harmful to check again for functions in main, but is that something that would be intended?

consider the following example:
lib.scl

foo(x) -> x+1;
callme(fun, arg) -> call(fun, arg);

app.sc

import('lib', 'callme');
foo(x) -> x*x;
test() -> callme('foo' , 5);

What should be the output when I call /script in app invoke test? 6 or 25?

commented

I think 25, cos its gonna execute callme function in context of app.sc, so considering pre-existing funcs and vars of app.sc

commented

hmm, but scope of 'lib' should always be preferred because in that case foo in lib would never be accessible, cause it will be masked.

commented

I agree. I actualy think it would be counterintuitive if callme could see funtions in its module that were never imported. Idk, I interpret import as "that function that was over there is now defined here too".

commented

its important to note that when callme is called - it only accepts a plain string that doesn't have any semanting meaning, and that semantics is resolved when its used. Currently lib does only see methods defined in its own module and its imports (lib),but I am considering adding fallback to the main module and their imports

commented

yeah - so lib should search for a function in its own module first and dependencies, and then in the main app. This way authors of lib will never be affected with and app that uses it, while allowing to pass methods per name from the app that uses them. Alternative option is to leave it as it, mentioning in the docs that when passing of functions to libraries, you could only do that using function values. In that case I would prefer to enforce that throwing an error if passing a function value to an imported call, but that would mean doing extra parameters check for every imported call which would lower the performance of imported methods. (cause of extra argument checks)

commented

I just checked in 1.4.20 and even if you imported foo also, you would still get 25, which makes sense to me. It seems reasonable to me that you need to import any method you need along with anything they call, and if the main program overwrites an imported function or another function it uses, then the imported functions work differently. If it's possible to have a debug message saying that a dependency was not imported and should be or that a function in the main program overwrites an imported one or its dependencies, which could lead to problems, that would be handy.

commented

These are executed in order, so import happens first, then a definition, but that makes sense to warn when defining a function that is already defined / is imported.

In general there is no difference between foo(x) and call('foo', x) , all functions that are not buil-in are just translated to call('name', ... args). So if I add a fallback for imports from places other than main it is possible that a library itself would have a code that could fails if something is not defined, but main, or some other libraries it imports may fill these missing functions which.... might be confusing.

Definitely first place to get the function from have to be that module and its imports and then maybe main. Using function values in this case never fails, because function is defined / resolved right where the first call is.

commented

ur contradicting urself, unless im mistaken....
image

commented

As @BisUmTo stated on discord

I would prefer leave it as it... It makes sense to me

In that case I would only suggest changing the wording if calling an unknown method from a library / imported module meaning that the ScriptHost::getAssertFunction would look like

public FunctionValue getAssertFunction(Module module, String name)
    {
        FunctionValue ret = getFunction(module, name);
        if (ret == null)
        {
            if (module == main)
                throw new InternalExpressionException("Function '"+name+"' is not defined yet");
            else
                throw new InternalExpressionException("Function '"+name+"' is not defined nor visible by its name in the imported module '"+module.getName()+"'");
        }
        return ret;
    }

The other scenario where all calls in the app can be potentially resolved within the app would be by:

 public FunctionValue getAssertFunction(Module module, String name)
    {
        FunctionValue ret = getFunction(module, name);
        if (ret == null) ret = getFunction(main, name);
        if (ret == null) throw new InternalExpressionException("Function "+name+" is not defined yet");
        return ret;
    }

note that technically if you modularize your app and don't plan the library to be universal, you can always import back and forth (scarpet is ok with circular imports) making so that all functions will be visible in any module scope. That can be resolved by explaining in the docs how to do it.

I get that the library (proper library, not part of your project) should not depend / use functions defined in the apps that use them, That would be super sketchy to use and tricky to explain.