Improvements to function and record inference #1586
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
6319ab6
to
5391395
Compare
9ee49ac
to
abee755
Compare
2abc11d
to
fed387d
Compare
abee755
to
b101506
Compare
31dcfad
to
7f79c38
Compare
7f79c38
to
9199f61
Compare
b101506
to
25e6dc9
Compare
9199f61
to
ed0aec5
Compare
bb4e009
to
196265d
Compare
9b2ec8b
to
0693ce2
Compare
40f1e1a
to
19744c3
Compare
ec377d6
to
94e7940
Compare
94e7940
to
25f505f
Compare
oxisto
commented
Jul 3, 2024
| * A very EXPERIMENTAL feature. If this is enabled, we will try to infer return types of | ||
| * functions based on the context of the call it originated out of. This is disabled by default. | ||
| */ | ||
| val inferReturnTypes: Boolean, |
There was a problem hiding this comment.
I am somewhat satisfied with this feature, so we could also enable this by default
oxisto
commented
Jul 3, 2024
| @@ -68,4 +68,6 @@ interface ArgumentHolder : Holder<Expression> { | |||
| operator fun minusAssign(node: Expression) { | |||
| removeArgument(node) | |||
| } | |||
|
|
|||
| fun hasArgument(expression: Expression): Boolean | |||
There was a problem hiding this comment.
@konradweiss this is needed in order to check the context of a call expression in order to derive the function type. But adding this "polutes" the PR somewhat because I need to change all files that implement ArgumentHolder. I guess it would make sense to have this in a separate PR?
oxisto
commented
Jul 3, 2024
| // the following function are more for type-resolving, but they are placed inside the | ||
| // symbol resolver, because this allows us to have the necessary location of nodes where | ||
| // we infer from | ||
| is RecordDeclaration -> handleRecordDeclaration(node) |
There was a problem hiding this comment.
I need to check, I think this is not necessary anymore, this should already be handled by the type resolver now.
oxisto
commented
Jul 3, 2024
| val root = it.root as? ObjectType | ||
| root?.recordDeclaration | ||
| } | ||
| fun handleRecordDeclaration(record: RecordDeclaration) { |
There was a problem hiding this comment.
see above, I think this is not needed anymore
oxisto
commented
Jul 3, 2024
| @@ -528,6 +544,52 @@ class Inference internal constructor(val start: Node, override val ctx: Translat | |||
| this.scopeManager = ctx.scopeManager | |||
| this.typeManager = ctx.typeManager | |||
| } | |||
|
|
|||
| fun inferReturnType(call: CallExpression?): Type { | |||
oxisto
commented
Jul 3, 2024
| @@ -605,3 +667,197 @@ fun RecordDeclaration.inferMethod( | |||
| call | |||
| ) as? MethodDeclaration | |||
| } | |||
|
|
|||
| fun TranslationContext.tryNamespaceInference( | |||
oxisto
commented
Jul 3, 2024
| * Tries to infer a [RecordDeclaration] from an unresolved [Type]. This will return `null`, if | ||
| * inference was not possible, or if it was turned off in the [InferenceConfiguration]. | ||
| */ | ||
| fun TranslationContext.tryRecordInference( |
There was a problem hiding this comment.
add a quick comment that we are also inferring the (nested) scope of the record, if it does not exist
oxisto
commented
Jul 3, 2024
| return methods.any { it.name.localName == name } | ||
| } | ||
|
|
||
| fun TranslationContext.tryParentScopeInference( |
oxisto
commented
Jul 3, 2024
| @@ -58,7 +58,7 @@ class CXXAmbiguitiesTest { | |||
| assertNotNull(tu) | |||
|
|
|||
| // make sure we still have only one declaration in the file (the record) | |||
| assertEquals(1, tu.declarations.size) | |||
| assertEquals(3, tu.declarations.size) | |||
There was a problem hiding this comment.
need to double-check why it is 3 now
This adds a small utility function to `ArgumentHolder`, so we can check if an expression is part of their "arguments".
4c5cdca
to
e90e715
Compare
e90e715
to
4932845
Compare
|
2ff4961
to
0e7fcb6
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR achieves several things:
A::B::C