abcl icon indicating copy to clipboard operation
abcl copied to clipboard

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

Open alanruttenberg opened this issue 8 years ago • 5 comments

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.

alanruttenberg avatar Oct 06 '17 15:10 alanruttenberg

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.

easye avatar Oct 12 '17 06:10 easye

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" [email protected] 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 .

alessiostalla avatar Oct 12 '17 06:10 alessiostalla

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

  • the name is the name that you'd use to dynamically load the class with, for example, a call to Class.forName with the default ClassLoader.
  • the canonical name is the name that would be used in an import statement and uniquely identifies the class. Might be useful during toString or logging operations.
  • the simple name loosely identifies the class, again might be useful during toString or logging operations but is not guaranteed to be unique.

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 avatar Oct 14 '17 03:10 alanruttenberg

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?

alanruttenberg avatar Oct 14 '17 03:10 alanruttenberg

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?

Ferada avatar Nov 21 '17 12:11 Ferada