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

port library to newly generified classes in java 7 #3634

Closed
scabug opened this issue Jul 2, 2010 · 26 comments
Closed

port library to newly generified classes in java 7 #3634

scabug opened this issue Jul 2, 2010 · 26 comments
Assignees

Comments

@scabug
Copy link

@scabug scabug commented Jul 2, 2010

On the theory java 7 will come out someday, we should have a strategy in mind for dealing with it. As things stand it looks to me like we would have to fork java6 and java7 versions. This is due to the generification of certain classes such as javax.swing.Jlist:

[http://java.sun.com/javase/6/docs/api/javax/swing/JList.html java 6 JList]

[http://download.java.net/jdk7/docs/api/javax/swing/JList.html java 7 JList]

Since #2970 was closed wontfix I take it improving support for raw types is not on the agenda. I spent some time tweaking trunk toward compiling but any change which leads toward success on java 7 invariably leads toward failure on java 6. This may be a bullet we just have to take, but I'm opening this so everyone knows the gun has been fired.

The outcome of compiling trunk scala.swing against java7:

[scalacfork] /scratch/trunk4/src/swing/scala/swing/ComboBox.scala:134: error: type mismatch;
[scalacfork]  found   : AnyRef
[scalacfork]  required: E
[scalacfork]       def getElementAt(n: Int) = items(n).asInstanceOf[AnyRef]
[scalacfork]                                                       ^
[scalacfork] /scratch/trunk4/src/swing/scala/swing/ListView.scala:32: error: trait ListCellRenderer takes type parameters
[scalacfork]     def wrap[A](r: ListCellRenderer): Renderer[A] = new Wrapped[A](r)
[scalacfork]                    ^
[scalacfork] /scratch/trunk4/src/swing/scala/swing/ListView.scala:72: error: trait ListCellRenderer takes type parameters
[scalacfork]     def peer: ListCellRenderer = new ListCellRenderer {
[scalacfork]               ^
[scalacfork] /scratch/trunk4/src/swing/scala/swing/ListView.scala:27: error: class JList takes type parameters
[scalacfork]   def wrap[A](c: JList) = new ListView[A] {
[scalacfork]                  ^
[scalacfork] /scratch/trunk4/src/swing/scala/swing/ListView.scala:145: error: class JList takes type parameters
[scalacfork]   override lazy val peer: JList = new JList with SuperMixin
[scalacfork]                           ^
[scalacfork] /scratch/trunk4/src/swing/scala/swing/ListView.scala:37: error: trait ListCellRenderer takes type parameters
[scalacfork]   	class Wrapped[A](override val peer: ListCellRenderer) extends Renderer[A] {
[scalacfork]                                             ^
[scalacfork] /scratch/trunk4/src/swing/scala/swing/ListView.scala:72: error: trait ListCellRenderer takes type parameters
[scalacfork]     def peer: ListCellRenderer = new ListCellRenderer {
[scalacfork]                                      ^
[scalacfork] /scratch/trunk4/src/swing/scala/swing/ListView.scala:73: error: class JList takes type parameters
[scalacfork]       def getListCellRendererComponent(list: JList, a: Any, index: Int, isSelected: Boolean, focused: Boolean) = 
[scalacfork]                                              ^
[scalacfork] /scratch/trunk4/src/swing/scala/swing/ListView.scala:126: error: trait ListCellRenderer takes type parameters
[scalacfork]     override lazy val peer: ListCellRenderer = new DefaultListCellRenderer
[scalacfork]                             ^
[scalacfork] /scratch/trunk4/src/swing/scala/swing/ListView.scala:153: error: type mismatch;
[scalacfork]  found   : AnyRef
[scalacfork]  required: Nothing
[scalacfork]     def getElementAt(n: Int) = items(n).asInstanceOf[AnyRef]
[scalacfork]                                                     ^
[scalacfork] /scratch/trunk4/src/swing/scala/swing/ListView.scala:195: error: too many arguments for constructor Object: ()java.lang.Object
[scalacfork]     object indices extends Indices(peer.getSelectedIndices) {
[scalacfork]                    ^
[scalacfork] 11 errors found
@scabug
Copy link
Author

@scabug scabug commented Jul 2, 2010

@scabug
Copy link
Author

@scabug scabug commented Jul 13, 2010

@adriaanm said:
as I understand it, the problem is not really with raw types

you're compiling code that was developed against non-generic libraries against the "generified" library
that means the compiler now has more constraints to check: where we had AnyRef before, we now have a type parameter

in order to leverage Java 7, we'd need to port our library so that it compiles against Java 7 libraries, but target Java 6 bytecode when compling

because the generics in the Java 7 libs will erase to something compatible with the Java 6 libs, code compiled against (carefully crafted) Java 7 libraries should run unmodified on both versions 6&7.

note that the compiler does not need to be changed for any of this, it's simply a build process thing (what could possible go wrong)

furthermore, Java 7 is still quite a ways off (let's say 1-2 years), so I don't think this is going to be high priority for the time being

still, it doesn't hurt to give it a shot (if you aim right)

@scabug
Copy link
Author

@scabug scabug commented Dec 6, 2010

@paulp said:
I have an irresolvable issue which I can't allocate to the build process. I'd really like to be able to use java7 for the speed and better tools, but I can't finish porting swing because of this.

% JAVA_HOME=/Library/Java/JavaVirtualMachines/openjdk-1.7-x86_64/Contents/Home scala29
Welcome to Scala version 2.9.0.r23690-b20101205201822 (OpenJDK 64-Bit Server VM, Java 1.7.0-internal).
Type in expressions to have them evaluated.
Type :help for more information.

scala> new javax.swing.DefaultComboBoxModel { }
<console>:6: error: illegal inheritance;
 anonymous class $$anon inherits different type instances of trait ListModel:
javax.swing.ListModel[E] and javax.swing.ListModel[E]
       new javax.swing.DefaultComboBoxModel { }
           ^

This interesting situation arises because javax.swing.ComboBoxModel extends javax.swing.ListModel. But the interface is javax.swing.ListModel, so it swallows up the type parameter in an unrecoverable way. Meanwhile javax.swing.AbstractListModel extends javax.swing.ListModel. In scala.swing this hits in ComboBox:

// the code at present
new AbstractListModel with ComboBoxModel
// what it needs to look like
new AbstractListModel[something] with ComboBoxModel
// what happens, regardless of "something"
//
// [scalacfork]  anonymous class $$anon inherits different type instances of trait ListModel:
// [scalacfork] javax.swing.ListModel and javax.swing.ListModel[AnyRef]
// [scalacfork]     new AbstractListModel[AnyRef] with ComboBoxModel {
// [scalacfork]         ^
//
// [scalacfork]  anonymous class $$anon inherits different type instances of trait ListModel:
// [scalacfork] javax.swing.ListModel and javax.swing.ListModel[Nothing]
// [scalacfork]     new AbstractListModel with ComboBoxModel {
// [scalacfork]         ^

Reclassifying as defect and kicking back to scala reviewer. It's worth the trouble to figure something out now that lets us use java7, even if we don't anticipate supporting it for a long while yet.

@scabug
Copy link
Author

@scabug scabug commented Dec 7, 2010

@odersky said:
Paul, is that "inherits different type instances" the only thing that prevents you from building swing on Java 7? We could probably solve this one with a pretty gross hack (disable the test for existentials of Java types). It would make everything more fragile.
So, if that's the whole price we have to pay for dealing with Swing under Java 7, so be it. But if it's just the first in a long series of disgusting hacks, we should reconsider.

@scabug
Copy link
Author

@scabug scabug commented Dec 7, 2010

@paulp said:

Paul, is that "inherits different type instances" the only thing that prevents you from building swing on Java 7?

I can't yet say more errors don't arise at some later stage, but as far as I know yes, that one error is the entire obstacle.

I'm with you on avoiding a dribble of gross hacks. I will verify that this is the one and only issue and report back.

@scabug
Copy link
Author

@scabug scabug commented Dec 7, 2010

@paulp said:
That was the one and only issue. Not that you wouldn't believe me, but in case you or anyone would like to exercise java7, I put the resulting build up here:

https://github.com/downloads/paulp/scala-full/scala-2.9.0.r23705-java7.tgz

Also, of possibly tremendous interest to OSX users (it was for me anyway) there is a project creating 32-bit and 64-bit OSX openjdk builds. There are installers and they coexist peacefully with the shipped jdks in apple's new /Library/Java/JavaVirtualMachines space. So it lowers the barrier to using openjdk for appropriate tasks down around zero.

http://code.google.com/p/openjdk-osx-build/

@scabug
Copy link
Author

@scabug scabug commented Dec 15, 2010

@ijuma said:
Is there a chance that a patch could be submitted to OpenJDK so that ComboBoxModel does not swallow the type parameter?

In the context of using Java 7 with Scala, the standard library will be changed to use some of the Java 7 features, which may (or may not) impact Scala:

http://blogs.sun.com/darcy/entry/project_coin_minty_fresh_libraries

@scabug
Copy link
Author

@scabug scabug commented Dec 15, 2010

@adriaanm said:
Paul, (existence of build) implies (existence of scalac patch -- gross or not) ?

Or did you skip the offending java bits?

@scabug
Copy link
Author

@scabug scabug commented Dec 15, 2010

@paulp said:
Here is the patch.

https://github.com/scala/scala/tree/openjdk

It's all legit except for the issue already outlined in this ticket. For that one I brought out good old Mr. Null and his bag of nullities.

+  // XXX Returning null since there's not yet a workaround for SI-3634.
+  def newConstantModel[A](items: Seq[A]): ComboBoxModel = { null
@scabug
Copy link
Author

@scabug scabug commented Dec 15, 2010

@paulp said:
Replying to [comment:16 extempore]:

Here is the patch.

...and to disentangle these not quite intersecting thoughts, ijuma is presumably talking about submitting a java patch. I'm not your man for that. My world starts where their world ends.

@scabug
Copy link
Author

@scabug scabug commented Dec 15, 2010

@adriaanm said:

It's all legit except for the issue already outlined in this ticket. For that one I brought out good old Mr. Null and his bag of nullities.
{code}

  • // XXX Returning null since there's not yet a workaround for #3634.
  • def newConstantModel[A](items: Seq[A]): ComboBoxModel = { null
    }}
    ok, so as I understand it, you've worked around the issue by removing some code from our swing library -- I'll put a scalac-level workaround on my list of things to do before the year ends, so that we can compile the library in all its unchanged glory against openjdk7 (and by we I mean you)
@scabug
Copy link
Author

@scabug scabug commented Dec 15, 2010

@paulp said:
Replying to [comment:18 moors]:

(and by we I mean you)

Wait, we means something other than that sometimes? I'm having the sinking feeling I should have looked that word up a long time ago.

@scabug
Copy link
Author

@scabug scabug commented Jan 17, 2011

@ijuma said:
Steve McJones posted about this in the jdk7-dev mailing list:

http://mail.openjdk.java.net/pipermail/jdk7-dev/2011-January/001813.html

@scabug
Copy link
Author

@scabug scabug commented Mar 6, 2011

@ijuma said:
After a bit of prodding, we got a reply about this. Two relevant links:

https://bugs.openjdk.java.net/attachment.cgi?id=191&action=diff
https://bugs.openjdk.java.net/show_bug.cgi?id=100153

I have asked if there is a plan to fix this before the release of Java 7. I'll update this issue if/when there's a reply.

@scabug
Copy link
Author

@scabug scabug commented Mar 7, 2011

@soc said:
ijuma: Do you have a link to the reply/your question?

@scabug
Copy link
Author

@scabug scabug commented Mar 7, 2011

@ijuma said:
It's just a continuation of the thread in OpenJDK started by Steve that I linked to above. Anyway:

http://mail.openjdk.java.net/pipermail/jdk7-dev/2011-March/001947.html

@scabug
Copy link
Author

@scabug scabug commented Apr 7, 2011

@soc said:
I filed a bug in Oracle's bugtracker:

http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7029316

I hope this helps ...

@scabug
Copy link
Author

@scabug scabug commented May 19, 2011

@ijuma said:
I think this is now fixed, but not yet available in a binary snapshot. You can verify the code changes here:

http://hg.openjdk.java.net/jdk7/jdk7/jdk/rev/ea6bd2607399

A new binary snapshot should be out this week and then we should try to build Scala with it.

@scabug
Copy link
Author

@scabug scabug commented May 19, 2011

@soc said:
Great to see! Seems like their bugtracker works better than expected. :-)

@scabug
Copy link
Author

@scabug scabug commented May 19, 2011

@ijuma said:
This was part of the original plan according to the reply I got in the mailing list from Pavel Porvatov.

@scabug
Copy link
Author

@scabug scabug commented May 21, 2011

@ijuma said:
So, the new build is now available from http://jdk7.java.net/download.html . I tried compiling Scala using it, but it failed since scala.swing needs to be ported. Paul, you don't have your changes in a public branch, do you?

@scabug
Copy link
Author

@scabug scabug commented May 21, 2011

@scabug
Copy link
Author

@scabug scabug commented May 21, 2011

@paulp said:
Swing builds now. The openjdk github branch is updated.

@scabug
Copy link
Author

@scabug scabug commented May 21, 2011

@paulp said:
Now all the tests pass too.

@scabug
Copy link
Author

@scabug scabug commented May 21, 2011

@paulp said:
I think our work here is done.

@scabug scabug closed this May 21, 2011
@scabug
Copy link
Author

@scabug scabug commented Aug 18, 2011

@soc said:
Adding some links to Github, so that I don't have to search for answering the same question again and again:

The changes necessary to compile scala.swing with Java 7 are here:

scala/scala@258f2bc

scala/scala@743f88d

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.