The Inbox: Tools-ct.991.mcz

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

The Inbox: Tools-ct.991.mcz

commits-2
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]!


Reply | Threaded
Open this post in threaded view
|

Re: The Inbox: Tools-ct.991.mcz

marcel.taeumel
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 [hidden email] <[hidden email]>:

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]!




Reply | Threaded
Open this post in threaded view
|

Re: The Inbox: Tools-ct.991.mcz

Christoph Thiede

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 <[hidden email]> 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 [hidden email] <[hidden email]>:

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]!




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

Re: The Inbox: Tools-ct.991.mcz

marcel.taeumel
Hi Christoph,

I use a small Vivide script to keep track of your (and other) changes:



Best,
Marcel

Am 01.10.2020 14:08:05 schrieb Thiede, Christoph <[hidden email]>:

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 <[hidden email]> 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 [hidden email] <[hidden email]>:

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]!




Reply | Threaded
Open this post in threaded view
|

Re: The Inbox: Tools-ct.991.mcz

Christoph Thiede

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 <[hidden email]> 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:



Best,
Marcel

Am 01.10.2020 14:08:05 schrieb Thiede, Christoph <[hidden email]>:

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 <[hidden email]> 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 [hidden email] <[hidden email]>:

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]!




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

Re: The Inbox: Tools-ct.991.mcz

marcel.taeumel
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.

Best,
Marcel

Am 01.10.2020 22:34:21 schrieb Thiede, Christoph <[hidden email]>:

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 <[hidden email]> 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:



Best,
Marcel

Am 01.10.2020 14:08:05 schrieb Thiede, Christoph <[hidden email]>:

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 <[hidden email]> 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 [hidden email] <[hidden email]>:

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]!




Reply | Threaded
Open this post in threaded view
|

Re: The Inbox: Tools-ct.991.mcz

Eliot Miranda-2


On Oct 1, 2020, at 11:37 PM, Marcel Taeumel <[hidden email]> 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 <[hidden email]>:

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 <[hidden email]> 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 <[hidden email]>:

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 <[hidden email]> 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 [hidden email] <[hidden email]>:

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>