feat: support specifying generics on a struct when calling an associated function

[asterite]

Description

Problem

Resolves #5584
Resolves #6000

Summary

Also errors if you have something like this:

this_is_a_module::<i32>::foo();
                  ^^^^^
            turbofish not allowed on module `this_is_a_module`

Additional Context

Note: this only works for structs. I captured a couple of issues to make this work for traits and aliases, but I'd prefer to do those in separate PRs.

Documentation

Check one:

• No documentation needed.
• Documentation included in this PR.
• [For Experimental Features] Documentation to be submitted in a separate PR.

PR Checklist

• I have tested the changes locally.
• I have formatted the changes with Prettier and/or cargo fmt on default settings.

[jfecher]

Going to block this temporarily to give me some time to think about the approach some more.

[jfecher]

The generic_type_in_path part seems odd to me - it doesn't really say where that type is and I think it may be possible to have multiple types in the path. E.g. foo::bar::Baz::<i32>::AssociatedType::<u32>::method::<i8>()

Maybe it'd be possible to resolve to a module + type bindings? E.g. resolve to the struct module of Baz then handle any paths from there separately? Currently this would only be to methods I think since I don't think we support referring to associated types using the syntax I gave above.

[asterite]

The generic_type_in_path part seems odd to me

Makes sense. I thought of different possibilities to do this and that one was the less disruptive to the existing code.

That said... there's something that I always felt wasn't quite right: path resolution/lookup. There are two things here:

• imports (use statements)
• paths in code

Imports can't have generics in their path, while paths do. But: the code to resolve them is the same one.

So we have two choices:

1. Have two different code paths to solve imports and paths
2. Have a single code path, but returning a better type as the output

I'm leaning towards option 2, mainly to avoid code duplication. When resolving an import we can assert/panic when we get a resolution that shouldn't happen.

The better type I'm thinking is, instead of returning ModuleDefId we could return some other enum like this:

enum PathResolutionKind {
    Module(ModuleId),
    Struct(StructId, Option<Vec<UnresolvedType>>), // The generics, if any (using Option to distinguish the `::<>` case)
    TypeAlias(TypeAliasId, Option<Vec<UnresolvedType>>),
    Trait(TraitId, Option<Vec<UnresolvedType>>),
    ModuleFunction(FuncId),
    StructFunction(StructId, Option<Vec<UnresolvedType>>, FuncId),
    TypeAliasFunction(TypeAliasId, Option<Vec<UnresolvedType>>, FuncId),
    TraitFunction(TraitId, Option<Vec<UnresolvedType>>, FuncId),
}

That way it's a bit more clear what a path resolved too... and if eventually we support that associated type line, we'll add a new member to the above enum.

What do you think?

[jfecher]

That way it's a bit more clear what a path resolved too... and if eventually we support that associated type line, we'll add a new member to the above enum.
What do you think?

I like it - I agree path resolution leaves things to be desired currently. I was trying to avoid suggesting "let's rewrite path resolution entirely" but maybe it is needed 😅. I'm fine with the two different code paths for imports and paths in code in general as well though. I think there are enough differences there that using one for both could (unverified) end up being more complicated in the end. E.g. Passing booleans and such to determine which cases you need to support and which you should error for. That's why I was initially thinking of the approach to break a path up into two parts: the prefix path which can only contain modules, functions, structs without turbofish and other things that can be imported, and the suffix path which may contain structs with turbofish, methods on them, and possibly associated types on those structs. Your call on which is cleaner, it can be a bit hard to guess without trying it first.

[asterite]

I went down the route of having path and import resolution use the same code but return a better type.

It turns out, resolving a path gave two things:

• a ModuleDefId
• a namespace (PerNs)

The ModuleDefId part was used when looking up paths, the namespace part for imports. So, I changed that ModuleDefId to be PathResolutionItem in this case, with the enum I mentioned above. Then GenericTypeInPath is gone.

I think the code is much cleaner now. I still don't know if we should split the path/import lookup into two... maybe I can work on getting this working for traits and aliases, and then we can see if another refactor is worth it.

Let me know! 🙏 (this is ready for review again)

[jfecher]

👍

[asterite]

Sorry I had to force push, yesterday I continued working on this branch and forgot about it, then merged and got unrelated changes.