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

Two receivers (extension and dispatch) support #234

Merged
merged 6 commits into from
Aug 25, 2024

Conversation

GrigoriiSolnyshkin
Copy link
Collaborator

  • This PR allows to correctly convert methods like
class Class {
  fun OtherClass.extensionMember() { }
}

(into function with two arguments).

  • It also supports lookup of this which chooses dispatch/extension receiver, but does not take into consideration this parameters from outer scopes.
  • It does not do the same thing for properties although it should be done eventually

Copy link
Owner

@jesyspa jesyspa left a comment

Choose a reason for hiding this comment

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

Thanks, this generally looks good! Some questions on the details

Comment on lines 40 to 41
if (isExtension) signature.run { extensionReceiver }
else signature.run { dispatchReceiver }
Copy link
Owner

Choose a reason for hiding this comment

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

Why not just signature.extensionReceiver?

Comment on lines 51 to 52
if (isExtension) substitutions[ExtraSpecialNames.E_THIS]
else substitutions[ExtraSpecialNames.D_THIS]
Copy link
Owner

Choose a reason for hiding this comment

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

For names in the code, I'd prefer to avoid such abbreviations

@@ -165,7 +165,7 @@ class ProgramConverter(val session: FirSession, override val config: PluginConfi
val receiverType: TypeEmbedding? = type.receiverType(session)?.let { embedType(it) }
val paramTypes: List<TypeEmbedding> = type.valueParameterTypesWithoutReceivers(session).map(::embedType)
val returnType: TypeEmbedding = embedType(type.returnType(session))
val signature = CallableSignatureData(receiverType, paramTypes, returnType)
val signature = CallableSignatureData(receiverType, null, paramTypes, returnType)
Copy link
Owner

Choose a reason for hiding this comment

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

Why can't we resolve this type at this point?

Comment on lines +222 to +211
val isExtensionReceiverUnique = symbol.receiverParameter?.isUnique(session) ?: false
val isExtensionReceiverBorrowed = symbol.receiverParameter?.isBorrowed(session) ?: false
Copy link
Owner

Choose a reason for hiding this comment

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

Is this information always about the extension receiver? That seems a bit strange

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

At least I don't quite understand here what would be the syntax for dispatch. If it is not possible to mark the dispatch receiver as unique directly, I suppose we could introduce a special annotation @MemberUnique for member functions specifically.

Copy link
Owner

Choose a reason for hiding this comment

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

@francescoo22 We definitely want to be able to have the dispatch receiver be unique. I would expect @receiver:Unique to work.

Copy link
Collaborator

Choose a reason for hiding this comment

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

As far as I know it's not possible to annotate the dispatch receiver with @receiver:Unique.
I think @GrigoriiSolnyshkin is right, we need to introduce a special annotation like @receiverUnique and annotate the function.

This is what the Kotlin doc says about the receiver target for annotations: "receiver (receiver parameter of an extension function or property)"

Copy link
Owner

Choose a reason for hiding this comment

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

Ah, that's annoying. LGTM then

Comment on lines +161 to +162
private fun embeddedVarByIndex(ix: Int): VariableEmbedding =
resolveByIndex(ix, { signature.dispatchReceiver!! }) { signature.params[it] }
Copy link
Owner

Choose a reason for hiding this comment

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

What are the rules on using an extension receiver here? I'm guessing we just don't support it.

Comment on lines +222 to +211
val isExtensionReceiverUnique = symbol.receiverParameter?.isUnique(session) ?: false
val isExtensionReceiverBorrowed = symbol.receiverParameter?.isBorrowed(session) ?: false
Copy link
Owner

Choose a reason for hiding this comment

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

@francescoo22 We definitely want to be able to have the dispatch receiver be unique. I would expect @receiver:Unique to work.

Comment on lines +51 to +52
if (isExtension) substitutions[ExtraSpecialNames.EXTENSION_THIS]
else substitutions[ExtraSpecialNames.DISPATCH_THIS]
Copy link
Owner

Choose a reason for hiding this comment

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

Modifying the key type would probably be the more elegant way to deal with this, but it's okay to do as-is.

@GrigoriiSolnyshkin GrigoriiSolnyshkin merged commit c696346 into formal-verification Aug 25, 2024
1 check passed
@GrigoriiSolnyshkin GrigoriiSolnyshkin deleted the multiple-receivers branch August 25, 2024 22:37
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.

3 participants