Skip to content

Add a test for supercalls in traits. #611

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

Merged
merged 8 commits into from
May 28, 2015

Conversation

DarkDimius
Copy link
Contributor

review by @odersky

@DarkDimius DarkDimius force-pushed the supercalls-traits-test branch from 6c276dc to 0733b5f Compare May 27, 2015 16:18
@DarkDimius DarkDimius changed the title Add a test that tests supercalls in traits. Add a test for supercalls in traits. May 27, 2015
DarkDimius and others added 7 commits May 27, 2015 19:17
   ()Unit  translates to   ()BoxedUnit   not   BoxedUnit.
A class might implement several fields in inherited traits with the same and type.
In that case only one getter should be produced, but all initializing expressions
have to be executed.
Also added non-unit fields and a class that directly implements two traits with the same fields.
@odersky odersky force-pushed the supercalls-traits-test branch from 0733b5f to 05679f3 Compare May 27, 2015 17:23
@odersky
Copy link
Contributor

odersky commented May 27, 2015

Added fixes and rebased to master.

@odersky
Copy link
Contributor

odersky commented May 27, 2015

I get a non-zero exit code on run/t261.scala. But running the test manually succeeds with the expected output. Can someone help me to figure out what might be wrong here?

@DarkDimius
Copy link
Contributor Author

t261.scala also runs fine for me from command line, but fails in partest.

@DarkDimius
Copy link
Contributor Author

Ok, after cleaning environment on my machine(git clean) t261 also fails in CLI.

Exception in thread "main" java.lang.AbstractMethodError: Method Test$.B$$foo()Ljava/lang/String; is abstract
    at Test$.B$$foo(t261.scala)
    at B.f(t261.scala:3)
    at Test$.main(t261.scala:8)
    at Test.main(t261.scala)

Seems like this is a real bug in dotty.

@DarkDimius DarkDimius force-pushed the supercalls-traits-test branch from 05679f3 to 512bb15 Compare May 27, 2015 19:26
@DarkDimius
Copy link
Contributor Author

rebased over commit that will force jenkins to be more verbose about why tests fail. In particular, it should print the exception message shown above

@DarkDimius
Copy link
Contributor Author

running git clean -x -d -n in the repo will show(but not remove) which files and directories that you have are not present in the repo, and thus not present in jenkins.

@vsalvis
Copy link
Contributor

vsalvis commented May 27, 2015

Cool, now that's progress! So it sounds like the dotc script needs to clean up/rebuild some more things than it does now?

@odersky
Copy link
Contributor

odersky commented May 27, 2015

OK, let me summarize, even though it's probably obvious. We have two issues
here:

(1) Why does git clean make a difference. We seem to have some classpath
pollution in our tests here.

(2) Why the difference between partest and local run?

  • Martin

On Wed, May 27, 2015 at 9:07 PM, Dmitry Petrashko [email protected]
wrote:

t261.scala also runs fine for me from command line, but fails in partest.


Reply to this email directly or view it on GitHub
#611 (comment).

Martin Odersky
EPFL

@DarkDimius
Copy link
Contributor Author

(2) Why the difference between partest and local run?

t261 was succeeding in local run for me before I deleted class-files that had in tests/run. After I've deleted them, test failed locally. Indeed, I guess we have some kind of classpath pollution.

@odersky, which command do you use to run dotty-generated files?

@DarkDimius
Copy link
Contributor Author

In my case, the source of problem was:
I had a Test.class left in the root of the project(under ./workspace/dotty/).
It was copied over by sbt to dotty jar. And that obviously created problems, as depending on how I write the classpath by hand, I'll get different class loaded by VM.

@odersky
Copy link
Contributor

odersky commented May 27, 2015

I just use

java <classname>

to run.

@odersky
Copy link
Contributor

odersky commented May 27, 2015

Hmm. I don't have any classes in workspace/dotty.

@odersky
Copy link
Contributor

odersky commented May 27, 2015

@DarkDimius Good to see the excepion trace that caused the error. And it's quite likely it has something to do with the changes. But I am still puzzled why I am not seeing this if I run the tests locally.

@DarkDimius
Copy link
Contributor Author

@odersky

I just use
java
to run.

As you aren't including the dotty-defined or scala-define artifacts in the classpath, where does scala.Predef come from? Where does dotty.runtime.* come from?

eg:

dark@tsf-476-wpa-4-002 ~/workspace/dotty/tests/run ((3fbd5e9...)) $ dotc t261.scala
dark@tsf-476-wpa-4-002 ~/workspace/dotty/tests/run ((3fbd5e9...)) $ java Test
Exception in thread "main" java.lang.NoClassDefFoundError: scala/Predef$
    at Test$.main(t261.scala:6)
    at Test.main(t261.scala)
Caused by: java.lang.ClassNotFoundException: scala.Predef$
    at java.net.URLClassLoader$1.run(URLClassLoader.java:372)
    at java.net.URLClassLoader$1.run(URLClassLoader.java:361)
    at java.security.AccessController.doPrivileged(Native Method)
    at java.net.URLClassLoader.findClass(URLClassLoader.java:360)
    at java.lang.ClassLoader.loadClass(ClassLoader.java:424)
    at sun.misc.Launcher$AppClassLoader.loadClass(Launcher.java:308)
    at java.lang.ClassLoader.loadClass(ClassLoader.java:357)
    ... 2 more

@odersky
Copy link
Contributor

odersky commented May 27, 2015

Predef and Runtime are on my classpath.

@odersky
Copy link
Contributor

odersky commented May 27, 2015

I checked the javap output and I do find a B$$foo in Test$. So it seems the partest is somehow pulling in a different classfile.

public java.lang.String B$$foo();
Code:
0: aload_0
1: getfield #37 // Field B$$foo$$local:Ljava/lang/String;
4: areturn

@DarkDimius
Copy link
Contributor Author

@odersky, can you please show your entire classpath? I guess the problem could be a similar one, with shadowing on classpath.
The Test that I see on my machine, after compiling t261.scala by hand does not have a B$$foo() defined:

dark@tsf-476-wpa-4-002 ~/workspace/dotty/tests/run ((3fbd5e9...)) $ javap -p -v Test
Classfile /Users/dark/workspace/dotty/tests/run/Test.class
  Last modified May 27, 2015; size 356 bytes
  MD5 checksum c2b61d269cdfc2e72950209792a543d6
  Compiled from "t261.scala"
public final class Test
  minor version: 0
  major version: 52
  flags: ACC_PUBLIC, ACC_FINAL, ACC_SUPER
Constant pool:
   #1 = Utf8               Test
   #2 = Class              #1             // Test
   #3 = Utf8               java/lang/Object
   #4 = Class              #3             // java/lang/Object
   #5 = Utf8               t261.scala
   #6 = Utf8               f
   #7 = Utf8               ()V
   #8 = Utf8               Test$
   #9 = Class              #8             // Test$
  #10 = Utf8               MODULE$
  #11 = Utf8               LTest$;
  #12 = NameAndType        #10:#11        // MODULE$:LTest$;
  #13 = Fieldref           #9.#12         // Test$.MODULE$:LTest$;
  #14 = NameAndType        #6:#7          // f:()V
  #15 = Methodref          #9.#14         // Test$.f:()V
  #16 = Utf8               foo
  #17 = Utf8               ()Ljava/lang/String;
  #18 = NameAndType        #16:#17        // foo:()Ljava/lang/String;
  #19 = Methodref          #9.#18         // Test$.foo:()Ljava/lang/String;
  #20 = Utf8               main
  #21 = Utf8               ([Ljava/lang/String;)V
  #22 = NameAndType        #20:#21        // main:([Ljava/lang/String;)V
  #23 = Methodref          #9.#22         // Test$.main:([Ljava/lang/String;)V
  #24 = Utf8               Code
  #25 = Utf8               SourceFile
  #26 = Utf8               ScalaSig
{
  public static void f();
    descriptor: ()V
    flags: ACC_PUBLIC, ACC_STATIC
    Code:
      stack=1, locals=0, args_size=0
         0: getstatic     #13                 // Field Test$.MODULE$:LTest$;
         3: invokevirtual #15                 // Method Test$.f:()V
         6: return

  public static java.lang.String foo();
    descriptor: ()Ljava/lang/String;
    flags: ACC_PUBLIC, ACC_STATIC
    Code:
      stack=1, locals=0, args_size=0
         0: getstatic     #13                 // Field Test$.MODULE$:LTest$;
         3: invokevirtual #19                 // Method Test$.foo:()Ljava/lang/String;
         6: areturn

  public static void main(java.lang.String[]);
    descriptor: ([Ljava/lang/String;)V
    flags: ACC_PUBLIC, ACC_STATIC
    Code:
      stack=2, locals=1, args_size=1
         0: getstatic     #13                 // Field Test$.MODULE$:LTest$;
         3: aload_0
         4: invokevirtual #23                 // Method Test$.main:([Ljava/lang/String;)V
         7: return
}
SourceFile: "t261.scala"
Error: unknown attribute
  ScalaSig: length = 0x0

@DarkDimius
Copy link
Contributor Author

@odersky could you please run sbt package and try once again?

@odersky
Copy link
Contributor

odersky commented May 27, 2015

@DarkDimius Here's the transcript of what I did:

/Users/odersky/workspace/dotty/tests/run> dotc t261.scala -d /classes
/Users/odersky/workspace/dotty/tests/run> echo $CLASSPATH
/classes:/Users/odersky/workspace/dotty/classes:Users/odersky/workspace/dotty/target/scala-2.11/classes:/Users/odersky/bin/scala-2.11.5/lib/scala-library.jar:/Users/odersky/bin/scala-2.11.5/lib/scala-reflect.jar:/Users/odersky/lib/scala-compiler-2.11.5.jar
/Users/odersky/workspace/dotty/tests/run> java Test
A
B

@DarkDimius
Copy link
Contributor Author

On 2bfbb8e there is a public java.lang.String B$$foo(), on 512bb15 there is no. This is indeed a regression, that I can observe from both command line and partest.

@odersky If you are using the provided dotc script, be aware that it does not rebuild dotty with sbt, if target/scala-2.11/dotty_2.11-0.1-SNAPSHOT.jar is present. We could potentially rebuild it always, but this means that dotc will need to always launch sbt and this will be slow. So if you haven't ran sbt package for a long time, dotc could be using an outdated artifact.

@odersky
Copy link
Contributor

odersky commented May 27, 2015

I now did the following:

  • Rename all files in the test by adding random suffixes.
  • Run the resulting renamed file.

The test succeeds. Which seems to show that partest suffers from classfile pollution, not the local setup.

@DarkDimius
Copy link
Contributor Author

The test succeeds.

can you please run this command in project root and show it's output:
find ./src/ -iname "*.scala" -newer target/scala-2.11/dotty_2.11-0.1-SNAPSHOT.jar

@DarkDimius
Copy link
Contributor Author

@odersky If you are using the provided dotc script, be aware that it does not rebuild dotty with sbt, if target/scala-2.11/dotty_2.11-0.1-SNAPSHOT.jar is present. We could potentially rebuild it always, but this means that dotc will need to always launch sbt and this will be slow. So if you haven't ran sbt package for a long time, dotc could be using an outdated artifact.

Should be improved in #615. Relies on timestamps received from filesystem.

@DarkDimius
Copy link
Contributor Author

@odersky, you could see class files generated by partest in
dotty/tests/partest-generated/run/t261-run.obj/

@odersky
Copy link
Contributor

odersky commented May 28, 2015

Here's a hypothesis: Is it true that the output directory of partest is on the classpath? If yes, then could it be that the failing test is compiled by the compiler compiled by dotc itself, not the one originally built?

Here's a full transcript of what I did. There's no room for error that I can see here. The "2233" suffix was introduced freshly; it is used nowhere else.

/Users/odersky/workspace/dotty/tests/run> echo $CLASSPATH
/classes:/Users/odersky/workspace/dotty/classes:Users/odersky/workspace/dotty/target/scala-2.11/classes:/Users/odersky/bin/scala-2.11.5/lib/scala-library.jar:/Users/odersky/bin/scala-2.11.5/lib/scala-reflect.jar:/Users/odersky/lib/scala-compiler-2.11.5.jar
/Users/odersky/workspace/dotty/tests/run> cat t261.scala                                         
trait A2233 { val foo: String = "A" }
trait B2233 {
   private val foo: String = "B"
   def f = println(foo)
}
object Test2233 extends A2233 with B2233 {
   def main(args: Array[String]) = {
     println(foo)
     f
   }
}
/Users/odersky/workspace/dotty/tests/run> java -Xms1g -Xmx3g dotty.tools.dotc.Main -d /classes t261.scala
/Users/odersky/workspace/dotty/tests/run> java Test2233
A
B

@DarkDimius
Copy link
Contributor Author

On 28 May 2015 09:40:42 odersky [email protected] wrote:

Here's a hypothesis: Is it true that the output directory of partest is on
the classpath? If yes, then could it be that the failing test is compiled
by the compiler compiled by dotc itself, not the one originally built?

Partest uses output directory per test. And it doesn't compile dotty, as
it's not part of run tests. Dotty is only part of junit tests and it's
executed in a different VM.

Dotty compiled by dotty currently dies with NPE during initialization of
ContextBase.

Here's a full transcript of what I did. There's no room for error that I
can see here. The "2233" suffix was introduced freshly; it is used nowhere
else.

/Users/odersky/workspace/dotty/tests/run> echo $CLASSPATH
/classes:/Users/odersky/workspace/dotty/classes:Users/odersky/workspace/dotty/target/scala-2.11/classes:/Users/odersky/bin/scala-2.11.5/lib/scala-library.jar:/Users/odersky/bin/scala-2.11.5/lib/scala-reflect.jar:/Users/odersky/lib/scala-compiler-2.11.5.jar
/Users/odersky/workspace/dotty/tests/run> cat t261.scala
trait A2233 { val foo: String = "A" }
trait B2233 {
private val foo: String = "B"
def f = println(foo)
}
object Test2233 extends A2233 with B2233 {
def main(args: Array[String]) = {
println(foo)
f
}
}
/Users/odersky/workspace/dotty/tests/run> java -Xms1g -Xmx3g
dotty.tools.dotc.Main -d /classes t261.scala
/Users/odersky/workspace/dotty/tests/run> java Test2233
A
B


Reply to this email directly or view it on GitHub:
#611 (comment)

@odersky
Copy link
Contributor

odersky commented May 28, 2015

(Palm slaps face) I was on the wrong git branch when I tested. I could reproduce now with the setup I showed.

Fixes problem with run/t261.scala.
@odersky odersky force-pushed the supercalls-traits-test branch from 512bb15 to 7779b8f Compare May 28, 2015 08:19
@vsalvis
Copy link
Contributor

vsalvis commented May 28, 2015

@smarter showed me how to get the git branch displayed in the bash prompt, it has definitely prevented a lot of confusion since: http://code-worrier.com/blog/git-branch-in-bash-prompt/

@DarkDimius
Copy link
Contributor Author

@vsalvis, you'd better not follow this guide. Because git-promt.sh is frequently updated to match the current state of git, and it could be out-of-sync with your installation(I had non-obvious problems with this at some point). You'd better use the one that is shipped with your installation of git:
if you use macports to install git add to your bashrc this:

if [ -f /opt/local/share/git/contrib/completion/git-prompt.sh ]; then
  . /opt/local/share/git/contrib/completion/git-prompt.sh
  export PS1='\[\033[01;32m\]\u@\h\[\033[01;34m\] \w\[\033[01;33m\]$(__git_ps1)\[\033[01;34m\] \$\[\033[00m\] '
fi

@odersky there's also a git-completion.tcsh there, but it does not define variables for promt. This is how you can do it on your own:

source /opt/local/share/git/contrib/completion/git-completion.tcsh

alias __git_current_branch 'git rev-parse --abbrev-ref HEAD >& /dev/null && echo "{`git rev-parse --abbrev-ref HEAD`}"'
alias precmd 'set prompt="%n@%m[%c2]`__git_current_branch` "'

@DarkDimius
Copy link
Contributor Author

LGTM.

DarkDimius added a commit that referenced this pull request May 28, 2015
@DarkDimius DarkDimius merged commit ef27460 into scala:master May 28, 2015
@allanrenucci allanrenucci deleted the supercalls-traits-test branch December 14, 2017 19:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants