[squeak-dev] The Inbox: Tools-ct.991.mcz

Eliot Miranda eliot.miranda at gmail.com
Fri Oct 2 13:53:03 UTC 2020



> On Oct 1, 2020, at 11:37 PM, Marcel Taeumel <marcel.taeumel at hpi.de> wrote:
> 
> 
> Hi Christoph,
> 
> > .. and they are still isolated from the discussion on the mailing list ... :-)
> 
> That may be right. Still, the biggest effort lies in the code review itself. It's really easy to look up such a discussion if needed. Sure, you need to re-type that version number into the search function on forum.world.st ... still ... not that hard. ;-)
> 
> You might think that an inbox commit is often not merged because of an open question on the list. Well, I think that's rarely the case. It's typically just time and people. ;-) No need to throw in even more tools into the ring.

+1.  And we’re having wide ranging discussions on a number of topics here on the list right now.  No one is vying for our attention.  No specific workflow or intent constrains the discussion.  We’re free to fork threads to discuss tangents.  And we have an archive to ease searching and provide a record.

I agree that the bug tracking part of GitHub is compelling and we could use it more.  But there is a real danger in embracing GitHub fully, as shown by Pharo.  But I don’t want to spend breath on that right now.  I will say that the VisualWorks team had a great experience designing its own excellent Action Request System.  When I arrived in ‘95 there was a mature system implemented over email that had states (open, in progress, rejected, reviewed, on deck, etc).  The key point was that the system had been designed to support the VW team at ParcPlace and was key in achieving high quality.  IIRC, two engineers, Kish Khemani and Bob Westergaard both had fun and drastically improved the system by giving it a web interface and calling it MARS, for Minimal Action Request System, implemented using the Visual Wave web tools that allowed presenting VW interfaces through the web (an early Seaside).  This was a joy to use, and for a few months it evolved rapidly and then settled down into a solid tool that we knew we could tweak with very little effort.  When we were bought by Cincom we had to use some commercial piece of utter crap, that was miserable.

Now I’m not suggesting we can afford to implement our own.  But I know from experience that you should never allow the tail to wag the dog.  That if you adopt someone else’s tool and it is not best in class, not tailorable with reasonable effort, you will suffer badly.  And the closer a tool is to the core of what you do the worse those effects will be.

Kish, Bob, I wonder what it would take to do a Seaside AR system ;-)

> 
> Best,
> Marcel
>> Am 01.10.2020 22:34:21 schrieb Thiede, Christoph <christoph.thiede at student.hpi.uni-potsdam.de>:
>> 
>> Hi Marcel,
>> 
>> 
>> 
>> this is a very nice script and demonstrates the powerfulness of Vivide very well! Still, it does not show any links between multiple inbox/trunk versions, and they are still isolated from the discussion on the mailing list ... :-)
>> 
>> 
>> 
>> Best,
>> 
>> Christoph
>> 
>> Von: Squeak-dev <squeak-dev-bounces at lists.squeakfoundation.org> im Auftrag von Taeumel, Marcel
>> Gesendet: Donnerstag, 1. Oktober 2020 14:35:16
>> An: squeak-dev
>> Betreff: Re: [squeak-dev] The Inbox: Tools-ct.991.mcz
>>  
>> Hi Christoph,
>> 
>> I use a small Vivide script to keep track of your (and other) changes:
>> 
>> <image.png>
>> 
>> 
>> Best,
>> Marcel
>>> Am 01.10.2020 14:08:05 schrieb Thiede, Christoph <christoph.thiede at student.hpi.uni-potsdam.de>:
>>> 
>>> Hi Marcel,
>>> 
>>> 
>>> 
>>> sorry. As already mentioned, it can be hard to keep an overview of currently 307 open inbox versions and their interdependencies ... 😫
>>> 
>>> 
>>> 
>>> Best,
>>> 
>>> Christoph
>>> 
>>> Von: Squeak-dev <squeak-dev-bounces at lists.squeakfoundation.org> im Auftrag von Taeumel, Marcel
>>> Gesendet: Donnerstag, 1. Oktober 2020 14:04:06
>>> An: squeak-dev
>>> Betreff: Re: [squeak-dev] The Inbox: Tools-ct.991.mcz
>>>  
>>> Hi Christoph,
>>> 
>>> that's funny. You already did that with Tools-ct.957. :-) I will move both 956 and 957 to treated.
>>> 
>>> Best,
>>> Marcel
>>>> Am 30.09.2020 19:46:56 schrieb commits at source.squeak.org <commits at source.squeak.org>:
>>>> 
>>>> Christoph Thiede uploaded a new version of Tools to project The Inbox:
>>>> http://source.squeak.org/inbox/Tools-ct.991.mcz
>>>> 
>>>> ==================== Summary ====================
>>>> 
>>>> Name: Tools-ct.991
>>>> Author: ct
>>>> Time: 30 September 2020, 7:46:39.949807 pm
>>>> UUID: 4d2f75ef-336d-cc4c-aa0d-dd4f7ff99fc7
>>>> Ancestors: Tools-mt.990
>>>> 
>>>> Fixes a bug in DebuggerMethodMap's rangeForPC lookup
>>>> 
>>>> Steps to reproduce:
>>>> 
>>>> c := Object newSubclass.
>>>> c compile: 'foo: foo
>>>> foo = #foo ifTrue: [^ true].
>>>> ^ (foo ifNil: [^ false]) = #bar'.
>>>> c new foo: #bar.
>>>> "Debug it. Step into #foo:, step over #=.
>>>> Before this commit, the selection was '^ true'"
>>>> 
>>>> The error was that #findNearbyBinaryIndex: uses to return the lower possible index if there is no exact match. For debugging, we cannot need this behavior, because we want to select the next operation to be executed.
>>>> 
>>>> Furthermore, this commit refactors some duplication with DebuggerMethodMapForFullBlockCompiledMethod. Please review!
>>>> 
>>>> Uploaded again due to totally broken ancestry. Replaces Tools-ct.956, which can be moved into the treated inbox.
>>>> 
>>>> =============== Diff against Tools-mt.990 ===============
>>>> 
>>>> Item was changed:
>>>> ----- Method: DebuggerMethodMap>>rangeForPC:in:contextIsActiveContext: (in category 'source mapping') -----
>>>> rangeForPC: contextsConcretePC in: method contextIsActiveContext: contextIsActiveContext
>>>> + "Answer the indices in the source code for the supplied pc. If the context is the active context (is at the hot end of the stack) then its pc is the current pc. But if the context isn't, because it is suspended sending a message, then its current pc is the previous pc."
>>>> - "Answer the indices in the source code for the supplied pc.
>>>> - If the context is the actve context (is at the hot end of the stack)
>>>> - then its pc is the current pc. But if the context isn't, because it is
>>>> - suspended sending a message, then its current pc is the previous pc."
>>>> 
>>>> + | pc i end sortedMap |
>>>> - | pc i end |
>>>> pc := method abstractPCForConcretePC: (contextIsActiveContext
>>>> + ifTrue: [contextsConcretePC]
>>>> + ifFalse: [(method pcPreviousTo: contextsConcretePC)
>>>> + ifNil: [contextsConcretePC]]).
>>>> + (self abstractSourceMapForMethod: method)
>>>> + at: pc
>>>> + ifPresent: [:range | ^ range].
>>>> + sortedMap := self sortedSourceMapForMethod: method.
>>>> + sortedMap ifEmpty: [^ 1 to: 0].
>>>> + i := sortedMap
>>>> + findBinaryIndex: [:assoc | pc - assoc key]
>>>> + ifNone: [:lower :upper | upper].
>>>> + i < 1 ifTrue: [^ 1 to: 0].
>>>> + i > sortedMap size ifTrue: [
>>>> + end := sortedMap inject: 0 into: [:prev :this |
>>>> + prev max: this value last].
>>>> + ^ end + 1 to: end].
>>>> + ^ (sortedMap at: i) value!
>>>> - ifTrue: [contextsConcretePC]
>>>> - ifFalse: [(method pcPreviousTo: contextsConcretePC)
>>>> - ifNotNil: [:prevpc| prevpc]
>>>> - ifNil: [contextsConcretePC]]).
>>>> - (self abstractSourceMap includesKey: pc) ifTrue:
>>>> - [^self abstractSourceMap at: pc].
>>>> - sortedSourceMap ifNil:
>>>> - [sortedSourceMap := self abstractSourceMap associations
>>>> - replace: [ :each | each copy ];
>>>> - sort].
>>>> - sortedSourceMap isEmpty ifTrue: [^1 to: 0].
>>>> - i := sortedSourceMap findNearbyBinaryIndex: [:assoc| pc - assoc key].
>>>> - i < 1 ifTrue: [^1 to: 0].
>>>> - i > sortedSourceMap size ifTrue:
>>>> - [end := sortedSourceMap inject: 0 into:
>>>> - [:prev :this | prev max: this value last].
>>>> - ^end+1 to: end].
>>>> - ^(sortedSourceMap at: i) value
>>>> - 
>>>> - "| method source scanner map |
>>>> - method := DebuggerMethodMap compiledMethodAt: #rangeForPC:in:contextIsActiveContext:.
>>>> - source := method getSourceFromFile asString.
>>>> - scanner := InstructionStream on: method.
>>>> - map := method debuggerMap.
>>>> - Array streamContents:
>>>> - [:ranges|
>>>> - [scanner atEnd] whileFalse:
>>>> - [| range |
>>>> - range := map rangeForPC: scanner pc in: method contextIsActiveContext: true.
>>>> - ((map abstractSourceMap includesKey: scanner abstractPC)
>>>> - and: [range first ~= 0]) ifTrue:
>>>> - [ranges nextPut: (source copyFrom: range first to: range last)].
>>>> - scanner interpretNextInstructionFor: InstructionClient new]]"!
>>>> 
>>>> Item was added:
>>>> + ----- Method: DebuggerMethodMap>>sortedSourceMap (in category 'private') -----
>>>> + sortedSourceMap
>>>> + 
>>>> + ^ sortedSourceMap ifNil: [
>>>> + sortedSourceMap := self abstractSourceMap associations
>>>> + replace: [:each | each copy];
>>>> + sort]!
>>>> 
>>>> Item was added:
>>>> + ----- Method: DebuggerMethodMap>>sortedSourceMapForMethod: (in category 'source mapping') -----
>>>> + sortedSourceMapForMethod: aCompiledMethod
>>>> + 
>>>> + ^ self sortedSourceMap!
>>>> 
>>>> Item was changed:
>>>> ----- Method: DebuggerMethodMapForFullBlockCompiledMethods>>abstractSourceMap (in category 'source mapping') -----
>>>> abstractSourceMap
>>>> + 
>>>> + ^ self shouldNotImplement!
>>>> - self shouldNotImplement!
>>>> 
>>>> Item was removed:
>>>> - ----- Method: DebuggerMethodMapForFullBlockCompiledMethods>>rangeForPC:in:contextIsActiveContext: (in category 'source mapping') -----
>>>> - rangeForPC: contextsConcretePC in: method contextIsActiveContext: contextIsActiveContext
>>>> - "Answer the indices in the source code for the supplied pc.
>>>> - If the context is the actve context (is at the hot end of the stack)
>>>> - then its pc is the current pc. But if the context isn't, because it is
>>>> - suspended sending a message, then its current pc is the previous pc."
>>>> - 
>>>> - | pc i end mapForMethod sortedMap |
>>>> - pc := method abstractPCForConcretePC: (contextIsActiveContext
>>>> - ifTrue: [contextsConcretePC]
>>>> - ifFalse: [(method pcPreviousTo: contextsConcretePC)
>>>> - ifNotNil: [:prevpc| prevpc]
>>>> - ifNil: [contextsConcretePC]]).
>>>> - ((mapForMethod := self abstractSourceMapForMethod: method) includesKey: pc) ifTrue:
>>>> - [^mapForMethod at: pc].
>>>> - sortedSourceMap ifNil:
>>>> - [sortedSourceMap := IdentityDictionary new].
>>>> - sortedMap := sortedSourceMap
>>>> - at: method
>>>> - ifAbsentPut: [mapForMethod associations
>>>> - replace: [ :each | each copy ];
>>>> - sort].
>>>> - sortedMap isEmpty ifTrue: [^1 to: 0].
>>>> - i := sortedMap findNearbyBinaryIndex: [:assoc| pc - assoc key].
>>>> - i < 1 ifTrue: [^1 to: 0].
>>>> - i > sortedMap size ifTrue:
>>>> - [end := sortedMap inject: 0 into:
>>>> - [:prev :this | prev max: this value last].
>>>> - ^end+1 to: end].
>>>> - ^(sortedMap at: i) value
>>>> - 
>>>> - "| method source scanner map |
>>>> - method := DebuggerMethodMapForFullBlockCompiledMethods compiledMethodAt: #rangeForPC:in:contextIsActiveContext:.
>>>> - source := method getSourceFromFile asString.
>>>> - scanner := InstructionStream on: method.
>>>> - map := method debuggerMap.
>>>> - Array streamContents:
>>>> - [:ranges|
>>>> - [scanner atEnd] whileFalse:
>>>> - [| range |
>>>> - range := map rangeForPC: scanner pc in: method contextIsActiveContext: true.
>>>> - ((map abstractSourceMap includesKey: scanner abstractPC)
>>>> - and: [range first ~= 0]) ifTrue:
>>>> - [ranges nextPut: (source copyFrom: range first to: range last)].
>>>> - scanner interpretNextInstructionFor: InstructionClient new]]"!
>>>> 
>>>> Item was added:
>>>> + ----- Method: DebuggerMethodMapForFullBlockCompiledMethods>>sortedSourceMap (in category 'source mapping') -----
>>>> + sortedSourceMap
>>>> + 
>>>> + ^ self shouldNotImplement!
>>>> 
>>>> Item was added:
>>>> + ----- Method: DebuggerMethodMapForFullBlockCompiledMethods>>sortedSourceMapForMethod: (in category 'source mapping') -----
>>>> + sortedSourceMapForMethod: method
>>>> + 
>>>> + sortedSourceMap ifNil: [
>>>> + sortedSourceMap := IdentityDictionary new].
>>>> + ^ sortedSourceMap
>>>> + at: method
>>>> + ifAbsentPut: [(self abstractSourceMapForMethod: method) associations
>>>> + replace: [ :each | each copy ];
>>>> + sort]!
>>>> 
>>>> 
> 
> <image.png>
> 
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.squeakfoundation.org/pipermail/squeak-dev/attachments/20201002/da9cdd50/attachment.html>


More information about the Squeak-dev mailing list