Skip to content

Conversation

@Jasper-M
Copy link
Contributor

fixes #5121

Copy link
Member

@dottybot dottybot left a comment

Choose a reason for hiding this comment

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

Hello, and thank you for opening this PR! 🎉

All contributors have signed the CLA, thank you! ❤️

Have an awesome day! ☀️

val explanation = ""
}

case class ValueClassParameterMayNotBeCallByName(valueClass: Symbol, param: Symbol)(implicit ctx: Context)
Copy link
Contributor

Choose a reason for hiding this comment

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

We should reuse VarValParametersMayNotBeCallByName instead and maybe addapt a bit it's explanation.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That was my first idea, but I thought it might be confusing, because it's not really a val and doesn't seem to behave like a case class parameter, which implicitly is a val, either.

Copy link
Contributor

Choose a reason for hiding this comment

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

You are right

@allanrenucci
Copy link
Contributor

@nicolasstucki I don't think we should forbid this. dotc does not require value class parameter to be a val as opposed to scalac, so we don't need to impose this restriction.

We should fix the infinite loop of course

@nicolasstucki nicolasstucki dismissed their stale review September 27, 2018 18:35

My concern was wrong

@nicolasstucki
Copy link
Contributor

nicolasstucki commented Sep 27, 2018

@Jasper-M then could you just change the commit message to be Fix #5121: Check if value class parameter is call-by-name. The fix looks good.

@nicolasstucki nicolasstucki merged commit 5b91393 into scala:master Sep 28, 2018
@nicolasstucki
Copy link
Contributor

Thanks @Jasper-M

@Blaisorblade
Copy link
Contributor

Thanks @Jasper-M, and nice to see you here!

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.

Infinite loop with by name value classes

5 participants