The Inbox: Kernel-ct.1339.mcz

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
11 messages Options
Reply | Threaded
Open this post in threaded view
|

The Inbox: Kernel-ct.1339.mcz

commits-2
A new version of Kernel was added to project The Inbox:
http://source.squeak.org/inbox/Kernel-ct.1339.mcz

==================== Summary ====================

Name: Kernel-ct.1339
Author: ct
Time: 4 September 2020, 9:09:14.713421 pm
UUID: edc35a82-ce03-014c-85de-13d68c7fc46f
Ancestors: Kernel-ct.1338

Implement missing simulation of objects as methods.

In the past, it was not possible to debug/simulate code that used objects as methods properly. (Thanks to Marcel for the hint!) This very simple commit adds support of the OaM protocol [1] to the simulation machinery. Now you can debug all tests in TestObjectsAsMethods as you would expect, instead of crashing your image!

Update: Uploaded again, this time with additional documentation comment, reformatted code, and multilingual support/fix of typös. Replaces Kernel-ct.1306, which can be moved to the treated inbox.

[1] "The [Objects as Methods] contract is that, when the VM encounters an ordinary object (rather than a compiled method) in the method dictionary during lookup, it sends it the special selector #run:with:in: providing the original selector, arguments, and receiver.". DOI: 10.1145/2991041.2991062.

=============== Diff against Kernel-ct.1338 ===============

Item was changed:
  ----- Method: Context>>send:to:with:lookupIn: (in category 'controlling') -----
  send: selector to: rcvr with: arguments lookupIn: lookupClass
+ "Simulate the action of sending a message with selector and arguments to rcvr. The argument, lookupClass, is the class in which to lookup the message. This is the receiver's class for normal messages, but for super messages it will be some specific class related to the source method."
- "Simulate the action of sending a message with selector and arguments
- to rcvr. The argument, lookupClass, is the class in which to lookup the
- message.  This is the receiver's class for normal messages, but for super
- messages it will be some specific class related to the source method."
 
  | meth primIndex val ctxt |
+ (meth := lookupClass lookupSelector: selector) ifNil: [
+ ^ self
+ send: #doesNotUnderstand:
+ to: rcvr
+ with: {(Message selector: selector arguments: arguments) lookupClass: lookupClass}
+ lookupIn: lookupClass].
+
+ meth isCompiledMethod ifFalse: [
+ "Object as Methods (OaM) protocol: 'The contract is that, when the VM encounters an ordinary object (rather than a compiled method) in the method dictionary during lookup, it sends it the special selector #run:with:in: providing the original selector, arguments, and receiver.'. DOI: 10.1145/2991041.2991062."
+ ^ self send: #run:with:in:
+ to: meth
+ with: {selector. arguments. rcvr}].
+
+ meth numArgs ~= arguments size ifTrue: [
+ ^ self error: ('Wrong number of arguments in simulated message {1}' translated format: {selector})].
+ (primIndex := meth primitive) > 0 ifTrue: [
+ val := self doPrimitive: primIndex method: meth receiver: rcvr args: arguments.
+ (self isPrimFailToken: val) ifFalse: [^ val]].
+
+ (selector == #doesNotUnderstand: and: [lookupClass == ProtoObject]) ifTrue: [
+ ^ self error: ('Simulated message {1} not understood' translated format: {arguments first selector})].
+
- (meth := lookupClass lookupSelector: selector) ifNil:
- [^self send: #doesNotUnderstand:
- to: rcvr
- with: {(Message selector: selector arguments: arguments) lookupClass: lookupClass}
- lookupIn: lookupClass].
- meth numArgs ~= arguments size ifTrue:
- [^self error: 'Wrong number of arguments in simulated message ', selector printString].
- (primIndex := meth primitive) > 0 ifTrue:
- [val := self doPrimitive: primIndex method: meth receiver: rcvr args: arguments.
- (self isPrimFailToken: val) ifFalse:
- [^val]].
- (selector == #doesNotUnderstand: and: [lookupClass == ProtoObject]) ifTrue:
- [^self error: 'Simulated message ', arguments first selector, ' not understood'].
  ctxt := Context sender: self receiver: rcvr method: meth arguments: arguments.
+ (primIndex notNil and: [primIndex > 0]) ifTrue: [
+ ctxt failPrimitiveWith: val].
+
+ ^ ctxt!
- primIndex > 0 ifTrue:
- [ctxt failPrimitiveWith: val].
- ^ctxt!



Reply | Threaded
Open this post in threaded view
|

Re: The Inbox: Kernel-ct.1339.mcz

marcel.taeumel
Hi all!

Can we merge this into Trunk?

(Also see a discussion here: http://forum.world.st/Debugger-Simulator-cannot-simulate-some-objects-as-methods-td5123791.html)

Best,
Marcel

Am 04.09.2020 21:09:30 schrieb [hidden email] <[hidden email]>:

A new version of Kernel was added to project The Inbox:
http://source.squeak.org/inbox/Kernel-ct.1339.mcz

==================== Summary ====================

Name: Kernel-ct.1339
Author: ct
Time: 4 September 2020, 9:09:14.713421 pm
UUID: edc35a82-ce03-014c-85de-13d68c7fc46f
Ancestors: Kernel-ct.1338

Implement missing simulation of objects as methods.

In the past, it was not possible to debug/simulate code that used objects as methods properly. (Thanks to Marcel for the hint!) This very simple commit adds support of the OaM protocol [1] to the simulation machinery. Now you can debug all tests in TestObjectsAsMethods as you would expect, instead of crashing your image!

Update: Uploaded again, this time with additional documentation comment, reformatted code, and multilingual support/fix of typös. Replaces Kernel-ct.1306, which can be moved to the treated inbox.

[1] "The [Objects as Methods] contract is that, when the VM encounters an ordinary object (rather than a compiled method) in the method dictionary during lookup, it sends it the special selector #run:with:in: providing the original selector, arguments, and receiver.". DOI: 10.1145/2991041.2991062.

=============== Diff against Kernel-ct.1338 ===============

Item was changed:
----- Method: Context>>send:to:with:lookupIn: (in category 'controlling') -----
send: selector to: rcvr with: arguments lookupIn: lookupClass
+ "Simulate the action of sending a message with selector and arguments to rcvr. The argument, lookupClass, is the class in which to lookup the message. This is the receiver's class for normal messages, but for super messages it will be some specific class related to the source method."
- "Simulate the action of sending a message with selector and arguments
- to rcvr. The argument, lookupClass, is the class in which to lookup the
- message. This is the receiver's class for normal messages, but for super
- messages it will be some specific class related to the source method."

| meth primIndex val ctxt |
+ (meth := lookupClass lookupSelector: selector) ifNil: [
+ ^ self
+ send: #doesNotUnderstand:
+ to: rcvr
+ with: {(Message selector: selector arguments: arguments) lookupClass: lookupClass}
+ lookupIn: lookupClass].
+
+ meth isCompiledMethod ifFalse: [
+ "Object as Methods (OaM) protocol: 'The contract is that, when the VM encounters an ordinary object (rather than a compiled method) in the method dictionary during lookup, it sends it the special selector #run:with:in: providing the original selector, arguments, and receiver.'. DOI: 10.1145/2991041.2991062."
+ ^ self send: #run:with:in:
+ to: meth
+ with: {selector. arguments. rcvr}].
+
+ meth numArgs ~= arguments size ifTrue: [
+ ^ self error: ('Wrong number of arguments in simulated message {1}' translated format: {selector})].
+ (primIndex := meth primitive) > 0 ifTrue: [
+ val := self doPrimitive: primIndex method: meth receiver: rcvr args: arguments.
+ (self isPrimFailToken: val) ifFalse: [^ val]].
+
+ (selector == #doesNotUnderstand: and: [lookupClass == ProtoObject]) ifTrue: [
+ ^ self error: ('Simulated message {1} not understood' translated format: {arguments first selector})].
+
- (meth := lookupClass lookupSelector: selector) ifNil:
- [^self send: #doesNotUnderstand:
- to: rcvr
- with: {(Message selector: selector arguments: arguments) lookupClass: lookupClass}
- lookupIn: lookupClass].
- meth numArgs ~= arguments size ifTrue:
- [^self error: 'Wrong number of arguments in simulated message ', selector printString].
- (primIndex := meth primitive) > 0 ifTrue:
- [val := self doPrimitive: primIndex method: meth receiver: rcvr args: arguments.
- (self isPrimFailToken: val) ifFalse:
- [^val]].
- (selector == #doesNotUnderstand: and: [lookupClass == ProtoObject]) ifTrue:
- [^self error: 'Simulated message ', arguments first selector, ' not understood'].
ctxt := Context sender: self receiver: rcvr method: meth arguments: arguments.
+ (primIndex notNil and: [primIndex > 0]) ifTrue: [
+ ctxt failPrimitiveWith: val].
+
+ ^ ctxt!
- primIndex > 0 ifTrue:
- [ctxt failPrimitiveWith: val].
- ^ctxt!




Reply | Threaded
Open this post in threaded view
|

Re: The Inbox: Kernel-ct.1339.mcz

Eliot Miranda-2
Please can we not refornat this code as C and continue to use Beck-style rectangular blocks?  Please?

_,,,^..^,,,_ (phone)

On Oct 25, 2020, at 11:49 PM, Marcel Taeumel <[hidden email]> wrote:


Hi all!

Can we merge this into Trunk?

(Also see a discussion here: http://forum.world.st/Debugger-Simulator-cannot-simulate-some-objects-as-methods-td5123791.html)

Best,
Marcel

Am 04.09.2020 21:09:30 schrieb [hidden email] <[hidden email]>:

A new version of Kernel was added to project The Inbox:
http://source.squeak.org/inbox/Kernel-ct.1339.mcz

==================== Summary ====================

Name: Kernel-ct.1339
Author: ct
Time: 4 September 2020, 9:09:14.713421 pm
UUID: edc35a82-ce03-014c-85de-13d68c7fc46f
Ancestors: Kernel-ct.1338

Implement missing simulation of objects as methods.

In the past, it was not possible to debug/simulate code that used objects as methods properly. (Thanks to Marcel for the hint!) This very simple commit adds support of the OaM protocol [1] to the simulation machinery. Now you can debug all tests in TestObjectsAsMethods as you would expect, instead of crashing your image!

Update: Uploaded again, this time with additional documentation comment, reformatted code, and multilingual support/fix of typös. Replaces Kernel-ct.1306, which can be moved to the treated inbox.

[1] "The [Objects as Methods] contract is that, when the VM encounters an ordinary object (rather than a compiled method) in the method dictionary during lookup, it sends it the special selector #run:with:in: providing the original selector, arguments, and receiver.". DOI: 10.1145/2991041.2991062.

=============== Diff against Kernel-ct.1338 ===============

Item was changed:
----- Method: Context>>send:to:with:lookupIn: (in category 'controlling') -----
send: selector to: rcvr with: arguments lookupIn: lookupClass
+ "Simulate the action of sending a message with selector and arguments to rcvr. The argument, lookupClass, is the class in which to lookup the message. This is the receiver's class for normal messages, but for super messages it will be some specific class related to the source method."
- "Simulate the action of sending a message with selector and arguments
- to rcvr. The argument, lookupClass, is the class in which to lookup the
- message. This is the receiver's class for normal messages, but for super
- messages it will be some specific class related to the source method."

| meth primIndex val ctxt |
+ (meth := lookupClass lookupSelector: selector) ifNil: [
+ ^ self
+ send: #doesNotUnderstand:
+ to: rcvr
+ with: {(Message selector: selector arguments: arguments) lookupClass: lookupClass}
+ lookupIn: lookupClass].
+
+ meth isCompiledMethod ifFalse: [
+ "Object as Methods (OaM) protocol: 'The contract is that, when the VM encounters an ordinary object (rather than a compiled method) in the method dictionary during lookup, it sends it the special selector #run:with:in: providing the original selector, arguments, and receiver.'. DOI: 10.1145/2991041.2991062."
+ ^ self send: #run:with:in:
+ to: meth
+ with: {selector. arguments. rcvr}].
+
+ meth numArgs ~= arguments size ifTrue: [
+ ^ self error: ('Wrong number of arguments in simulated message {1}' translated format: {selector})].
+ (primIndex := meth primitive) > 0 ifTrue: [
+ val := self doPrimitive: primIndex method: meth receiver: rcvr args: arguments.
+ (self isPrimFailToken: val) ifFalse: [^ val]].
+
+ (selector == #doesNotUnderstand: and: [lookupClass == ProtoObject]) ifTrue: [
+ ^ self error: ('Simulated message {1} not understood' translated format: {arguments first selector})].
+
- (meth := lookupClass lookupSelector: selector) ifNil:
- [^self send: #doesNotUnderstand:
- to: rcvr
- with: {(Message selector: selector arguments: arguments) lookupClass: lookupClass}
- lookupIn: lookupClass].
- meth numArgs ~= arguments size ifTrue:
- [^self error: 'Wrong number of arguments in simulated message ', selector printString].
- (primIndex := meth primitive) > 0 ifTrue:
- [val := self doPrimitive: primIndex method: meth receiver: rcvr args: arguments.
- (self isPrimFailToken: val) ifFalse:
- [^val]].
- (selector == #doesNotUnderstand: and: [lookupClass == ProtoObject]) ifTrue:
- [^self error: 'Simulated message ', arguments first selector, ' not understood'].
ctxt := Context sender: self receiver: rcvr method: meth arguments: arguments.
+ (primIndex notNil and: [primIndex > 0]) ifTrue: [
+ ctxt failPrimitiveWith: val].
+
+ ^ ctxt!
- primIndex > 0 ifTrue:
- [ctxt failPrimitiveWith: val].
- ^ctxt!





Reply | Threaded
Open this post in threaded view
|

Re: The Inbox: Kernel-ct.1339.mcz

marcel.taeumel
Absolutely. Those Squeak modules should be written in a consistent formatting style.

Best,
Marcel

Am 26.10.2020 14:50:00 schrieb Eliot Miranda <[hidden email]>:

Please can we not refornat this code as C and continue to use Beck-style rectangular blocks?  Please?

_,,,^..^,,,_ (phone)

On Oct 25, 2020, at 11:49 PM, Marcel Taeumel <[hidden email]> wrote:


Hi all!

Can we merge this into Trunk?

(Also see a discussion here: http://forum.world.st/Debugger-Simulator-cannot-simulate-some-objects-as-methods-td5123791.html)

Best,
Marcel

Am 04.09.2020 21:09:30 schrieb [hidden email] <[hidden email]>:

A new version of Kernel was added to project The Inbox:
http://source.squeak.org/inbox/Kernel-ct.1339.mcz

==================== Summary ====================

Name: Kernel-ct.1339
Author: ct
Time: 4 September 2020, 9:09:14.713421 pm
UUID: edc35a82-ce03-014c-85de-13d68c7fc46f
Ancestors: Kernel-ct.1338

Implement missing simulation of objects as methods.

In the past, it was not possible to debug/simulate code that used objects as methods properly. (Thanks to Marcel for the hint!) This very simple commit adds support of the OaM protocol [1] to the simulation machinery. Now you can debug all tests in TestObjectsAsMethods as you would expect, instead of crashing your image!

Update: Uploaded again, this time with additional documentation comment, reformatted code, and multilingual support/fix of typös. Replaces Kernel-ct.1306, which can be moved to the treated inbox.

[1] "The [Objects as Methods] contract is that, when the VM encounters an ordinary object (rather than a compiled method) in the method dictionary during lookup, it sends it the special selector #run:with:in: providing the original selector, arguments, and receiver.". DOI: 10.1145/2991041.2991062.

=============== Diff against Kernel-ct.1338 ===============

Item was changed:
----- Method: Context>>send:to:with:lookupIn: (in category 'controlling') -----
send: selector to: rcvr with: arguments lookupIn: lookupClass
+ "Simulate the action of sending a message with selector and arguments to rcvr. The argument, lookupClass, is the class in which to lookup the message. This is the receiver's class for normal messages, but for super messages it will be some specific class related to the source method."
- "Simulate the action of sending a message with selector and arguments
- to rcvr. The argument, lookupClass, is the class in which to lookup the
- message. This is the receiver's class for normal messages, but for super
- messages it will be some specific class related to the source method."

| meth primIndex val ctxt |
+ (meth := lookupClass lookupSelector: selector) ifNil: [
+ ^ self
+ send: #doesNotUnderstand:
+ to: rcvr
+ with: {(Message selector: selector arguments: arguments) lookupClass: lookupClass}
+ lookupIn: lookupClass].
+
+ meth isCompiledMethod ifFalse: [
+ "Object as Methods (OaM) protocol: 'The contract is that, when the VM encounters an ordinary object (rather than a compiled method) in the method dictionary during lookup, it sends it the special selector #run:with:in: providing the original selector, arguments, and receiver.'. DOI: 10.1145/2991041.2991062."
+ ^ self send: #run:with:in:
+ to: meth
+ with: {selector. arguments. rcvr}].
+
+ meth numArgs ~= arguments size ifTrue: [
+ ^ self error: ('Wrong number of arguments in simulated message {1}' translated format: {selector})].
+ (primIndex := meth primitive) > 0 ifTrue: [
+ val := self doPrimitive: primIndex method: meth receiver: rcvr args: arguments.
+ (self isPrimFailToken: val) ifFalse: [^ val]].
+
+ (selector == #doesNotUnderstand: and: [lookupClass == ProtoObject]) ifTrue: [
+ ^ self error: ('Simulated message {1} not understood' translated format: {arguments first selector})].
+
- (meth := lookupClass lookupSelector: selector) ifNil:
- [^self send: #doesNotUnderstand:
- to: rcvr
- with: {(Message selector: selector arguments: arguments) lookupClass: lookupClass}
- lookupIn: lookupClass].
- meth numArgs ~= arguments size ifTrue:
- [^self error: 'Wrong number of arguments in simulated message ', selector printString].
- (primIndex := meth primitive) > 0 ifTrue:
- [val := self doPrimitive: primIndex method: meth receiver: rcvr args: arguments.
- (self isPrimFailToken: val) ifFalse:
- [^val]].
- (selector == #doesNotUnderstand: and: [lookupClass == ProtoObject]) ifTrue:
- [^self error: 'Simulated message ', arguments first selector, ' not understood'].
ctxt := Context sender: self receiver: rcvr method: meth arguments: arguments.
+ (primIndex notNil and: [primIndex > 0]) ifTrue: [
+ ctxt failPrimitiveWith: val].
+
+ ^ ctxt!
- primIndex > 0 ifTrue:
- [ctxt failPrimitiveWith: val].
- ^ctxt!





Reply | Threaded
Open this post in threaded view
|

Re: The Inbox: Kernel-ct.1339.mcz

Eliot Miranda-2
In reply to this post by marcel.taeumel


On Oct 25, 2020, at 11:49 PM, Marcel Taeumel <[hidden email]> wrote:


Hi all!

Can we merge this into Trunk?

(Also see a discussion here: http://forum.world.st/Debugger-Simulator-cannot-simulate-some-objects-as-methods-td5123791.html)

If the formatting is restored I could see what the changes are.  I do not like the use of C style blocks, changing the code I carefully wrote in Beck-style rectangular blocks, for well articulated reasons, being rewritten to use a style that is ugly, vertically wasteful, and in this case unnecessary as it obscures the changes.


Best,
Marcel

Am 04.09.2020 21:09:30 schrieb [hidden email] <[hidden email]>:

A new version of Kernel was added to project The Inbox:
http://source.squeak.org/inbox/Kernel-ct.1339.mcz

==================== Summary ====================

Name: Kernel-ct.1339
Author: ct
Time: 4 September 2020, 9:09:14.713421 pm
UUID: edc35a82-ce03-014c-85de-13d68c7fc46f
Ancestors: Kernel-ct.1338

Implement missing simulation of objects as methods.

In the past, it was not possible to debug/simulate code that used objects as methods properly. (Thanks to Marcel for the hint!) This very simple commit adds support of the OaM protocol [1] to the simulation machinery. Now you can debug all tests in TestObjectsAsMethods as you would expect, instead of crashing your image!

Update: Uploaded again, this time with additional documentation comment, reformatted code, and multilingual support/fix of typös. Replaces Kernel-ct.1306, which can be moved to the treated inbox.

[1] "The [Objects as Methods] contract is that, when the VM encounters an ordinary object (rather than a compiled method) in the method dictionary during lookup, it sends it the special selector #run:with:in: providing the original selector, arguments, and receiver.". DOI: 10.1145/2991041.2991062.

=============== Diff against Kernel-ct.1338 ===============

Item was changed:
----- Method: Context>>send:to:with:lookupIn: (in category 'controlling') -----
send: selector to: rcvr with: arguments lookupIn: lookupClass
+ "Simulate the action of sending a message with selector and arguments to rcvr. The argument, lookupClass, is the class in which to lookup the message. This is the receiver's class for normal messages, but for super messages it will be some specific class related to the source method."
- "Simulate the action of sending a message with selector and arguments
- to rcvr. The argument, lookupClass, is the class in which to lookup the
- message. This is the receiver's class for normal messages, but for super
- messages it will be some specific class related to the source method."

| meth primIndex val ctxt |
+ (meth := lookupClass lookupSelector: selector) ifNil: [
+ ^ self
+ send: #doesNotUnderstand:
+ to: rcvr
+ with: {(Message selector: selector arguments: arguments) lookupClass: lookupClass}
+ lookupIn: lookupClass].
+
+ meth isCompiledMethod ifFalse: [
+ "Object as Methods (OaM) protocol: 'The contract is that, when the VM encounters an ordinary object (rather than a compiled method) in the method dictionary during lookup, it sends it the special selector #run:with:in: providing the original selector, arguments, and receiver.'. DOI: 10.1145/2991041.2991062."
+ ^ self send: #run:with:in:
+ to: meth
+ with: {selector. arguments. rcvr}].
+
+ meth numArgs ~= arguments size ifTrue: [
+ ^ self error: ('Wrong number of arguments in simulated message {1}' translated format: {selector})].
+ (primIndex := meth primitive) > 0 ifTrue: [
+ val := self doPrimitive: primIndex method: meth receiver: rcvr args: arguments.
+ (self isPrimFailToken: val) ifFalse: [^ val]].
+
+ (selector == #doesNotUnderstand: and: [lookupClass == ProtoObject]) ifTrue: [
+ ^ self error: ('Simulated message {1} not understood' translated format: {arguments first selector})].
+
- (meth := lookupClass lookupSelector: selector) ifNil:
- [^self send: #doesNotUnderstand:
- to: rcvr
- with: {(Message selector: selector arguments: arguments) lookupClass: lookupClass}
- lookupIn: lookupClass].
- meth numArgs ~= arguments size ifTrue:
- [^self error: 'Wrong number of arguments in simulated message ', selector printString].
- (primIndex := meth primitive) > 0 ifTrue:
- [val := self doPrimitive: primIndex method: meth receiver: rcvr args: arguments.
- (self isPrimFailToken: val) ifFalse:
- [^val]].
- (selector == #doesNotUnderstand: and: [lookupClass == ProtoObject]) ifTrue:
- [^self error: 'Simulated message ', arguments first selector, ' not understood'].
ctxt := Context sender: self receiver: rcvr method: meth arguments: arguments.
+ (primIndex notNil and: [primIndex > 0]) ifTrue: [
+ ctxt failPrimitiveWith: val].
+
+ ^ ctxt!
- primIndex > 0 ifTrue:
- [ctxt failPrimitiveWith: val].
- ^ctxt!





Reply | Threaded
Open this post in threaded view
|

Re: The Inbox: Kernel-ct.1339.mcz

Christoph Thiede
Done, see Kernel-ct.1357.

Sometimes I believe we should store MethodNodes on Monticello only, or have
the code automatically formatted by the source.squeak.org servers right
during uploading using a pretty printer ... :-)



--
Sent from: http://forum.world.st/Squeak-Dev-f45488.html

Carpe Squeak!
Reply | Threaded
Open this post in threaded view
|

Re: The Inbox: Kernel-ct.1339.mcz

Eliot Miranda-2


On Wed, Oct 28, 2020 at 1:33 PM Christoph Thiede <[hidden email]> wrote:
Done, see Kernel-ct.1357.

Thanks!

Sometimes I believe we should store MethodNodes on Monticello only, or have
the code automatically formatted by the source.squeak.org servers right
during uploading using a pretty printer ... :-)

You're not alone in this thought.  Until comments are part of the grammar it is likely a vain hope however.

But another approach is to read Kent Beck's Smalltalk Best Practice Patterns and see if his arguments on formatting are persuasive.  This is as good a style guide as it gets.  IMO, Smalltalk is not a curly-brace language. [ & ] delimit an object, not some statements.  But many of the original Smalltalkers formatted code as you do so I'm not right.  It's just that I've rewritten lots of the Kernel code and so I'm overly possessive.  I hope you're not offended.

_,,,^..^,,,_
best, Eliot


Reply | Threaded
Open this post in threaded view
|

MethodNodes on Monticello (was: The Inbox: Kernel-ct.1339.mcz)

Christoph Thiede

Hi Eliot,


You're not alone in this thought.  Until comments are part of the grammar it is likely a vain hope however.


Well, I suppose that this would be a quite architectonical change both in Compiler/Parser and on the Monticello/SqueakSource end ... But this could be a bit easier by installing a pretty-printer before committing and using pretty diffs in Monticello only (for certain repositories).


But another approach is to read Kent Beck's Smalltalk Best Practice Patterns and see if his arguments on formatting are persuasive.  This is as good a style guide as it gets.

I read them, and they are not bad at all.

IMO, Smalltalk is not a curly-brace language.

Yep, and I hate to read this:

foo ifTrue: [
    "do something"
] ifFalse: [
    "do something else"
].

Still ...

[ & ] delimit an object, not some statements.

The problem is that when writing the opening bracket on the same line as the first block statement, the block statements will look inconsistently intended:

someBoolean
    ifTrue:
        [self doOneThing.
        self doAnotherThing.
        "Do you notice the visual glitch above?"].

Also, Marcel has argued that it's harder to select the whole block content at once if you have to strip of the brackets before. However, the latter might also be a question of tools.

Best,
Christoph


Von: Squeak-dev <[hidden email]> im Auftrag von Eliot Miranda <[hidden email]>
Gesendet: Freitag, 30. Oktober 2020 19:56 Uhr
An: The general-purpose Squeak developers list
Betreff: Re: [squeak-dev] The Inbox: Kernel-ct.1339.mcz
 


On Wed, Oct 28, 2020 at 1:33 PM Christoph Thiede <[hidden email]> wrote:
Done, see Kernel-ct.1357.

Thanks!

Sometimes I believe we should store MethodNodes on Monticello only, or have
the code automatically formatted by the source.squeak.org servers right
during uploading using a pretty printer ... :-)

You're not alone in this thought.  Until comments are part of the grammar it is likely a vain hope however.

But another approach is to read Kent Beck's Smalltalk Best Practice Patterns and see if his arguments on formatting are persuasive.  This is as good a style guide as it gets.  IMO, Smalltalk is not a curly-brace language. [ & ] delimit an object, not some statements.  But many of the original Smalltalkers formatted code as you do so I'm not right.  It's just that I've rewritten lots of the Kernel code and so I'm overly possessive.  I hope you're not offended.

_,,,^..^,,,_
best, Eliot


Carpe Squeak!
Reply | Threaded
Open this post in threaded view
|

Re: MethodNodes on Monticello (was: The Inbox: Kernel-ct.1339.mcz)

Jakob Reschke
Am Fr., 20. Nov. 2020 um 13:42 Uhr schrieb Thiede, Christoph
<[hidden email]>:

>
> foo ifTrue: [
>     "do something"
> ] ifFalse: [
>     "do something else"
> ].
>
> [...]
>
> Also, Marcel has argued that it's harder to select the whole block content at once if you have to strip of the brackets before. However, the latter might also be a question of tools.
>

If you double-click or press Cmd-space on the inside next to either
bracket, it will select the whole block. I believe this will get more
comfortable only with a projectional editor or something like a larger
hotspot to single-click on where you can currently double-click.

Funnily the need for this feature also goes the other way around: I
had to deal with the C-like style above in a legacy code base (plus
sometimes wrong indentation!) and being able to select the whole block
content at once was the only sane way to find the correct matching
bracket... The problem with this style is that you cannot easily
identify the headline of the block because there are no keywords like
if or else to look out for at the *beginning* of the line. And the
cascades of closing brackets take up too much screen space.

Reply | Threaded
Open this post in threaded view
|

Re: MethodNodes on Monticello (was: The Inbox: Kernel-ct.1339.mcz)

Jakob Reschke
In reply to this post by Christoph Thiede
Am Fr., 20. Nov. 2020 um 13:42 Uhr schrieb Thiede, Christoph
<[hidden email]>:
>
> The problem is that when writing the opening bracket on the same line as the first block statement, the block statements will look inconsistently intended:
>
> someBoolean
>     ifTrue:
>         [self doOneThing.
>         self doAnotherThing.
>         "Do you notice the visual glitch above?"].
>

This used to bug me, too. But eventually I realized that it does not
really matter for my reading comprehension. It's like all those
parentheses bugging the Lisp apprentice in the beginning.

Rectangular block does still have an issue when a complex block is the
receiver of a message, such as with on:do: and ensure:

    [anything
       ifTrue:
          [...]]
       ensure:
          ["do something else"
          that ifTrue: [...]].

Because the lower part of the receiver block is indented, the ensure
could be seen as being inside of it, just at a glance.
But the style that puts the opening bracket on the previous like does
not solve this issue at all.

Reply | Threaded
Open this post in threaded view
|

Re: MethodNodes on Monticello (was: The Inbox: Kernel-ct.1339.mcz)

Eliot Miranda-2


On Fri, Nov 20, 2020 at 6:51 AM Jakob Reschke <[hidden email]> wrote:
Am Fr., 20. Nov. 2020 um 13:42 Uhr schrieb Thiede, Christoph
<[hidden email]>:
>
> The problem is that when writing the opening bracket on the same line as the first block statement, the block statements will look inconsistently intended:
>
> someBoolean
>     ifTrue:
>         [self doOneThing.
>         self doAnotherThing.
>         "Do you notice the visual glitch above?"].
>

This used to bug me, too. But eventually I realized that it does not
really matter for my reading comprehension. It's like all those
parentheses bugging the Lisp apprentice in the beginning.

and I would never indent the ifTrue: unless it has a matching ifFalse:, so the above would be

    someBoolean ifTrue:
        [self doOneThing.
        self doAnotherThing.
        "Do you notice the visual glitch above?"].

and otherwise it would be

    someBoolean
        ifTrue: [self doOneThing]
        ifFalse: [self doTheOther]

etc.

Rectangular block does still have an issue when a complex block is the
receiver of a message, such as with on:do: and ensure:

    [anything
       ifTrue:
          [...]]
       ensure:
          ["do something else"
          that ifTrue: [...]].

Because the lower part of the receiver block is indented, the ensure
could be seen as being inside of it, just at a glance.
But the style that puts the opening bracket on the previous like does
not solve this issue at all.

It does indeed.  I tend to indent the body of the block with two tabs so that it is to the right of the ensure: or on:do:, in those cases.


_,,,^..^,,,_
best, Eliot