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

Instantiation registration pass in new analysis #14735

Conversation

cameel
Copy link
Member

@cameel cameel commented Dec 15, 2023

Depends on #14734.
Prerequisite for #14721.

This is pass 2.25 from Experimental Type System Notes:

Pass 2.25: visit all type class instantiations (header, not body!), registering class instances with the type system (note: no validation of actually properly implemented functions yet!)

It will allow me to remove out-of order visitation of instantiations from #14721.

@cameel cameel requested a review from ekpyron December 15, 2023 14:48
@cameel cameel self-assigned this Dec 15, 2023
@cameel cameel force-pushed the new-analysis-instantiation-registration branch from 9c0d9b8 to 46c24c4 Compare December 15, 2023 14:49
Comment on lines +613 to +624
if (typeClassDefinition)
// We may have not visited the type class definition yet. Do it just in case.
// TODO: Type classes should be processed in a separate pass to avoid this out-of-order visitation.
typeClassDefinition->accept(*this);
Copy link
Member Author

Choose a reason for hiding this comment

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

Note that we still need to visit type class definitions out of order to get members registered. This will become unnecessary once we have a separate class member registration pass. #14578 was an early attempt to do that but I'm going to redo it on top of #14721 eventually.

@cameel cameel force-pushed the new-analysis-instantiation-registration branch from 46c24c4 to ac95ace Compare December 15, 2023 14:57
@cameel cameel added the has dependencies The PR depends on other PRs that must be merged first label Dec 15, 2023
@cameel cameel force-pushed the new-analysis-instantiation-registration branch 3 times, most recently from f991650 to 08c856c Compare December 19, 2023 13:08
@cameel cameel force-pushed the new-analysis-move-builtin-class-registration-to-class-registration-pass branch from 16e30e4 to a674c85 Compare December 19, 2023 13:08
@github-actions github-actions bot added the stale The issue/PR was marked as stale because it has been open for too long. label Jan 3, 2024
@cameel cameel removed the stale The issue/PR was marked as stale because it has been open for too long. label Jan 3, 2024
@github-actions github-actions bot added the stale The issue/PR was marked as stale because it has been open for too long. label Jan 18, 2024
@cameel cameel removed the stale The issue/PR was marked as stale because it has been open for too long. label Jan 18, 2024
@cameel cameel force-pushed the new-analysis-move-builtin-class-registration-to-class-registration-pass branch from a674c85 to 868f0bd Compare January 26, 2024 18:53
@cameel cameel force-pushed the new-analysis-instantiation-registration branch from 08c856c to 858d874 Compare January 26, 2024 18:53
@github-actions github-actions bot added the stale The issue/PR was marked as stale because it has been open for too long. label Feb 10, 2024
@cameel cameel removed the stale The issue/PR was marked as stale because it has been open for too long. label Feb 11, 2024
@ethereum ethereum deleted a comment from github-actions bot Feb 11, 2024
@ethereum ethereum deleted a comment from github-actions bot Feb 11, 2024
@ethereum ethereum deleted a comment from github-actions bot Feb 11, 2024
@cameel cameel force-pushed the new-analysis-move-builtin-class-registration-to-class-registration-pass branch from 868f0bd to 56cfd69 Compare February 12, 2024 15:50
@cameel cameel force-pushed the new-analysis-instantiation-registration branch from 858d874 to 78d9d43 Compare February 12, 2024 15:57
@cameel cameel force-pushed the new-analysis-move-builtin-class-registration-to-class-registration-pass branch from 56cfd69 to 66309c7 Compare February 12, 2024 15:57
@cameel cameel force-pushed the new-analysis-move-builtin-class-registration-to-class-registration-pass branch from 66309c7 to 2b410a7 Compare February 19, 2024 20:28
Copy link

github-actions bot commented Mar 5, 2024

This pull request is stale because it has been open for 14 days with no activity.
It will be closed in 7 days unless the stale label is removed.

@github-actions github-actions bot added the stale The issue/PR was marked as stale because it has been open for too long. label Mar 5, 2024
@cameel cameel removed the stale The issue/PR was marked as stale because it has been open for too long. label Mar 6, 2024
Copy link

This pull request is stale because it has been open for 14 days with no activity.
It will be closed in 7 days unless the stale label is removed.

@github-actions github-actions bot added the stale The issue/PR was marked as stale because it has been open for too long. label Mar 20, 2024
@cameel cameel removed the stale The issue/PR was marked as stale because it has been open for too long. label Mar 20, 2024
Copy link

github-actions bot commented Apr 4, 2024

This pull request is stale because it has been open for 14 days with no activity.
It will be closed in 7 days unless the stale label is removed.

@github-actions github-actions bot added the stale The issue/PR was marked as stale because it has been open for too long. label Apr 4, 2024
@cameel cameel removed the stale The issue/PR was marked as stale because it has been open for too long. label Apr 4, 2024
@ekpyron ekpyron closed this Apr 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
experimental has dependencies The PR depends on other PRs that must be merged first
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants