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.

Sign up for GitHub

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

Jump to bottom

posTwice testNonCyclic fails if dotc.CompilationUnit does not import Types.Type #3383

Closed
nicolasstucki opened this issue Oct 25, 2017 · 2 comments
Closed

posTwice testNonCyclic fails if dotc.CompilationUnit does not import Types.Type #3383

nicolasstucki opened this issue Oct 25, 2017 · 2 comments
Assignees
@odersky
Labels
area:typer exp:advanced itype:bug

Comments

@nicolasstucki
Copy link
Contributor

If we remove the unused import import dotty.tools.dotc.core.Types.Type in dotty.tools.dotc.CompilationUnit the testNonCyclic fails in posTwice.

-- [E006] Unbound Identifier Error: ../compiler/src/dotty/tools/dotc/ast/Trees.scala:817:42 
817 |  abstract class Instance[T >: Untyped <: Type] extends DotClass { inst =>
    |                                          ^^^^
    |                                          not found: type Type

Note that the error manifests itself in another class.
@odersky odersky mentioned this issue Jan 2, 2018
Merged
nicolasstucki added a commit that referenced this issue Jan 3, 2018
@nicolasstucki
Merge pull request #3734 from dotty-staging/fix-#3383
22c0f91 
Fix #3383: Don't fail on unreported errors
nicolasstucki added a commit to dotty-staging/dotty that referenced this issue Jan 8, 2018
@nicolasstucki
Remove unused import after scala#3383 fix
354e2eb
@nicolasstucki
Copy link
Contributor Author

#3778 showed that this issue is not completely fixed. Now testNonCyclic passes but testIssue34 fails (http://dotty-ci.epfl.ch/lampepfl/dotty/2372/4).

@nicolasstucki nicolasstucki reopened this Jan 9, 2018
@nicolasstucki
Copy link
Contributor Author

To consider this issue as fixed we should include 354e2eb.

@nicolasstucki nicolasstucki mentioned this issue Jan 9, 2018
Closed
nicolasstucki added a commit to dotty-staging/dotty that referenced this issue Aug 17, 2018
@nicolasstucki
Fix scala#3383: Add regresion test
531ef1d
nicolasstucki added a commit that referenced this issue Aug 17, 2018
@nicolasstucki
Merge pull request #4956 from dotty-staging/fix-#3383
d00bdb3 
Fix #3383: Add regression test
smarter added a commit to smarter/dotty that referenced this issue Feb 2, 2019
@smarter
Workaround scala#3383
feeaebe 
Problem reintroduced after last commit.
smarter added a commit to dotty-staging/dotty that referenced this issue Feb 3, 2019
@smarter
Make all objects Serializable
83ee246 
To avoid deadlocks when combining objects, lambdas and multi-threading,
lambdas in objects are compiled to instance methods of the module class
instead of static methods (see tests/run/deadlock.scala and
scala/scala-dev#195 for details).

This has worked well for us so far but this is problematic for
serialization: serializing a lambda requires serializing all the values
it captures, if this lambda is in an object, this means serializing the
enclosing object, which fails if the object does not extend
Serializable.

Because serializing objects is basically free since scala#5775, it seems like
the simplest solution is to simply make all objects Serializable, this
certainly seems preferable to deadlocks.

For some reason, this commit causes the issue described in scala#3383 to
reappear, we add a workaround for that in Trees.scala.
smarter added a commit to dotty-staging/dotty that referenced this issue Feb 3, 2019
@smarter
Make all objects Serializable
bb770fe 
To avoid deadlocks when combining objects, lambdas and multi-threading,
lambdas in objects are compiled to instance methods of the module class
instead of static methods (see tests/run/deadlock.scala and
scala/scala-dev#195 for details).

This has worked well for us so far but this is problematic for
serialization: serializing a lambda requires serializing all the values
it captures, if this lambda is in an object, this means serializing the
enclosing object, which fails if the object does not extend
Serializable.

Because serializing objects is basically free since scala#5775, it seems like
the simplest solution is to simply make all objects Serializable, this
certainly seems preferable to deadlocks.

For some reason, this commit causes the issue described in scala#3383 to
reappear, we add a workaround for that in Trees.scala.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@odersky odersky

Labels
area:typer exp:advanced itype:bug
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@odersky @nicolasstucki