armedbear / abcl

Armed Bear Common Lisp <git+https://github.com/armedbear/abcl/> <--> <svn+https://abcl.org/svn> Bridge
https://abcl.org#rdfs:seeAlso<https://gitlab.common-lisp.net/abcl/abcl>
Other
290 stars 29 forks source link

sys:inspect-parts intended-class sometimes isn't #72

Open alanruttenberg opened 7 years ago

alanruttenberg commented 7 years ago

Consider java.util.HashMap$Node

sys:inspected-parts uses getCanonicalName to get the intended class, which yields java.util.HashMap.Node . However calling jclass on that string gives an error, which causes problems, in e.g. the slime debugger inspect of a java object.

The code that does this is in JavaObject.getParts()

parts = parts.push(new Cons("intendedClass", new SimpleString(intendedClass.getCanonicalName())));

AFAIK, the intended-class business is to handle wrapping of primitive types, i.e. substitute Byte for byte, etc. I see code like findmethod(.. intendedClass ..) which would suggest that the string intendedClass should be a valid argument to jclass.

So I think getName should be used here, vs. getCanonicalName unless I misunderstand the intention of intendedClass

I've worked around it for now by doing this check in the slime inspector code.

easye commented 7 years ago

As far as I remember, the "intendedClass" mechanism memoizes some successful result of runtime "Do what I mean" (DWIM) algorithms. getCannonicalName() should return the current memoized value. Someone should introspect the usages of these Java interfaces to best gauge where best to fix.

alessiostalla commented 7 years ago

IntendedClass is a (probably suboptimal) way to work around some quirky Java accessibility pitfalls without resorting to reflection's setAccessible(true) which is problematic in certain environments and more so in Java 9. (jcall "foo" (jcall "bar" some-obj)) might fail even if "foo" is public on class/interface Foo but bar returns an instance of a non-public subclass/implementation of Foo. That's why, according to bar's method signature, Foo is remembered and used to look methods up rather than the actual class.

Il 12 ott 2017 08:38, "Mark Evenson" notifications@github.com ha scritto:

As far as I remember, the "intendedClass" mechanism memoizes some successful result of runtime "Do what I mean" (DWIM) algorithms. getCannonicalName() should return the current memoized value. Someone should introspect the usages of these Java interfaces to best gauge where best to fix.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/armedbear/abcl/issues/72#issuecomment-336034937, or mute the thread https://github.com/notifications/unsubscribe-auth/AA9dtuil5Wg3i2fhqCUmxUZdVIW0Fxrsks5srbPXgaJpZM4PwrOp .

alanruttenberg commented 7 years ago

@easye getCanonicalName is a jdk-defined function, not one of ours. See this stackoverflow answer.

What I'm saying is that we want to use getName vs getCanonicalName - i.e. something that can be used to look up/load a class. I can't see any situation where getCanonicalName will win over getName. Most of the time it doesn't matter which we use but in the case of a nested class it getCanonicalName does the wrong thing by translating the "$" to a "." making the resulting string not a valid class identifier, at least insofar as it is supposed to be able to identify the nested class.

alanruttenberg commented 7 years ago

BTW, I did look through the code, and as far as I can tell, the intended class stuff is only used to handle the primitive types. But maybe I missed something?

Ferada commented 6 years ago

I can't think why the "parts" interface would use the canonical name, even during inspection I'd rather know it's a nested class! As in, completely agree with @alanruttenberg.

But, the inspector interface isn't for programmatic use (outside of the inspector) anyway, right?