Make tests pass #623

odersky · 2015-05-29T13:58:13Z

Subsumes #621, adding fixes for the test failures. #580 is open again. @review by @DarkDimius @smarter

Constants that are adapted to a different supertype need to do this explicitly (not just by changing the type). Otherwise tree checkers will compute the original type and fail. This caused a test failure in pos/harmonize. The mystery is why this was not caught in the checkin tests.

…nclosing class is also the top-level class" This reverts commit 6898d2c.

Previously was only done for DefDefs. Caused backend failure.

odersky · 2015-05-29T16:30:09Z

I believe the tests hang. Running locally shows that lots of tests fail, all because of the revert of 78aa682.
To summarize:

If 78aa682 is enabled junit tests fail because we cannot compile ClassPath.
If the commit is disabled lots of run tests fail with verify errors. For instance:

Users/odersky/workspace/dotty/tests/run> dotc arybufgrow.scala -d /classes
/Users/odersky/workspace/dotty/tests/run> java Test
Exception in thread "main" java.lang.VerifyError: Bad local variable type
Exception Details:
Location:
Test$.$anonfun$$init$$1(I)Lscala/collection/mutable/ArrayBuffer; @0: aload_0
Reason:
Type integer (current frame, locals[0]) is not assignable to reference type
Current Frame:
bci: @0
flags: { }
locals: { integer }
stack: { }
Bytecode:
0x0000000: 2ab6 005e 126f b600 73b0

The other failures are similar.

I am at the end of what I can do. @DarkDimius you need to take this over. It's really important to get the build back to work quickly!

DarkDimius · 2015-05-29T16:31:39Z

@odersky, taking this over.

DarkDimius · 2015-05-29T16:38:30Z

this failure is indeed manifestation of reverted commit:

private static scala.collection.mutable.ArrayBuffer $anonfun$$init$$1(int);
    Code:
       0: aload_0
       1: invokevirtual #94                 // Method buf:()Lscala/collection/mutable/ArrayBuffer;

This is static method, that tries to access this.

odersky · 2015-05-29T16:38:42Z

Curiously, the test of #580 was not affected by reverting the fix. It still works, but lots of other run tests fail. So I am bringing it back. @DarkDimius over to you now.

odersky · 2015-05-29T16:40:27Z

@DarkDimius You might want to cherry-pick db11fc5
It did not help when I tested it, but maybe it just needs tweaking.

smarter · 2015-05-30T17:50:55Z

/rebuild

smarter · 2015-05-30T17:53:00Z

@DarkDimius : Can you get this to rebuild? It seems that Jenkins restarted and now it's stuck saying "validate-partest — [85] Building. " even though the job was aborted.

See scala#624 (comment) for a lengthy explanation.

smarter · 2015-05-30T17:53:53Z

Forced rebuild by changing the commit message.

smarter · 2015-05-30T18:13:57Z

Partest now finish running! It fails because of tests/run/t6260-delambdafy.scala for exactly the reason I explained in: smarter@4d23983#commitcomment-11339871 It tries to check an implementation detail that can change. I'm going to push another commit to disable checking the methods of the lambda.

DarkDimius · 2015-05-30T18:16:49Z

@smarter, we should not disable checking of lamdas. Tests exist to ensure that the way how lambdas are generated does not change without our notice.
If the behavior changes, and the change is expected, the check file should be updated.

smarter · 2015-05-30T18:21:55Z

OK, I just updated the check file, but this might be annoying if we change the details of lambda generation often.

- Only transform static methods which are inside module classes. - Make sure that the prefix of the underlying type of the Ident is a ThisType of the current module class. For example in "scala.Int.box(42)", "box" is an Ident whose underlying type is "TermRef(ThisType(TypeRef(ThisType(TypeRef(NoPrefix,scala)),Int$)),box)", but we should not trigger an assertion in this case.

smarter · 2015-05-30T20:45:56Z

After fixing ElimStaticThis, all tests now pass! ✨ ✨
@DarkDimius: For some reason github says that validate-partest is still Building., but if you click on https://scala-ci.typesafe.com/job/dotty-master-validate-partest/88/ you'll see that the build finished successfully.

smarter · 2015-05-31T07:04:36Z

/synch

odersky · 2015-05-31T09:40:48Z

src/dotty/tools/dotc/transform/ElimStaticThis.scala

+    // so instead we require this phase to run after Flatten and use meth.owner
+    if (meth.is(JavaStatic) && meth.owner.is(ModuleClass)) {
+      tree.tpe match {
+        case TermRef(thiz: ThisType, _) if (thiz.underlying.typeSymbol == meth.owner) =>


Instead of .underlying.termSymbol it's simpler to use just .cls.

odersky · 2015-05-31T09:42:38Z

LGTM. Glad to have this working in master.

Make tests pass

DarkDimius · 2015-06-01T12:46:50Z

@odersky, some of the changes that have been merged in this PR should be reverted.
3d240ad actually broke the transformation of inner objects, and it needs to be reverted if we want to move labdas to static methods in companion class.

odersky added 4 commits May 29, 2015 15:45

Revert "Fix scala#580: use isContainedIn to support cases where the e…

78aa682

…nclosing class is also the top-level class" This reverts commit 6898d2c.

Move test that fails again to disabled.

d471211

Stop running scala/scala tests instead of our junit tests.

3350907

This was referenced May 29, 2015

Stop running scala/scala tests instead of our junit tests. #621

Closed

Implement Scala2 traits #622

Merged

odersky added 2 commits May 29, 2015 17:21

Eliminate `_' from rhs of ValDefs

c65504d

Previously was only done for DefDefs. Caused backend failure.

Bring back disabled test.

878b55a

New phase: ElimStaticThis

dd80fe0

DarkDimius force-pushed the make-tests-pass branch from c6c4c9f to dd80fe0 Compare May 29, 2015 16:56

DarkDimius mentioned this pull request May 30, 2015

Add/elim static this #624

Closed

smarter force-pushed the make-tests-pass branch from 3926ddf to beae6e8 Compare May 30, 2015 17:43

Avoid static initialization deadlock in run tests.

8f90105

See scala#624 (comment) for a lengthy explanation.

smarter force-pushed the make-tests-pass branch from beae6e8 to 8f90105 Compare May 30, 2015 17:53

Update t6260-delambdafy.check to account for change in lambda generation

cafd71a

odersky reviewed May 31, 2015
View reviewed changes

dottybot added the reviewed label May 31, 2015

odersky added a commit that referenced this pull request May 31, 2015

Merge pull request #623 from dotty-staging/make-tests-pass

5d99a49

Make tests pass

odersky merged commit 5d99a49 into scala:master May 31, 2015

smarter mentioned this pull request Jun 1, 2015

Fix/initializer deadlocks #628

Merged

allanrenucci deleted the make-tests-pass branch December 14, 2017 19:23

Make tests pass #623

Make tests pass #623

Uh oh!

Conversation

odersky commented May 29, 2015

Uh oh!

odersky commented May 29, 2015

Uh oh!

DarkDimius commented May 29, 2015

Uh oh!

DarkDimius commented May 29, 2015

Uh oh!

odersky commented May 29, 2015

Uh oh!

odersky commented May 29, 2015

Uh oh!

smarter commented May 30, 2015

Uh oh!

smarter commented May 30, 2015

Uh oh!

smarter commented May 30, 2015

Uh oh!

smarter commented May 30, 2015

Uh oh!

DarkDimius commented May 30, 2015

Uh oh!

smarter commented May 30, 2015

Uh oh!

smarter commented May 30, 2015

Uh oh!

smarter commented May 31, 2015

Uh oh!

odersky May 31, 2015

Choose a reason for hiding this comment

Uh oh!

odersky commented May 31, 2015

Uh oh!

DarkDimius commented Jun 1, 2015

Uh oh!

Uh oh!