SOM-st / SOM

SOM - Simple Object Machine
http://som-st.github.io/
Other
64 stars 12 forks source link

The handler for #unknownGlobal: and #escapedBlock: should be triggered on self #77

Closed smarr closed 3 years ago

smarr commented 3 years ago

This PR adds tests that #unknownGlobal: and #escapedBlock: are triggered on the self of a method, instead of the blockSelf of a block, when they are triggered in nested blocks.

I'd argue this is the more useful semantics of the two seemingly implemented by the different SOM implementations at the moment. Triggering the handlers on the blockSelf means the only way to handle the errors would be with a handler on the Block class, which means, there couldn't be any custom handling local to a class.

When asking self to handle the errors, we can have class-specific handlers, which seems to make more sense from a flexibility perspective.

And, another point in favor: this is the semantics supported by the original SOM (Java) implementation, and likely CSOM (but not verified).

@ltratt could you run this on ykSOM? My CI setup is blocked by the, I believe known, changes around libgc.

/cc @krono just in case

ltratt commented 3 years ago

All good on yksom:

$ cargo run -- --cp SOM/Smalltalk SOM/TestSuite/TestHarness.som

    Finished dev [unoptimized + debuginfo] target(s) in 0.04s
     Running `target/debug/yksom --cp SOM/Smalltalk SOM/TestSuite/TestHarness.som`
Test Suite: #ClassStructureTest
Tests passed: 6
------------------------------
Unsupported optional features: 1
        invokableTypes
                ClassStructureTest>>#testThatCertainMethodsArePrimitives
                        Expected Primitive but was Method.

Test Suite: #SystemTest
Tests passed: 2
------------------------------
Unsupported optional features: 1
        fullGCWithEffect
                SystemTest>>#testFullGCSupport
                        #fullGC is not supported or has not immediate effect.

Total number of tests:           122
Number of unsupported optionals: 2
Number of successful tests:      122
Number of assertions tested:     643
smarr commented 3 years ago

@ltratt Thank you!