Skip to content

Conversation

@Godin
Copy link
Member

@Godin Godin commented Jun 3, 2019

Similarly to #773 constructors in Kotlin can have default arguments with branches, what unfortunately was overlooked during implementation of #774 - currently we filter them out, because they gets compiled differently than functions:

class Example() {
    constructor(a: Boolean, b: String = if (a) "a" else "b") : this()
}

results in constructor with following signature

public synthetic <init>(ZLjava/lang/String;ILkotlin/jvm/internal/DefaultConstructorMarker;)V

This was discovered during implementation of #887.

@Godin Godin added this to the 0.8.5 milestone Jun 2, 2019
@Godin Godin self-assigned this Jun 2, 2019
@Godin Godin added type: bug 🐛 Something isn't working and removed type: enhancement labels Jun 3, 2019
@Godin Godin changed the title Do not filter out branches in Kotlin default arguments in constructors Do not filter out synthetic constructors that contain values of default arguments in Kotlin Jun 3, 2019
@Godin Godin requested a review from marchof June 4, 2019 00:20
return;
}

if (KotlinDefaultArgumentsFilter
Copy link
Member

Choose a reason for hiding this comment

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

Side note: When I see this I more and more like @Godin 's idea to separate filters for different compilers.

Copy link
Member Author

Choose a reason for hiding this comment

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

@marchof don't know about whom you're talking 😆

Not sure that separation can remove this dependency between filters, i.e. not every but some synthetic methods anyway should be filtered in Kotlin.

Copy link
Member

@marchof marchof left a comment

Choose a reason for hiding this comment

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

Some minor formal issues from my side.

@Godin Godin requested a review from marchof June 5, 2019 09:35
@Godin
Copy link
Member Author

Godin commented Jun 5, 2019

@marchof fixed, also updated javadoc of filter according to changes, please re-review

Copy link
Member

@marchof marchof left a comment

Choose a reason for hiding this comment

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

@Godin Thx for adressing the issues!

@marchof marchof merged commit 219088d into master Jun 5, 2019
@marchof marchof deleted the kotlin_default_arguments branch June 5, 2019 14:26
@jacoco jacoco locked as resolved and limited conversation to collaborators Oct 11, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

Archived in project
Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants