Skip to content

Conversation

@alexander-myltsev
Copy link
Contributor

Closes #733

@alexander-myltsev
Copy link
Contributor Author

@DarkDimius @smarter, please check if a test covers expected behaviour.

@DarkDimius
Copy link
Contributor

This PR does not currently have an implementation, only a test that fails due to non-implemented #733. Did you forget to push something?

@alexander-myltsev
Copy link
Contributor Author

It does not have yet. Is the test correct?

@DarkDimius
Copy link
Contributor

All names but name of the anonymous lambdas (Test$$anonfun$1) are correct.
For lambdas: I guess you would get something like Test$$lambda$1/XXXXX, where XXXX is a number of origin that I am unaware of.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd add an empty string to the output here, to have output structured.

@alexander-myltsev
Copy link
Contributor Author

There are compile-time warnings:

dotty/out$ ../bin/dotc ../tests/run/getclass.scala
warning: encountered F-bounded higher-kinded type parameters for type scala$collection$generic$GenMapFactory$$CC; assuming they are invariant
warning: encountered F-bounded higher-kinded type parameters for type scala$collection$generic$MapFactory$$CC; assuming they are invariant
warning: encountered F-bounded higher-kinded type parameters for type scala$collection$generic$ImmutableMapFactory$$CC; assuming they are invariant
three warnings found

And runtime errors as follows:

dotty/out$ java -cp .:/home/vagrant/.ivy2/cache/org.scala-lang/scala-library/jars/scala-library-2.11.5.jar:/home/vagrant/.ivy2/cache/org.scala-lang/scala-reflect/jars/scala-reflect-2.11.5.jar:/home/vagrant/dotty/target/scala-2.11/dotty_2.11-0.1-SNAPSHOT.jar Test
Exception in thread "main" java.lang.BootstrapMethodError: java.lang.NoClassDefFoundError: scala/compat/java8/JProcedure2
        at Test$.main(getclass.scala:9)
        at Test.main(getclass.scala)
Caused by: java.lang.NoClassDefFoundError: scala/compat/java8/JProcedure2
        ... 2 more
Caused by: java.lang.ClassNotFoundException: scala.compat.java8.JProcedure2
        at java.net.URLClassLoader.findClass(URLClassLoader.java:381)
        at java.lang.ClassLoader.loadClass(ClassLoader.java:424)
        at sun.misc.Launcher$AppClassLoader.loadClass(Launcher.java:331)
        at java.lang.ClassLoader.loadClass(ClassLoader.java:357)
        ... 2 more

What should be fixed?

Everything else passes.

@DarkDimius
Copy link
Contributor

There are compile-time warnings:

Those are known warnings. Ignore them.

NoClassDefFoundError: scala/compat/java8/JProcedure2

That is strange. This is a class defined in dotty/src/scala/compat/java8/JFunction2.java, and it should be(and on my machine is) part of dotty_2.11-0.1-SNAPSHOT.jar

@alexander-myltsev
Copy link
Contributor Author

Fixed. My CP was wrong.

There is another problem. Seems like Array(List("1")) would have type of Object after erasure. So, Array(List("1")).getClass would be class java.lang.Object instead of class [Lscala.collection.immutable.List;.

@DarkDimius
Copy link
Contributor

Seems like Array(List("1")) would have type of Object after erasure

AFAIK it should not. Though you should not handle arrays at all: classes that extend AnyRef have a runtime .getClass that should be used.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just import tpd._

@smarter
Copy link
Member

smarter commented Jul 24, 2015

There is code in InterceptedMethods to deal with getClass: https://github.com/lampepfl/dotty/blob/cf17b73cc02daf514f6d5499fd7a5562fc93349e/src/dotty/tools/dotc/transform/InterceptedMethods.scala#L116-L125
It'd be good to at least avoid duplication (for example by moving primitiveGetClassMethods inside Definitions.scala and using it in both GetClass and InterceptedMethods) but it might make sense to simply move this code from InterceptedMethods to GetClass, so that all the code that deals with getClass is inside GetClass. @DarkDimius : what do you think?

@DarkDimius
Copy link
Contributor

but it might make sense to simply move this code from InterceptedMethods to GetClass.

This code is left as it tries to cover non-trivial bug. Though I've checked now and the way how this was fixed in scalac does not work in dotty: 5.asInstanceOf[Int with AnyRef].getClass is still erased to 5.getClass.

I believe that we need a different fix. The way I would fix it in dotty is to rewrite

5.asInstanceOf[Int with AnyRef].getClass

to

(5.asInstanceOf[Int with AnyRef]: Object).getClass

This should do the trick.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not simply tree?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that works.

@alexander-myltsev alexander-myltsev force-pushed the add-getclass branch 4 times, most recently from 0f4cb1a to 721e840 Compare July 25, 2015 14:50
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you try to put it in next mini-phase block?

@DarkDimius
Copy link
Contributor

Otherwise LGTM

@alexander-myltsev
Copy link
Contributor Author

@DarkDimius , well, worked out. Should I squash both commits?

@DarkDimius
Copy link
Contributor

LGTM

DarkDimius added a commit that referenced this pull request Jul 27, 2015
@DarkDimius DarkDimius merged commit 056e124 into scala:master Jul 27, 2015
@DarkDimius
Copy link
Contributor

@alexander-myltsev, thanks for your contribution!

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