Skip to content

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?


/** 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