Hi James. I have some questions and comments about the two primitives you recently added, for #canUnderstand: and #includesSelector:.
1. Do these really need to be primitives? I don't know much about the internals (yet), but in other Smalltalks these things are not done with primitives. Behavior>canUnderstand: is like this: canUnderstand: selector "Answer whether the receiver can respond to the message whose selector is the argument. The selector can be in the method dictionary of the receiver's class or any of its superclasses." (self includesSelector: selector) ifTrue: [^true]. superclass == nil ifTrue: [^false]. ^superclass canUnderstand: selector and includesSelector: includesSelector: aSymbol "Answer whether the message whose selector is the argument is in the method dictionary of the receiver's class." ^ self methodDict includesKey: aSymbol where 'methodDict' is an instance variable of Behavior. I assume it is difficult (or impossible?) to provide such a methodDict variable? 2. I wrote a few quick tests for these two new primitives, e.g. self assertTrue: (Integer canUnderstand: #floor) withMessage: 'canUnderstand: failure'. self assertTrue: (Integer includesSelector: #floor) withMessage: 'includesSelector fail'. but these tests fail. Which brings me to a somewhat sensitive topic: 3. Unit tests. I'm frankly concerned that new code is being written that does not include corresponding unit tests. I think if you want to have any hope of redline being successful, everything must be rock solid. Just trying to be constructive... Lee
|
Making a method dictionary directly available to the smalltalk side represents a chicken and egg problem. How do I store methods before I have created the Dictionary into which to store them, hence canUnderstand: is now backed by a primitive. Should canUnderstand: be a primitive or just includesSelector:? I think we could have got away with just includesSelector: I ran all the tests after the change so I am surprised they passed and I posted, given that on another inspection on a different class (Integer canUnderstand: #floor) there was indeed a bug. I have fixed this and pushed. You are right that we need every change backed by a test. I'll put more tests around what I do - however in this case I thought I was writing a fix or support code for an existing test. Thanks for your comments. On Fri, Dec 30, 2011 at 10:27 AM, Lee Breisacher <[hidden email]> wrote: Hi James. I have some questions and comments about the two primitives you recently added, for #canUnderstand: and #includesSelector:. |
> I think we could have got away with just includesSelector:
I'll try it without the canUnderstand prim and see what happens. >
I ran all the tests after the change so I am surprised they passed > I thought I was writing a fix or support code for an existing test. Aha. That's where we have a small disconnect. There are indeed some tests in ObjectTest.st for these "bugs", but those tests are commented out, e.g. " self testRespondsTo. FAILS, see issue #48 " If they were not commented out, then the build would fail! So, what I've been doing after you submit a fix is uncommenting the relevant test, be sure it now passes, then commit the test class. So, I suppose you could look for the commented out tests in these kind of situations. Let me know what works best. Lee |
I should have been more diligent and looked for the commented out cases.
Will do in future. Please let me know if the tests now work for you - I could as well but it is nice to know what is pushed to repo works well for others. Even if you can get away with canUnderstand: without a primitive it might stay as a premature optimisation. On Fri, Dec 30, 2011 at 11:24 AM, Lee Breisacher <[hidden email]> wrote:
|
I pushed an update to ObjectTest.st a few minutes ago (leaving both as primitives).
When I try this non-prim version of canUnderstand: canUnderstand: selector "Answer whether the receiver can respond to the message whose selector is the argument. The selector can be in the method dictionary of the receiver's class or any of its superclasses." (self includesSelector: selector) ifTrue: [^true]. superclass == nil ifTrue: [^false]. ^superclass canUnderstand: selector I get this error: java.lang.VerifyError: (class: st/redline/Behavior$canUnderstand:$B2, method: applyTo signature: (Lst/redline/ProtoObject;Lst/redline/ThisContext;)Lst/redline/ProtoObject;) Unable to pop operand off an empty stack at java.lang.Class.getDeclaredConstructors0(Native Method) at java.lang.Class.privateGetDeclaredConstructors(Class.java:2389) at java.lang.Class.getConstructor0(Class.java:2699) at java.lang.Class.newInstance0(Class.java:326) at java.lang.Class.newInstance(Class.java:308) at st.redline.Primitives.compileBlock(Primitives.java:585) at st.redline.Behavior$canUnderstand:.applyTo(st/redline/Behavior.st:0) at st.redline.Primitives.send(Primitives.java:750) at st.redline.Object$respondsTo:.applyTo(st/redline/Object.st:71) at st.redline.Primitives.send(Primitives.java:750) at st.redline.ObjectTest$testRespondsTo.applyTo(st/redline/ObjectTest.st:45) ... Lee
|
Ill accept the pull, and will look into the block related error a.s.a.p
On Fri, Dec 30, 2011 at 12:15 PM, Lee Breisacher <[hidden email]> wrote: I pushed an update to ObjectTest.st a few minutes ago (leaving both as primitives). |
Fixed block related error.
On Fri, Dec 30, 2011 at 12:31 PM, James Ladd <[hidden email]> wrote: Ill accept the pull, and will look into the block related error a.s.a.p |
Hi James. I'm not sure what you fixed, but the non-primitive version of Behavior>canUnderstand: still fails for me, although it is a different failure now. The VerifyError is gone, but now on this line of Smalltalk::
(self includesSelector: selector) ifTrue: [^true]. I get this
st.redline.RedlineException: st.redline.BlockReturn at st.redline.RedlineException.withCause(RedlineException.java:15) at st.redline.ProtoObject.loadObject(ProtoObject.java:148) at st.redline.ProtoObject.resolveObject0(ProtoObject.java:133) at st.redline.ProtoObject.resolveObject(ProtoObject.java:115) at st.redline.ProtoObject.resolveObject(ProtoObject.java:77) at st.redline.Stic.createClassInstance(Stic.java:73) at st.redline.Stic.invoke(Stic.java:69) at st.redline.Stic.invokeWith(Stic.java:26) at st.redline.StTester.test(StTester.java:11) at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39) at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25) at java.lang.reflect.Method.invoke(Method.java:597) at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:44) at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:15) at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:41) at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:20) at org.junit.runners.BlockJUnit4ClassRunner.runNotIgnored(BlockJUnit4ClassRunner.java:79) at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:71) at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:49) at org.junit.runners.ParentRunner$3.run(ParentRunner.java:193) at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:52) at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:191) at org.junit.runners.ParentRunner.access$000(ParentRunner.java:42) at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:184) at org.junit.runners.ParentRunner.run(ParentRunner.java:236) at org.eclipse.jdt.internal.junit4.runner.JUnit4TestReference.run(JUnit4TestReference.java:50) at org.eclipse.jdt.internal.junit.runner.TestExecution.run(TestExecution.java:38) at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:467) at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:683) at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.run(RemoteTestRunner.java:390) at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.main(RemoteTestRunner.java:197) Caused by: st.redline.BlockReturn at st.redline.Primitives.blockAnswer(Primitives.java:621) at st.redline.Behavior$canUnderstand:$B2.applyTo(st/redline/Behavior.st:32) at st.redline.Primitives.p201(Primitives.java:222) at st.redline.BlockClosure$value.applyTo(st/redline/BlockClosure.st:7) at st.redline.Primitives.send(Primitives.java:738) at st.redline.True$ifTrue:.applyTo(st/redline/True.st:44) at st.redline.Primitives.send(Primitives.java:750) at st.redline.Behavior$canUnderstand:.applyTo(st/redline/Behavior.st:32) at st.redline.Primitives.send(Primitives.java:750) at st.redline.Object$respondsTo:.applyTo(st/redline/Object.st:71) at st.redline.Primitives.send(Primitives.java:750) at st.redline.ObjectTest$testRespondsTo.applyTo(st/redline/ObjectTest.st:45) at st.redline.Primitives.send(Primitives.java:738) at st.redline.ObjectTest$test.applyTo(st/redline/ObjectTest.st:11) at st.redline.Primitives.send(Primitives.java:738) at st.redline.TestSuite$run.applyTo(st/redline/TestSuite.st:9) at st.redline.Primitives.send(Primitives.java:738) at st.redline.TestRunner.construct(st/redline/TestRunner.st:1) at st.redline.TestRunner.<init>(st/redline/TestRunner.st) at sun.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method) at sun.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:39) at sun.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:27) at java.lang.reflect.Constructor.newInstance(Constructor.java:513) at java.lang.Class.newInstance0(Class.java:355) at java.lang.Class.newInstance(Class.java:308) at st.redline.ProtoObject.loadObject(ProtoObject.java:146) ... 30 more |
Yes this is expected and I should have let you know before you proceeded.
The BlockReturn is the return mechanism for a block. What I need to do is work through the examples and other code and work out the most appropriate place to extract the answer from the block and return it. There is also the case where I may need to know if I am inside or outside of a method. Working on this On Sun, Jan 1, 2012 at 1:42 AM, Lee Breisacher <[hidden email]> wrote: Hi James. I'm not sure what you fixed, but the non-primitive version of Behavior>canUnderstand: still fails for me, although it is a different failure now. The VerifyError is gone, but now on this line of Smalltalk:: |
While looking into this I noticed another bug with a superclass being null and needing to return nil.
Fixed and pushed. I get the same result as you now, which is good. Note: I changed the code that refers to superclass to 'self superclass' because superclass is not a variable on the smalltalk side. Working on the block return issue. On Sun, Jan 1, 2012 at 7:47 AM, James Ladd <[hidden email]> wrote: Yes this is expected and I should have let you know before you proceeded. |
Free forum by Nabble | Edit this page |