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

Improvements to function and record inference #1586

Merged
merged 7 commits into from
Nov 28, 2024

Conversation

oxisto
Copy link
Member

@oxisto oxisto commented Jun 11, 2024

This PR achieves several things:

  • We try to infer the return types of function based on their context
  • A simple heuristic to better decide whether a function should be inferred in a class or in a global scope
  • A simple heuristic to decide whether a record or a namespace should be inferred
  • We recursively infer nested namespaces, e.g. A::B::C

@oxisto oxisto changed the base branch from main to 2nd-try-type-normalisation June 11, 2024 10:40
@oxisto oxisto changed the title experimental inference return type Trying to infer return type of functions Jun 11, 2024
@oxisto oxisto force-pushed the experimental-inference-return-type branch from 6319ab6 to 5391395 Compare June 11, 2024 11:24
@oxisto oxisto changed the title Trying to infer return type of functions Improvements to function inference Jun 12, 2024
@oxisto oxisto force-pushed the 2nd-try-type-normalisation branch from 9ee49ac to abee755 Compare June 12, 2024 16:18
@oxisto oxisto force-pushed the experimental-inference-return-type branch 2 times, most recently from 2abc11d to fed387d Compare June 12, 2024 22:25
@oxisto oxisto force-pushed the 2nd-try-type-normalisation branch from abee755 to b101506 Compare June 13, 2024 13:02
@oxisto oxisto force-pushed the experimental-inference-return-type branch from 31dcfad to 7f79c38 Compare June 19, 2024 06:52
@oxisto oxisto changed the base branch from 2nd-try-type-normalisation to main June 19, 2024 06:53
@oxisto oxisto changed the base branch from main to 2nd-try-type-normalisation June 19, 2024 06:53
@oxisto oxisto force-pushed the experimental-inference-return-type branch from 7f79c38 to 9199f61 Compare June 19, 2024 06:53
@oxisto oxisto force-pushed the 2nd-try-type-normalisation branch from b101506 to 25e6dc9 Compare June 19, 2024 06:54
@oxisto oxisto force-pushed the experimental-inference-return-type branch from 9199f61 to ed0aec5 Compare June 19, 2024 06:54
@oxisto oxisto force-pushed the 2nd-try-type-normalisation branch 2 times, most recently from bb4e009 to 196265d Compare June 20, 2024 09:01
@oxisto oxisto changed the title Improvements to function inference Improvements to function and record inference Jun 20, 2024
@oxisto oxisto force-pushed the experimental-inference-return-type branch 2 times, most recently from 9b2ec8b to 0693ce2 Compare June 26, 2024 11:53
@oxisto oxisto force-pushed the 2nd-try-type-normalisation branch 2 times, most recently from 40f1e1a to 19744c3 Compare June 27, 2024 16:16
@oxisto oxisto force-pushed the experimental-inference-return-type branch 2 times, most recently from ec377d6 to 94e7940 Compare June 28, 2024 19:50
Base automatically changed from 2nd-try-type-normalisation to main July 2, 2024 14:21
@oxisto oxisto force-pushed the experimental-inference-return-type branch from 94e7940 to 25f505f Compare July 3, 2024 06:26
@oxisto oxisto marked this pull request as ready for review July 3, 2024 06:32
@oxisto oxisto changed the base branch from main to has-argument July 3, 2024 06:44
@oxisto oxisto force-pushed the has-argument branch 2 times, most recently from 2ff4961 to 0e7fcb6 Compare July 4, 2024 06:56
Base automatically changed from has-argument to main July 18, 2024 08:44
@oxisto oxisto changed the base branch from main to symbol-resolver-cleanup August 2, 2024 07:01
Base automatically changed from symbol-resolver-cleanup to main August 2, 2024 13:46
@oxisto oxisto mentioned this pull request Oct 4, 2024
@oxisto oxisto force-pushed the experimental-inference-return-type branch 2 times, most recently from 8573e0c to 2035f0e Compare November 21, 2024 14:08
Copy link

codecov bot commented Nov 21, 2024

Codecov Report

Attention: Patch coverage is 67.30769% with 34 lines in your changes missing coverage. Please review.

Project coverage is 76.24%. Comparing base (d2c7f28) to head (2642a3f).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
...fraunhofer/aisec/cpg/passes/inference/Inference.kt 65.51% 1 Missing and 19 partials ⚠️
...raunhofer/aisec/cpg/passes/inference/PassHelper.kt 68.42% 1 Missing and 11 partials ⚠️
...ain/kotlin/de/fraunhofer/aisec/cpg/ScopeManager.kt 0.00% 0 Missing and 1 partial ⚠️
...s/kotlin/de/fraunhofer/aisec/cpg/test/TestUtils.kt 50.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
Files with missing lines Coverage Δ
.../de/fraunhofer/aisec/cpg/InferenceConfiguration.kt 94.73% <100.00%> (+3.56%) ⬆️
...e/fraunhofer/aisec/cpg/frontends/LanguageTraits.kt 100.00% <ø> (ø)
.../de/fraunhofer/aisec/cpg/frontends/TestLanguage.kt 80.55% <100.00%> (ø)
...de/fraunhofer/aisec/cpg/frontends/cxx/CLanguage.kt 96.00% <ø> (ø)
...ain/kotlin/de/fraunhofer/aisec/cpg/ScopeManager.kt 80.05% <0.00%> (+2.74%) ⬆️
...s/kotlin/de/fraunhofer/aisec/cpg/test/TestUtils.kt 71.87% <50.00%> (+0.29%) ⬆️
...raunhofer/aisec/cpg/passes/inference/PassHelper.kt 80.48% <68.42%> (-0.13%) ⬇️
...fraunhofer/aisec/cpg/passes/inference/Inference.kt 76.28% <65.51%> (-2.47%) ⬇️

... and 5 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@oxisto oxisto force-pushed the experimental-inference-return-type branch from 5a2b3e8 to 2b7147c Compare November 21, 2024 14:24
Copy link
Contributor

@KuechA KuechA left a comment

Choose a reason for hiding this comment

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

I think the inference rules require some more discussion and a proper documentation to understand later why we did some things and took some assumptions...

Copy link
Contributor

@maximiliankaul maximiliankaul left a comment

Choose a reason for hiding this comment

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

I'm a bit undecided with this PR. I'm approving it, because it overall seems to be doing the job when looking at the tests. I like the positive examples demonstrated in the test cases. However, I'm not 100% sure on all the mechanics involved here. Maybe a short (in house) introduction to the inference logic would be beneficial...
I don't like all the tests being implemented for the CXX language. This inference logic is used for all languages, isn't it? I understand that there are plans of getting rid of the fluent dsl stuff for fronted-independent tests, but having everything only tested with CXX is a big negative point in my view.

@oxisto oxisto force-pushed the experimental-inference-return-type branch from 4d814d2 to 1e7ea0b Compare November 28, 2024 13:45
@Fraunhofer-AISEC Fraunhofer-AISEC deleted a comment from sonarcloud bot Nov 28, 2024
@oxisto oxisto enabled auto-merge (squash) November 28, 2024 13:58
@oxisto oxisto merged commit 07c1dd5 into main Nov 28, 2024
2 checks passed
@oxisto oxisto deleted the experimental-inference-return-type branch November 28, 2024 14:02
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 this pull request may close these issues.

4 participants