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

Compiler Hangs and Runs Out of Memory for very small file #5580

Closed
scabug opened this issue Mar 17, 2012 · 9 comments
Closed

Compiler Hangs and Runs Out of Memory for very small file #5580

scabug opened this issue Mar 17, 2012 · 9 comments
Assignees
Milestone

Comments

@scabug
Copy link

@scabug scabug commented Mar 17, 2012

When I try to compile the following program, the compiler runs out of memory after 2 minutes on my machine. Reproduced with all of the above versions. (2.9.0, 2.9.1, 2.10.0-M2). I attached a small test project, including a Gradle build script.

import scala.collection.mutable.WeakHashMap
class bar{ }
class foo{
  val map = WeakHashMap[AnyRef, collection.mutable.Map[bar, collection.mutable.Set[bar]]]()

  def test={
    val tmp:bar=null
    if (map.get(tmp).isEmpty) map.put(tmp,collection.mutable.Set())
  }
}
@scabug

This comment has been minimized.

Copy link
Author

@scabug scabug commented Mar 17, 2012

Imported From: https://issues.scala-lang.org/browse/SI-5580?orig=1
Reporter: Ruedi Steinmann (ruediste)
Affected Versions: 2.9.0, 2.9.1, 2.10.0-M2
Other Milestones: 2.10.0
Attachments:

  • crash.zip (created on Mar 17, 2012 7:54:06 AM UTC, 1137 bytes)
  • gargantuan-type.txt (created on Mar 17, 2012 11:44:54 AM UTC, 284574 bytes)
@scabug

This comment has been minimized.

Copy link
Author

@scabug scabug commented Mar 17, 2012

@retronym said (edited on Mar 17, 2012 11:57:10 AM UTC):
There is a cavalcade of interesting issues illuminated by this snippet.

Your code contains a type error -- this change will make it correct: map.put(tmp,collection.mutable.Map()).

When the compiler encounters this error, it triggers an implicit search to convert map to a type with a suitable put method. Alarmingly, this search includes JavaConversions.asJavaDictionary, despite the absense of an import! Why, you may ask? Well, WeakHashMap extends JavaConversions.JMapWrapper, and the implicits of a nested type include those defined in its enclosing scopes (object or package object). This behaviour isn't specified, see: #4427. I suspect that this consequence of putting JMapWrapper in JavaConversions was unintended.

So, the compiler tries to use:

collection.JavaConversions.asJavaDictionary(map).put(tmp, collection.mutable.Set())

This line of code, when compiled verbatim fails as one expects. The compiler infers the type argument as:

scala.collection.JavaConversions.asJavaDictionary[AnyRef, scala.collection.mutable.Map[Bar,scala.collection.mutable.Set[Bar]]]

and fails with:

error: polymorphic expression cannot be instantiated to expected type;
 found   : [A]scala.collection.mutable.Set[A]
 required: scala.collection.mutable.Map[Bar,scala.collection.mutable.Set[Bar]]
                collection.JavaConversions.asJavaDictionary(map).put(tmp, collection.mutable.Set())
                                        

But for implicit views, things proceed along a different path. The inferred type arguments are:

collection.this.JavaConversions.asJavaDictionary[AnyRef, <284574 characters of inferred type elided>]

No, really. I've attached said [^gargantuan-type.txt] for posterity. Unsurprisingly, the second argument to put doesn't conform. That's when TypeDiagnostics takes over and brings down the compiler.

TypeDiagnostics has the admirable intention to add just enough qualification of identifiers in type errors to make them unambiguous, while still maintaining readability. Processes each pair of types referenced in the type error, and adding qualification to those with the same name that refer to distinct types. This cartesian product is not processed lazily, and it exhausts the heap.

It seems that this work is all for nought anyway -- we just want to rule out ineligible implicit views, the carefully crafted message wouldn't make it to the user anyway.

@scabug

This comment has been minimized.

Copy link
Author

@scabug scabug commented Mar 18, 2012

@paulp said:
Oh, oh, aiya.

Impressively, the code actually compiles as-is if I stop TypeDiagnostics from meddling.

It's hard for "cavalcade" to be an understatement, but in this case...

@scabug

This comment has been minimized.

Copy link
Author

@scabug scabug commented Mar 18, 2012

@paulp said:
Hey, you totally must have picked up "cavalcade" from my comment in JavaConversions. (Not that you learned the word, but that it was on your mind.)

  // Note to implementors: the cavalcade of deprecated methods herein should
  // serve as a warning to any who follow: don't overload implicit methods.
@scabug

This comment has been minimized.

Copy link
Author

@scabug scabug commented Mar 18, 2012

@retronym said:
I hadn't spotted that sage commentary -- it seems that JavaConversions itself has the capacity to inspire specific language from those it crosses. Neat.

I'm surprised it compiles once you removed the hurdle -- which implicit saves the day?

@scabug

This comment has been minimized.

Copy link
Author

@scabug scabug commented Mar 18, 2012

@paulp said:
I didn't note it; not sure. It did preserve the 285K type all the way until erasure.

@scabug

This comment has been minimized.

Copy link
Author

@scabug scabug commented Mar 21, 2012

@paulp said:
39938bcc29

@scabug scabug closed this Mar 21, 2012
@scabug

This comment has been minimized.

Copy link
Author

@scabug scabug commented Mar 21, 2012

@paulp said:
You can check for yourself now, as the version which compiles (after importing JavaConversions._) is test/files/pos/t5580b.scala .

@scabug

This comment has been minimized.

Copy link
Author

@scabug scabug commented Apr 4, 2013

@paulp said:
Nice moment of the day - after rewriting a core typer method and having a test failure, I sighed and took a look to find the test referenced above, with my comment:

/** It's a pos test because it does indeed compile,
 *  not so much because I'm glad it does.  Testing
 *  that error messages created and discarded during
 *  implicit search don't blow it up.
 */

Fixed as side effect of beautiful rewrite of isSameType.

test/files/pos/t5580b.scala:17: error: polymorphic expression cannot be instantiated to expected type;
 found   : [A]scala.collection.mutable.Set[A]
 required: scala.collection.mutable.Map[bar,scala.collection.mutable.Set[bar]]
    if (map.get(tmp).isEmpty) map.put(tmp,collection.mutable.Set())
                                                                ^
one error found
@scabug scabug added this to the 2.10.0-M2 milestone Apr 7, 2017
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
2 participants
You can’t perform that action at this time.