Skip to content
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

resolve status of scala/scala#8466 for 2.12.11 #672

Closed
SethTisue opened this issue Feb 27, 2020 · 5 comments
Closed

resolve status of scala/scala#8466 for 2.12.11 #672

SethTisue opened this issue Feb 27, 2020 · 5 comments
Assignees
Labels
Milestone

Comments

@SethTisue
Copy link
Member

@SethTisue SethTisue commented Feb 27, 2020

scala/scala#8466 caused a community build failure in scalariform;
needs investigation before releasing 2.12.11

@lrytz @retronym @mkeskells

@SethTisue

This comment has been minimized.

Copy link
Member Author

@SethTisue SethTisue commented Feb 27, 2020

failure log: https://scala-ci.typesafe.com/job/scala-2.12.x-integrate-community-build/5532/artifact/logs/scalariform-build.log

could our be regression, but it could perhaps just be some wrong/fragile assumption in scalariform

the problem is reproducible outside of dbuild as follows:

hub clone scala-ide/scalariform
cd scalariform
sbt
set resolvers += "scala-integration" at "https://scala-ci.typesafe.com/artifactory/scala-integration/" ++2.12.11-bin-466f92f
scalariform/testOnly *MiscExpressionFormatterTest
@SethTisue

This comment has been minimized.

Copy link
Member Author

@SethTisue SethTisue commented Feb 27, 2020

if it does turn out to be a regression in HashMap, it's a bit alarming that scala/scala-collection-laws (which is part of the community build) didn't catch it — would be nice to add an appropriate law there once the nature of the problem is understood

@retronym

This comment has been minimized.

Copy link
Member

@retronym retronym commented Feb 28, 2020

Looks like a regression in merging HashMap-s that are distinct.

This:

image

That:

image

Comes together in:

mergeDistinct$1:802, HashMap$HashTrieMap
merge0:878, HashMap$HashTrieMap
merged:137, HashMap
$plus$plus:184, HashMap
$plus$plus:173, HashMap
mergeWith:45, FormatResult

Which computes:

image

The allBits bitmask looks wrong. In 2.12.10, the bitmap is -259938295:

image

I'll construct a unit test with the same Trie structure.

@SethTisue

This comment has been minimized.

Copy link
Member Author

@SethTisue SethTisue commented Feb 28, 2020

@Ichoran can I interest you in adding a law for this?

@retronym

This comment has been minimized.

Copy link
Member

@retronym retronym commented Feb 28, 2020

I think the reason it might have slipped through is that foreach and iterator (and the stack of generic operations built atop that) seems to work for HashTries with a corrupt bitmap. I think that's because TrieIterator doesn't use the bitmap, it just uses instanceOf-s to discriminate between data and sub-nodes in the elements array.

HashMap.equals is sensitive to the bitmaps, as is .get.

@lrytz lrytz closed this Mar 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants
You can’t perform that action at this time.