Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow use statements to happen before the items they reference #2940

Draft
wants to merge 11 commits into
base: master
Choose a base branch
from

Conversation

CohenArthur
Copy link
Member

@CohenArthur CohenArthur commented Apr 4, 2024

  • nr2.0: default-visitor: Conditionally visit type in self parameters.
  • [fixup wip] notes: Add them for how to resolve this
  • toplevel: Build list of imports for Early to resolve
  • early: Resolve imports and create import mappings
  • imports: Add FinalizeImports class

This is not ready to review. There is currently a regression compared to master which I'm investigating:

pub mod module {
    pub fn f() {}

    pub fn returns_i32() -> i32 {
        15
    }

    pub fn bar() -> bool {
        true
    }
}

use module::bar;
use module::f;
use module::returns_i32;

fn main() {
    let foo = f();
    let bar = f();

    let a: bool = f();
    let b: bool = returns_i32();

    let c = module::bar();
}

The above code should compile, and does when using -frust-name-resolution-2.0 on master, but not here. This is probably due to FinalizeImports not being implemented properly.

This also needs testing and cleaning up

@CohenArthur CohenArthur changed the title Allow use statements to happen before the items they references Allow use statements to happen before the items they reference Apr 4, 2024
@CohenArthur CohenArthur force-pushed the allow-use-before-items-being-used branch from 0dea923 to ced3ded Compare April 5, 2024 09:28
This could trigger an assertions as `get_type` on `SelfParam` asserts that
the self param does have a given type, which is not always the case.

gcc/rust/ChangeLog:

	* resolve/rust-default-resolver.cc (DefaultResolver::visit): Do not
	visit self's type if it does not have one.
@CohenArthur
Copy link
Member Author

we're now storing the namespaces and inserting definitions properly - the issue is inserting them in the right Rib. toplevel.insert_or_error_out(...) inserts them in the topmost Rib, which is not correct. we can store the Rib in which to insert data in ImportKind - maybe via a reference/pointer? we need to ensure these pointers will stay live for the duration of FinalizeImports, which I think is a fair assumption

@CohenArthur
Copy link
Member Author

CohenArthur commented Apr 5, 2024

but we can't know which namespace to get the Rib from before Early so we're hitting the same problem :( maybe FinalizeImports should be a fully fledged visitor as well? urgh

we can also store 4 pointers to all 4 ribs we're currently visiting when creating the import, and then choose based on what Early resolved to - is that stupid?

@CohenArthur
Copy link
Member Author

Storing all ribs gets messy and unwieldy. I guess having a map<UseTree, ImportKind> would make more sense, and we could just visit all UseTrees in FinalizeImports and get the import data from this

@CohenArthur
Copy link
Member Author

last step is to declare imports in each namespace where they resolved properly. this is highlighted by name_resolution16.rs in the testsuite.

the current implementation bails as soon as it finds one namespace an import was resolved in, but we actually need to build a list of namespaces + node Ids for each import

@CohenArthur
Copy link
Member Author

Last hurdle comes from imports depending on each other, like so:

mod foo {
    pub struct Foo;
}

use foo::Foo;
use Foo as Fo;

fn main() {}

this can probably be fixed by making it more "fixed-pointy"

@P-E-P P-E-P self-requested a review April 23, 2024 14:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

None yet

1 participant