Aliasing a package import doesn't apply to imported default method implementations

[ergl]

This issue was raised on Zulip by @rhagenson.

If one imports a package using an alias (use alias = "./path/to/package), this alias is not applied to all symbols imported from the package, as the following code shows:

// This code lives under lib/lib.pony

primitive LibPrimitive

trait LibTrait
  fun foo(): LibPrimitive => LibPrimitive

use lib = "./lib"

actor Main is lib.LibTrait
  new create(env: Env) => None

This issue also applies if we use interfaces, so it's not unique to using a trait.

The error:

$ ponyc .
Building builtin -> /Users/ryan/.local/share/ponyup/ponyc-release-0.39.1-x86_64-darwin/packages/builtin
Building . -> /Users/ryan/Downloads/test
Building ./lib -> /Users/ryan/Downloads/test/lib
Error:
/Users/ryan/Downloads/test/lib/lib.pony:5:5: can't find declaration of 'LibPrimitive'
    LibPrimitive
    ^

If one implements the foo() method on Main with the applied alias, the code will work correctly. If one looks at the AST, we can verify that the implementation of foo() is copied into the scope of Main without any renaming (diff shows the AST change that should occur for the code to work correctly):

 (fun:scope
   box
   (id foo)
   x
   x
-  (nominal (id $2) (id LibPrimitive) x val x x)
+  (nominal (id $2) (id LibPrimitive) x val x (id lib))
   x
- (seq (reference (id LibPrimitive)))
+ (seq (. (reference (id lib)) (id LibPrimitive)))
   x
 )

[mfelsche]

Thanks for that nice writeup.
I am collecting some related existing bug reports in the realm of default implementations of traits that give us a picture of the full defects of the current implementation. Might come in handy when starting to work on that:

• reference to "abstract" method from implemented method in trait fails to verify #2365
• Default trait methods do not have access to imports from their declaring file. #2150

[SeanTAllen]

So the issue here seems to be that as stands, we don't consider this case. In scope.c we have the following in use_package:

  if(name != NULL && ast_id(name) == TK_ID) // We have an alias
    return set_scope(options, ast, name, package, false);

  ast_setflag(ast, AST_FLAG_IMPORT);

Note, that AST_FLAG_IMPORT isn't set if we have an alias.

Then in import.c, in import_use:

static bool import_use(pass_opt_t* opt, ast_t* ast)
{
  pony_assert(ast != NULL);

  if(!ast_checkflag(ast, AST_FLAG_IMPORT))
    return true;

which means we bail out before any imports because it is an alias.

I think we need more sophisticated handling. I'm thinking something like setting a flag for a regular import vs an aliased import. For a regular import, all works as current. For an aliased import, we need to look for anything that is a TK_NOMINAL and if its referenced through an alias, we import into the object in question.

@ponylang/committer thoughts?

[SeanTAllen]

@ergl what do you mean by:

"If one implements the foo() method on Main with the applied alias, the code will work correctly. "

I'm not sure what that code would look like on Main.

[SeanTAllen]

traits.c

add_method

doesnt take into account aliasing from the use when doing:

  ast_t* local = ast_append(ast_childidx(entity, 4), basis_method);
  ast_set(entity, name, local, SYM_DEFINED, false);
  ast_t* body_donor = (ast_t*)ast_data(basis_method);
  method_t* info = attach_method_t(local);
  info->trait_ref = trait_ref;

[SeanTAllen]

I think this is best handled in:

static ast_t* reify_provides_type(ast_t* method, ast_t* trait_ref,
  pass_opt_t* opt)

which currently assumes no aliasing and is creating the basis method that ends up getting copied into the provider.

[SeanTAllen]

Update on that, maybe doing in reify_provides_type is wrong and we want to do in a new method after reification and before adding the method. Not sure.

[ergl]

@SeanTAllen I meant implementing the function in Main, instead of using the default implementation of foo:

use lib = "./lib"

actor Main is lib.LibTrait
  new create(env: Env) => None
  fun foo(): lib.LibPrimitive => lib.LibPrimitive

I mentioned that so that I could diff the AST of both versions, which I highlighted in the original comment.

[SeanTAllen]

@ergl Ok, I thought you meant that somehow the default method worked. Which isn't the case. Gotcha.

[SeanTAllen]

I'm working on this with some assistance from @jemc.

[SeanTAllen]

I have a fix for this. I need to clean it up as its two bunches of identical code copied into two locations, I'll have a PR open sometime this week.