support/release for 2.12.1
fommil opened this issue · 10 comments
me and @sugakandrey are looking into this, because it's blocking ensime 2.12.1 support.
OK, I've analysed the failing PrettyPrinterTests. The problem is, that 2.12.1 does not set valDef.mods.positions in cases where 2.12.0 (although with off by one errors) sets it. This can be most clearly seen by enabling tracing, and adding a few trace statements (see mlangc@4c52a46#diff-27e36165688042a64da8ce038be3e404), and comparing the resulting traces in vimdiff. Let's look for example at test-setters-scala-2.12.0.txt and test-setters-scala-2.12.1.txt. The most relevant part of the resulting diff looks like
Now we have two options: We could either wait until this is fixed in the compiler, or we could change the pretty printer not to use positions at all, which seems wrong to me anyway. I will think about this. In the meantime it would be nice to have some feedback on this from people working on the compiler.
Great work! If this is a regression in scalac it shouldn't stop us from releasing. We could monkey patch if we know a fix in the compiler?
Do we know which commits caused this upstream? Have you pinged the authors?
I've not yet pinged upstream about this... I'll do so in the evening (but feel free to contact them right away if you don't want to wait 😉).
Unfortunately the information that used to be stored in valDef.mods.positions is critical for tree printing, as without it, I cannot differentiate between class C(val x: Int) and class C(private val x: Int).
My suggestion therefore is to deactivate the affected tests for 2.12.1 and see if we can get this fixed in the compiler. @fommil, @SethTisue do you have an idea who to ping on this?
@fommil, @SethTisue do you have an idea who to ping on this?
if you open a JIRA ticket, I'll make sure to bring it to the Scala team's attention.
@SethTisue thanks a lot! See https://issues.scala-lang.org/browse/SI-10163
Can you get an accurate position from valDef.symbol.position?
Yes valDef.symbol.pos is set, seems to be accurate, and is identical between 2.12.0 and 2.12.1; maybe I can use this position to obtain the information that is now extracted by looking at valDef.mods.positions.
I looked at this more closely and was able to fix some of the 2.12.1 related text failures by using valDef.symbol.pos and extracting the modifier positions by hand. I optimistically assume that I could fix all tests using this approach, but I don't think it's worth the effort because
- this problem affects refactorings based on tree printing only, which is broken anyway and is being replaced on a best effort basis
- in my opinion this should be fixed in the compiler