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

Introduction of concepts for next-generation call resolving #1496

Merged
merged 11 commits into from
Apr 12, 2024

Conversation

oxisto
Copy link
Member

@oxisto oxisto commented Apr 2, 2024

This PR tries to implement the initial concepts described in #704 and also migrates the resolving of regular function calls to the new system.

Fixes #406
Fixes #1498
Fixes #1497

@oxisto oxisto force-pushed the next-gen-call-resolver branch from adf271c to 695537e Compare April 3, 2024 07:30
@oxisto oxisto changed the title Initial skeleton of new call resolving Introduction of next-generation call resolving Apr 3, 2024
@oxisto oxisto changed the title Introduction of next-generation call resolving Introduction of concepts for next-generation call resolving Apr 3, 2024
@oxisto oxisto force-pushed the next-gen-call-resolver branch from 7cbda07 to 375dc20 Compare April 3, 2024 20:40
@oxisto oxisto marked this pull request as ready for review April 3, 2024 20:46
@oxisto oxisto requested a review from KuechA as a code owner April 4, 2024 13:53
@oxisto oxisto force-pushed the next-gen-call-resolver branch from 409c8a1 to 2d569fd Compare April 5, 2024 09:34
@KuechA
Copy link
Contributor

KuechA commented Apr 5, 2024

Did you check if this PR fixes #1045?

@oxisto oxisto force-pushed the next-gen-call-resolver branch from 3214f4e to ba50ede Compare April 5, 2024 12:05
@oxisto
Copy link
Member Author

oxisto commented Apr 5, 2024

Did you check if this PR fixes #1045?

I theory yes, in practice this still fails because of #194 because the function definition and declaration are not connected and both end up as candidates and we therefore think this is a problematic result :(

@oxisto oxisto force-pushed the next-gen-call-resolver branch from 2863c40 to 079e781 Compare April 5, 2024 15:36
@oxisto
Copy link
Member Author

oxisto commented Apr 5, 2024

Did you check if this PR fixes #1045?

I theory yes, in practice this still fails because of #194 because the function definition and declaration are not connected and both end up as candidates and we therefore think this is a problematic result :(

After #1484, this now also fixes #1045, at least for the case where the stdlib is included. Otherwise we cannot know that size_t is an integer and it will not match.

@oxisto oxisto linked an issue Apr 5, 2024 that may be closed by this pull request
@oxisto
Copy link
Member Author

oxisto commented Apr 5, 2024

@KuechA I will wait until next week to merge this after @maximiliankaul and @konradweiss are back for some more discussions. It is important to get the concepts correct this time.

@oxisto oxisto requested a review from maximiliankaul April 5, 2024 16:14
@oxisto oxisto added the blocked Blocked by an external factor label Apr 5, 2024
@oxisto oxisto force-pushed the next-gen-call-resolver branch from 4c965f5 to 0458666 Compare April 6, 2024 22:36
Copy link
Collaborator

@konradweiss konradweiss left a comment

Choose a reason for hiding this comment

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

The functionality looks like what we want and I have only some minor remarks on documentation and naming.

@oxisto oxisto force-pushed the next-gen-call-resolver branch from 806fdc2 to 5bfbdd0 Compare April 11, 2024 18:07
@oxisto oxisto requested a review from konradweiss April 11, 2024 18:08
Copy link

sonarcloud bot commented Apr 11, 2024

@oxisto oxisto removed the blocked Blocked by an external factor label Apr 11, 2024
@oxisto oxisto merged commit aaa979f into main Apr 12, 2024
3 checks passed
@oxisto oxisto deleted the next-gen-call-resolver branch April 12, 2024 07:47
oxisto added a commit that referenced this pull request Apr 12, 2024
This should have been part of #1496 but I forgot to clean this up.
oxisto added a commit that referenced this pull request Apr 13, 2024
This should have been part of #1496 but I forgot to clean this up.
oxisto added a commit that referenced this pull request Apr 13, 2024
This should have been part of #1496 but I forgot to clean this up.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants