Skip to content

Separate type class registration pass in experimental analysis #14558

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

Merged
merged 3 commits into from
Sep 18, 2023

Conversation

cameel
Copy link
Member

@cameel cameel commented Sep 11, 2023

Base PR: #14510.

@cameel cameel requested a review from ekpyron September 11, 2023 12:45
@cameel cameel self-assigned this Sep 11, 2023
Comment on lines -139 to 143
private:
friend class TypeEnvironment;

TypeClassInfo const& typeClassInfo(TypeClass _class) const
{
return m_typeClasses.at(_class.m_index);
}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure why this was private in the first place. I need to access it to get at the type variable after the class is already registered. Is there some other way I am supposed to get it?

Comment on lines +42 to +44
struct GlobalAnnotation
{
};
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps I should move more things from TypeInference here? builtinClasses and builtinClassesByName looks like good candidates.

I tried that already with typeClassFunctions but the problem is that I don't have the function types to put in it - those are only available after TypeInference processes function definitions. I could just gather FunctionDefinitions but TypeInference would have to iterate over them again and it did not seem like it would save us much code.

Copy link
Member

@ekpyron ekpyron left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me for merging into newAnalysis

@cameel cameel force-pushed the new-analysis-type-class-registration-pass branch from 2746135 to 98269f3 Compare September 11, 2023 16:33
@cameel
Copy link
Member Author

cameel commented Sep 11, 2023

soltest was failing due to the missing EVMVersion: >=constantinople. Fixed now.

@nikola-matic nikola-matic force-pushed the newAnalysis branch 2 times, most recently from b2363a8 to acad00f Compare September 14, 2023 05:56
@cameel cameel force-pushed the new-analysis-type-class-registration-pass branch from 98269f3 to da83b35 Compare September 14, 2023 15:34
@cameel cameel merged commit 2956345 into newAnalysis Sep 18, 2023
@cameel cameel deleted the new-analysis-type-class-registration-pass branch September 18, 2023 12:07
cameel added a commit that referenced this pull request Oct 10, 2023
…tration-pass

Separate type class registration pass in experimental analysis
cameel added a commit that referenced this pull request Oct 12, 2023
…tration-pass

Separate type class registration pass in experimental analysis
cameel added a commit that referenced this pull request Oct 13, 2023
…tration-pass

Separate type class registration pass in experimental analysis
cameel added a commit that referenced this pull request Oct 17, 2023
…tration-pass

Separate type class registration pass in experimental analysis
cameel added a commit that referenced this pull request Oct 23, 2023
…tration-pass

Separate type class registration pass in experimental analysis
cameel added a commit that referenced this pull request Dec 8, 2023
…tration-pass

Separate type class registration pass in experimental analysis
cameel added a commit that referenced this pull request Dec 13, 2023
…tration-pass

Separate type class registration pass in experimental analysis
cameel added a commit that referenced this pull request Dec 13, 2023
…tration-pass

Separate type class registration pass in experimental analysis
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants