-
Notifications
You must be signed in to change notification settings - Fork 21
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
getClass fails for doubly nested inner-class #2034
Comments
Imported From: https://issues.scala-lang.org/browse/SI-2034?orig=1
|
@phaller said: |
Eric Willigers (ewilligers) said: java.lang.InternalError: Malformed class name
at java.lang.Class.getSimpleName(Unknown Source) like Ticket #286 - getSimpleName expects a $$ between TestScalaBug$$foo$$ and foo2's class name? Could TestScalaBug$$foo$$foo2 instead be named TestScalaBug$$foo$$$$foo2 ? I understand that this conflicts with the goal of having nice names when accessing Scala classes from Java. The alternate approach would be to generate a TestScalaBug$$foo class, with TestScalaBug$$foo$$foo2 being an inner class of that class. But I suspect that approach may have already been considered and rejected in Ticket #1572 |
@odersky said: |
Barry Kaplan (memelet) said: |
Ed Gow (egow) said (edited on Oct 4, 2011 6:34:04 PM UTC): Test case, in the REPL, is class Foo This bug has been around for over two years. I'm really curious whether it's a permanent feature of Scala. Let me explain my predicament. I'm extremely interested in Scala, and I'd like very much to use it. For my first foray, I tried doing some experimenting with the OrientDB document/object database using Scala. The very first time I tried to store an instance of a class created in the Scala REPL I got the getSimpleName() exception. That ended my use of Scala for that project. The function getSimpleName() has been in Java for seven years, and I don't think it's going away. Any java framework, library, etc. is free to use it, and if they don't use it today they just might in their next version. Then any Scala code using that java library is toast. So that is my concern. Java interoperability is a key feature of Scala, but if this bug is any indication, then it's not a very reliable feature. This bug, and others associated with it, have named several java libraries that are broken by the getSimpleName() problem. I understand that it's difficult to fix, but it seems to be such a basic flaw that allowing it to persist is essentially a declaration that Java interop is a secondary concern. Given the current Scala focus on stability of the binary, is this bug likely to ever be fixed, or is the interop flaw permanent? |
@paulp said: |
Matthew Pocock (drdozer) said: |
Sarah Gerweck (gerweck) said: (I apologize that this is a bit of a "me too," but I think this issue is serious enough that it warrants the bump. This, along with the related #6546, are in danger of killing the Scala pilot at my company. I can't even load our code into the same container without breaking the whole deployment.) If binary compatibility is the concern, maybe we can have a compiler flag or something? If a new compiler could support both versions, I could happily throw the switch and not worry about old compilers being able to use my libraries. I might even be willing to write some code here, but I'm not sure that it would actually save any time by the time someone finished pointing me in the right direction as I don't really know the Scala compiler. :-) |
@paulp said: |
Charles Oliver Nutter (headius) said: I do not have a solution for you, but I will mention that JRuby has been using the techniques in John Rose's "symbolic freedom" post for many of our internal classes. It works pretty well...but there are some JVMs that are more strict (than they are supposed to be) that have had issues with some of our method names (J9 that I recall offhand). In any case, we'll probably have to work around this in JRuby as well. I'm not sure I agree that it's a Scala issue, since getSimpleName really shouldn't blow up like this. But the way things are is the way things are. I hope Scala will find an alternative that works with current getSimpleName, and I hope the JDK gets patched to not blow up on unusually-named classes. |
@paulp said: |
Olivier Toupin (oliviertoupin) said (edited by @dragos on Jun 23, 2014 1:20:14 PM UTC): object A {
object B {
class C {
}
}
}
val c = new A.B.C()
c.getClass.getSimpleName() // Throws: java.lang.InternalError: Malformed class name |
@dragos said (edited on Jun 23, 2014 2:08:41 PM UTC):
The issue in this case is the inconsistency between the InnerClasses attribute and the binary name of the inner class. In Olivier's example, the flattened name of C is The code that decides the binary name of C is this: override def name: TermName = {
if (Statistics.hotEnabled) Statistics.incCounter(nameCount)
if (!isMethod && needsFlatClasses) {
if (flatname eq null)
flatname = nme.flattenedName(rawowner.name, rawname)
flatname
}
else rawname
}
} This code does not know about the logic of encoding The whole scheme needs a rehaul. The fact that these names are computed at different times and with completely disconnected logic will bite us in the future too.
An even more ambitious plan would be to rehaul all synthetic name generation. I find the way we generate anonfun names to be unnecessarily verbose. Having a spec would be a great step forward, and justify breaking some tools if the new scheme is sane and well defined. There's material for a SIP here (not a very glamorous one, but a very useful one). |
@paulp said: |
@dragos said: - flatname = nme.flattenedName(rawowner.name, rawname)
+ flatname = nme.flattenedName(rawowner.javaSimpleName,rawname) This seems to only look at the simple name of the owner, while the previous code would recurse through |
@paulp said: // All eight combinations of class/object nesting to three levels.
package s {
object o1 {
object o2 { object o3; class o3 }
class o2 { object o3; class o3 }
}
class o1 {
object o2 { object o3; class o3 }
class o2 { object o3; class o3 }
}
}
// Yes, that's the only way to write these types.
show[s.o1.o2.o3.type]("O-O-O")
show[s.o1.o2.o3]("O-O-C")
show[p.o3.type forSome { val p: s.o1.o2 }]("O-C-O")
show[s.o1.o2#o3]("O-C-C")
show[p.o2.o3.type forSome { val p: s.o1 }]("C-O-O")
show[p.o2.o3 forSome { val p: s.o1 }]("C-O-C")
show[p.o3.type forSome { val p: s.o1#o2 }]("C-C-O")
show[s.o1#o2#o3]("C-C-C")
// Now again, but calling getClass on an instance instead
// of using the type to summon a class object. This should
// print the same eight lines as above.
showRef("O-O-O", s.o1.o2.o3)
showRef("O-O-C", new s.o1.o2.o3)
showRef("O-C-O", (new s.o1.o2).o3)
showRef("O-C-C", { val p = new s.o1.o2 ; new p.o3 })
showRef("C-O-O", (new s.o1).o2.o3)
showRef("C-O-C", { val p = (new s.o1).o2 ; new p.o3 })
showRef("C-C-O", { val p = (new s.o1) ; (new p.o2).o3 })
showRef("C-C-C", { val p = new s.o1 ; val q = new p.o2 ; new q.o3 })
Nesting Outer Simple Full
------- ---------- ------ ----------
O-O-O s.o1$$o2$ o3$ s.o1$$o2$$o3$
O-O-C s.o1$$o2$ o3 s.o1$$o2$$o3
O-C-O s.o1$$o2 o3$ s.o1$$o2$o3$
O-C-C s.o1$$o2 o3 s.o1$$o2$o3
C-O-O s.o1$o2$ o3$ s.o1$o2$$o3$
C-O-C s.o1$o2$ o3 s.o1$o2$$o3
C-C-O s.o1$o2 o3$ s.o1$o2$o3$
C-C-C s.o1$o2 o3 s.o1$o2$o3
O-O-O s.o1$$o2$ o3$ s.o1$$o2$$o3$
O-O-C s.o1$$o2$ o3 s.o1$$o2$$o3
O-C-O s.o1$$o2 o3$ s.o1$$o2$o3$
O-C-C s.o1$$o2 o3 s.o1$$o2$o3
C-O-O s.o1$o2$ o3$ s.o1$o2$$o3$
C-O-C s.o1$o2$ o3 s.o1$o2$$o3
C-C-O s.o1$o2 o3$ s.o1$o2$o3$
C-C-C s.o1$o2 o3 s.o1$o2$o3 |
@paulp said: + def show[T: ClassTag](nesting: String) = showClass(nesting, classTag[T].runtimeClass)
+ def showRef(nesting: String, x: AnyRef) = showClass(nesting, x.getClass)
+ def showClass(nesting: String, c: Class[_]) {
+ print(nesting, c.getEnclosingClass.getName, c.getSimpleName, c.getName)
+ } |
@adriaanm said: |
@paulp said: Maybe you could pinpoint which it is that you think is wrong among these names. Or define what is "inner class metadata" if it's something other than getEnclosingClass.getName + getSimpleName, which is what these names are built from. Nesting Outer Simple Full
------- ---------- ------ ----------
O-O-O s.o1$$o2$ o3$ s.o1$$o2$$o3$
O-O-C s.o1$$o2$ o3 s.o1$$o2$$o3
O-C-O s.o1$$o2 o3$ s.o1$$o2$o3$
O-C-C s.o1$$o2 o3 s.o1$$o2$o3
C-O-O s.o1$o2$ o3$ s.o1$o2$$o3$
C-O-C s.o1$o2$ o3 s.o1$o2$$o3
C-C-O s.o1$o2 o3$ s.o1$o2$o3$
C-C-C s.o1$o2 o3 s.o1$o2$o3
O-O-O s.o1$$o2$ o3$ s.o1$$o2$$o3$
O-O-C s.o1$$o2$ o3 s.o1$$o2$$o3
O-C-O s.o1$$o2 o3$ s.o1$$o2$o3$
O-C-C s.o1$$o2 o3 s.o1$$o2$o3
C-O-O s.o1$o2$ o3$ s.o1$o2$$o3$
C-O-C s.o1$o2$ o3 s.o1$o2$$o3
C-C-O s.o1$o2 o3$ s.o1$o2$o3$
C-C-C s.o1$o2 o3 s.o1$o2$o3 |
@paulp said: Me: no exceptions from java reflection. For this code sample: package s {
object o1 {
object o2 { object o3; class o3 }
class o2 { object o3; class o3 }
}
class o1 {
object o2 { object o3; class o3 }
class o2 { object o3; class o3 }
}
} Me: It compiles and creates the appropriate classfiles for eight distinct entities, each of which has correct and consistent names. scalac ./a.scala
./a.scala:4: error: name clash: class o2 defines object o3
and its companion object o2 also defines class o3
class o2 { object o3; class o3 }
^
./a.scala:4: error: name clash: class o2 defines class o3
and its companion object o2 also defines class o3
class o2 { object o3; class o3 }
^
./a.scala:7: error: name clash: class o1 defines object o2
and its companion object o1 also defines class o2
object o2 { object o3; class o3 }
^
./a.scala:8: error: name clash: class o1 defines class o2
and its companion object o1 also defines class o2
class o2 { object o3; class o3 }
^
./a.scala:8: error: name clash: class o2 defines object o3
and its companion object o2 also defines class o3
class o2 { object o3; class o3 }
^
./a.scala:8: error: name clash: class o2 defines class o3
and its companion object o2 also defines class o3
class o2 { object o3; class o3 }
^
6 errors found It seems like status quo should be armed with more than some nebulous and as-yet unfalsifiable analysis. |
Sarah Gerweck (gerweck) said: |
@paulp said: |
@paulp said: It seems like neither of you observed the addition and usage of rawOwnerChain. You should know that it's demonstrably false that this bug can be fixed by anything involving the metadata. All you have to do is read the source of java.lang.Class. |
@paulp said: Now I'm deleting the branch. |
@paulp said: |
@paulp said: |
The binary name encoding for Scala has been broken for a while. It does not follow the Java source name rules because it does not append a `$` after the mirror class of an object, which already ends in `$`. Zinc works around this fix in the only way it can: it gets the enclosing class, which is obtained from the `InnerClasses` java classfile metadata, and it reconstructs the path from there. It circumvents, by design, the core issue: it doesn't call Java's name parser at all. This issue has been fixed in the JDK9, but unfortunately it's going to be tormenting us for a while because 2.13 does not target Java 9, and we're still at 2.12.x. This is the fix in JDK9: http://bugs.java.com/bugdatabase/view_bug.do?bug_id=8057919 In order to fully fix this issue, the Scala compiler should do the desired behaviour in 2.13.x. 2.12.x is already lost since this fix is binary incompatible. The appropriate JIRA ticket is here: scala/bug#2034.
The binary name encoding for Scala has been broken for a while. It does not follow the Java source name rules because it does not append a `$` after the mirror class of an object, which already ends in `$`. Zinc works around this fix in the only way it can: it gets the enclosing class, which is obtained from the `InnerClasses` java classfile metadata, and it reconstructs the path from there. It circumvents, by design, the core issue: it doesn't call Java's name parser at all. This issue has been fixed in the JDK9, but unfortunately it's going to be tormenting us for a while because 2.13 does not target Java 9, and we're still at 2.12.x. This is the fix in JDK9: http://bugs.java.com/bugdatabase/view_bug.do?bug_id=8057919 In order to fully fix this issue, the Scala compiler should do the desired behaviour in 2.13.x. 2.12.x is already lost since this fix is binary incompatible. The appropriate JIRA ticket is here: scala/bug#2034.
The binary name encoding for Scala has been broken for a while. It does not follow the Java source name rules because it does not append a `$` after the mirror class of an object, which already ends in `$`. Zinc works around this fix in the only way it can: it gets the enclosing class, which is obtained from the `InnerClasses` java classfile metadata, and it reconstructs the path from there. It circumvents, by design, the core issue: it doesn't call Java's name parser at all. This issue has been fixed in the JDK9, but unfortunately it's going to be tormenting us for a while because 2.13 does not target Java 9, and we're still at 2.12.x. This is the fix in JDK9: http://bugs.java.com/bugdatabase/view_bug.do?bug_id=8057919 In order to fully fix this issue, the Scala compiler should do the desired behaviour in 2.13.x. 2.12.x is already lost since this fix is binary incompatible. The appropriate JIRA ticket is here: scala/bug#2034.
The binary name encoding for Scala has been broken for a while. It does not follow the Java source name rules because it does not append a `$` after the mirror class of an object, which already ends in `$`. Zinc works around this fix in the only way it can: it gets the enclosing class, which is obtained from the `InnerClasses` java classfile metadata, and it reconstructs the path from there. It circumvents, by design, the core issue: it doesn't call Java's name parser at all. This issue has been fixed in the JDK9, but unfortunately it's going to be tormenting us for a while because 2.13 does not target Java 9, and we're still at 2.12.x. This is the fix in JDK9: http://bugs.java.com/bugdatabase/view_bug.do?bug_id=8057919 In order to fully fix this issue, the Scala compiler should do the desired behaviour in 2.13.x. 2.12.x is already lost since this fix is binary incompatible. The appropriate JIRA ticket is here: scala/bug#2034.
This comment has been minimized.
This comment has been minimized.
|
Just to point out that the "this isn't critical because it's just pretty printing" argument is false, the implementation of public boolean isAnonymousClass() {
return "".equals(getSimpleName());
} I guess it's fixed in JDK9, but I don't see a rush of people upgrading. |
…c test failures.
…c test failures - the previous commit that addressed this couldn't catch the *error*.
…c test failures.
…c test failures - the previous commit that addressed this couldn't catch the *error*.
Closing this because there is Android and there is "oh is jdk 13 available already? I mean 14?". Perhaps closing will dissuade "pushed a commit that referenced this issue." This issue is no longer "open to Scala" in the sense of actionable under the aegis of this project or "the Scala team", aka "team Scala", aka "team Adriaan", aka "team Flash". |
…eeply-nested objects (related: scala/bug#2034)
Try to get past JMH bug on JDK 8. scala/bug#2034
Avoid scala/bug#2034 when running tests
The following code fails with java.lang.ClassNotFoundException: TestScalaBug$$foo
workaround is to add a class foo
may be related to bug 1572
The text was updated successfully, but these errors were encountered: