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

Resolution Design #27

Closed
NalaGinrut opened this issue May 18, 2020 · 7 comments · Fixed by #33 or #64
Closed

Resolution Design #27

NalaGinrut opened this issue May 18, 2020 · 7 comments · Fixed by #33 or #64

Comments

@NalaGinrut
Copy link
Contributor

I think it's better to separate type and name resolution, fortunately, if we do it now, it's just a little change. Here're the steps:

  1. Create Resolution class inherited from AST::ASTVisitor, and move static bool ResolveNamesAndTypes into it.
  2. Separate TypeResolution and NameResolution class inherited from Resolution class.
  3. Put them in different files.

This is better for maintenance and working parallelly.
And the name resolution will distinct names from different namespaces, like values, macros, so it's an important prerequisite for the rest of the work.

If it's OK, then I can raise PR for it.

What do you think? @philberty @SimplyTheOther

@NalaGinrut
Copy link
Contributor Author

NalaGinrut commented May 18, 2020

BTW, to perform type resolution correctly, I think the order should be:

name resolution -> macro expansion -> type resolution

But it's fine if we implement each step parallelly.

@philberty
Copy link
Member

Yes this is a really good idea.

The way i envision things is many "Passes" and a pass i mean a Class using the ASTVisitor pattern and we have a state machine to work the work in there. We can even flesh out later with warning passes such as DeadCode recounting the names as it goes and when things go out of scope we iterate all nodes with a refcount == 0 as dead code.

I recently Added a simple pass rust-scan.cc which we will need in subsequent passes to know about global declarations as functions can be declared after they are used etc.

@bjorn3
Copy link

bjorn3 commented May 18, 2020

Name resolution and macro expansion need to be interleaved for the following to work:

macro_rules! a {
    () => {
        macro_rules! b {
            () => {}
        }
    }
}

a!();
b!();
#![feature(decl_macro)]

macro a($b:ident) {
    macro $b() {}
}

b!();
a!(b);

@NalaGinrut
Copy link
Contributor Author

@bjorn3 Ah yes, thanks for mention, I'll keep it.

@NalaGinrut
Copy link
Contributor Author

NalaGinrut commented May 18, 2020

@philberty Yes, the pass, I think this term implies everything, extensible and can be disabled for certain purposes. We need a tiny framework for these passes (and options for disabling and debug), maybe we can do it after resolution an expansion.

@philberty philberty linked a pull request May 21, 2020 that will close this issue
@philberty philberty reopened this May 22, 2020
philberty added a commit that referenced this issue Dec 1, 2020
This adds type inferencing and validation for arrays such as:

  let x: [i32, 5] = [1,2,3,4,5]
  ley y = [1,2,3];

It checks that each element is of a valid type and they line up correctly.
There needs to be some refactoring of the type resolution to handle the
array index expressions so this is a good save point for now.

Addresses: #27 #55
philberty added a commit that referenced this issue Dec 1, 2020
Adds new TypeVisitor to resolve Types from the AST via Visitor Pattern, so
the element type can be extracted.

Addresses: #27 #55
philberty added a commit that referenced this issue Dec 1, 2020
The index expression must be validated to ensure it is an integer.

Addresses: #27 #55
philberty added a commit that referenced this issue Dec 1, 2020
This adds type inferencing and validation for arrays such as:

  let x: [i32, 5] = [1,2,3,4,5]
  ley y = [1,2,3];

It checks that each element is of a valid type and they line up correctly.
There needs to be some refactoring of the type resolution to handle the
array index expressions so this is a good save point for now.

Addresses: #27 #55
philberty added a commit that referenced this issue Dec 1, 2020
Adds new TypeVisitor to resolve Types from the AST via Visitor Pattern, so
the element type can be extracted.

Addresses: #27 #55
philberty added a commit that referenced this issue Dec 1, 2020
The index expression must be validated to ensure it is an integer.

Addresses: #27 #55
@philberty
Copy link
Member

I have now pushed a big pr for the new resolution frameworks. For macros we need a toplevel name resolution pass and then we can do macro expansion. I will make some documentation on how the IDs are working to help.

@philberty philberty added this to the Core Datastructures milestone Dec 18, 2020
@philberty philberty changed the title Resolution Resolution Design Dec 18, 2020
@philberty
Copy link
Member

I will add documentation to the WIKI on how the resolution is working to be able to close this issue.

@philberty philberty moved this from Done to To do in Data Structures 1 - Core Dec 22, 2020
@philberty philberty moved this from To do to In progress in Data Structures 1 - Core Dec 22, 2020
@philberty philberty removed this from the Core Datastructures milestone Jan 6, 2021
@philberty philberty removed this from In progress in Data Structures 1 - Core Jan 6, 2021
@philberty philberty linked a pull request Jan 6, 2021 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants