Skip to content

Fix #7416: Don't emit signatures of fields of primitive types #7417

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

Merged
merged 2 commits into from
Oct 15, 2019

Conversation

odersky
Copy link
Contributor

@odersky odersky commented Oct 13, 2019

Don't emit signatures for private fields of primitive type.

Maybe there's a better fix for this?

Don't emit signatures for private fields of primitive type
@odersky odersky requested a review from Duhemm October 13, 2019 13:26
@smarter
Copy link
Member

smarter commented Oct 13, 2019

scalac has a very similar fix: scala/scala@e3bee06

Copy link
Contributor

@Duhemm Duhemm left a comment

Choose a reason for hiding this comment

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

Since Scalac has a similar fix, would it make sense to port their test?

@@ -147,6 +147,12 @@ object TypeErasure {
def valueErasure(tp: Type)(implicit ctx: Context): Type =
erasureFn(isJava = false, semiEraseVCs = true, isConstructor = false, wildcardOK = false)(tp)(erasureCtx)

/** Like value class erasure, but value classes erase to their underlying type erasure */
def fullErasure(tp: Type)(implicit ctx: Context): Type =
valueErasure(tp) match
Copy link
Contributor

Choose a reason for hiding this comment

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

Was there some decision about mixing significant indentation syntax with braces syntax? (this match has no braces)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I currently use indentation syntax for all code I touch to get a better feeling for it. If we decide not to do indentation, we can rewrite to braces automatically.

The reason not to convert to indentation wholesale is that is would destroy commit history.

@odersky
Copy link
Contributor Author

odersky commented Oct 13, 2019

I don't have time to port the test (we do not have an analogue of GenericSignaturesTest, it seems) but if someone could do it that would be welcome!

@odersky odersky merged commit 2e2a2df into scala:master Oct 15, 2019
@odersky odersky deleted the fix-#7416 branch October 15, 2019 09:50
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