-
Notifications
You must be signed in to change notification settings - Fork 60
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
base: main
Are you sure you want to change the base?
Conversation
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
* 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am somewhat satisfied with this feature, so we could also enable this by default
@@ -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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@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?
// 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I need to check, I think this is not necessary anymore, this should already be handled by the type resolver now.
val root = it.root as? ObjectType | ||
root?.recordDeclaration | ||
} | ||
fun handleRecordDeclaration(record: RecordDeclaration) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
see above, I think this is not needed anymore
@@ -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 { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
kdoc
@@ -605,3 +667,197 @@ fun RecordDeclaration.inferMethod( | |||
call | |||
) as? MethodDeclaration | |||
} | |||
|
|||
fun TranslationContext.tryNamespaceInference( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
kdoc
* 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
add a quick comment that we are also inferring the (nested) scope of the record, if it does not exist
return methods.any { it.name.localName == name } | ||
} | ||
|
||
fun TranslationContext.tryParentScopeInference( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
kdoc
@@ -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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
This PR achieves several things:
A::B::C