PS: For styling, models should always override #hasBindingOf: and #hasBindingThatBeginsWith: together. The latter is currently missing in the BlockClosureInspector, leading to wrong styling for literals that have been typed incompletely. :-)

Best,
Christoph

---
Sent from Squeak Inbox Talk

On 2022-09-20T18:43:05+02:00, christoph.thiede at student.hpi.uni-potsdam.de wrote:

> Hi Marcel,
> 
> the new BlockClosureInspector turns out more and more helpful to me! Could you maybe elaborate further on your choice of configuration for the syntax highlighter? :-)
> 
> First, through #hasBindingOf:, you declared temps and instvars as bindings. In its current form, this has two - IMHO - negative consequences:
> 
> (i) While typing a tempvar into a code pane, I will expect that I can actually compile this expression. However, I can't. Compiler and Shout operate on different data here.
> (ii) To my knowledge, bindings and temps/instvars have opposite semantics. Afaik, Compiler and Shout use the term "binding" in the sense of "literal" only. In the BlockClosureInspector, tempvars/instvars are styled in the wrong color (in SqueakTheme, temps should be gray but not black), and again, lead to false assumptions.
> 
> I see two possible ways to restore consistency:
> 
> (i) Expose all instvars/tempvars as literals indeed via #bindingOf:. As the compiler apparently checks temp vars, inst vars, and bindings in this order, some tempvars such as "receiver" will be unavailable but shadowed by the instvar of the BlockClosure, but "self", "receiver", etc. will still point to the inspected BlockClosure. So, not perfectly consistent either, but already a better trade-off for 90% of all inspectees.
> (ii) Don't trick with literals at all and remove the workspace styling/#hasBindingOf: again.
> 
> 
> Second, do you have an example of literals that are available from a CompiledBlock? (self object method literalsDo: ...)? I'm only aware of CompiledMethod literals that can be produced in a workspace with automatic variable definitions.
> 
> Thanks in advance!
> 
> Best,
> Christoph
> 
> ---
> Sent from Squeak Inbox Talk
> 
> On 2021-12-15T15:52:02+00:00, commits at source.squeak.org wrote:
> 
> > Marcel Taeumel uploaded a new version of Tools to project The Trunk:
> > http://source.squeak.org/trunk/Tools-mt.1087.mcz
> > 
> > ==================== Summary ====================
> > 
> > Name: Tools-mt.1087
> > Author: mt
> > Time: 15 December 2021, 4:52:00.953892 pm
> > UUID: 627887b0-5064-ab43-b7ac-60b329a8b606
> > Ancestors: Tools-mt.1086
> > 
> > Tweak BlockClosureInspector to show all the closured stuff including temps from the outer context and inst-vars from the current receiver. Adds some caching to make it faster. Support cache invalidation only for changing the #receiver field. Not sure whether it makes sense to support changing the outerContext as well...
> > 
> > =============== Diff against Tools-mt.1086 ===============
> > 
> > Item was changed:
> >   Inspector subclass: #BlockClosureInspector
> > + ????instanceVariableNames: 'debuggerMap tempVarNames instVarNames'
> > - ????instanceVariableNames: ''
> >   ????classVariableNames: ''
> >   ????poolDictionaries: ''
> >   ????category: 'Tools-Inspector'!
> > 
> > Item was added:
> > + ----- Method: BlockClosureInspector>>debuggerMap (in category 'private') -----
> > + debuggerMap
> > + 
> > + ????^ debuggerMap ifNil: [debuggerMap := self object method debuggerMap]!
> > 
> > Item was changed:
> >   ----- Method: BlockClosureInspector>>hasBindingOf: (in category 'user interface - styling') -----
> >   hasBindingOf: aString
> >   ????
> >   ????self object method literalsDo: [:literal |
> >   ????????(literal isVariableBinding and: [literal key = aString])
> >   ????????????ifTrue: [^ true]].
> > + ????????
> > + ????(self tempVarNames includes: aString) ifTrue: [^ true].
> > + ????(self instVarNames includes: aString) ifTrue: [^ true].
> > + ????
> > + ????^ false!
> > - ????^ self object outerContext tempNames includes: aString!
> > 
> > Item was added:
> > + ----- Method: BlockClosureInspector>>instVarNames (in category 'private') -----
> > + instVarNames
> > + 
> > + ????^ instVarNames ifNil: [instVarNames := (self object receiver perform: #class "avoid inlining; support proxies") allInstVarNames]!
> > 
> > Item was added:
> > + ----- Method: BlockClosureInspector>>replaceSelectionValue: (in category 'selection') -----
> > + replaceSelectionValue: anObject
> > + 
> > + ????| success |
> > + ????(success := super replaceSelectionValue: anObject) ifTrue: [
> > + ????????self selectedField key = #receiver
> > + ????????????ifTrue: [instVarNames := nil]].
> > + ????^ success!
> > 
> > Item was changed:
> >   ----- Method: BlockClosureInspector>>streamBindingsOn: (in category 'fields - streaming') -----
> >   streamBindingsOn: aStream
> >   ????
> > + ????| compiledCode instNames tempNames |
> > - ????| compiledCode tempNames |
> >   ????self flag: #decompile. "mt: Use #to: and #do: instead of #to:do: to avoid inlining to preserve bindings in enumeration block for later decompilation. See InspectorField."
> > + 
> > + ????(1 to: (instNames := self instVarNames) size) do: [:index |
> > + ????????aStream nextPut: ((self newFieldForType: #instVar key: index)
> > + ????????????????name: ('[[[{1}]]]' format: {instNames at: index});
> > + ????????????????valueGetter: [:closure | closure receiver instVarAt: index];
> > + ????????????????valueSetter: [:closure :value | closure receiver instVarAt: index put: value];
> > + ????????????????yourself)].
> > + ????
> > + ????tempNames := self tempVarNames.
> > + ????"No need to show closured bindings from outerContext. The important temps are already covered in the indexed fields below."
> > + ????(1 to: tempNames size - self object basicSize) do: [:index |
> > + ????????aStream nextPut: ((self newFieldForType: #tempVar key: index)
> > + ????????????????name: ('[[{1}]]' format: {tempNames at: index});
> > + ????????????????valueGetter: [:closure | closure outerContext tempAt: index];
> > + ????????????????valueSetter: [:closure :value | closure outerContext tempAt: index put: value];
> > + ????????????????yourself)].
> > + ????????????
> >   ????(1 to: (compiledCode := self object method) numLiterals) do: [:index |
> >   ????????| literal |
> >   ????????literal := compiledCode literalAt: index.
> >   ????????literal isVariableBinding ifTrue: [ ????????
> >   ????????????aStream nextPut: ((self newFieldForType: #tempVar key: index)
> >   ????????????????name: ('[{1}]' format: {literal key});
> >   ????????????????valueGetter: [:closure | (closure method literalAt: index) value];
> >   ????????????????valueSetter: [:closure :value | (closure method literalAt: index) value: value];
> >   ????????????????yourself)]].
> > + 
> > - ????
> > - ????tempNames := self object outerContext tempNames.
> > - ????"No need to show closured bindings from outerContext. The important temps are already covered in the indexed fields below."
> > - ????"(1 to: tempNames size - self object basicSize) do: [:index |
> > - ????????aStream nextPut: ((self newFieldForType: #tempVar key: index)
> > - ????????????????name: ('[{1}]' format: {tempNames at: index});
> > - ????????????????valueGetter: [:closure | closure outerContext tempAt: index];
> > - ????????????????valueSetter: [:closure :value | closure outerContext tempAt: index put: value];
> > - ????????????????yourself)]."
> > - ????????????
> >   ????(1 to: self object basicSize) do: [:index |
> >   ????????aStream nextPut: ((self newFieldForType: #tempVar key: index)
> >   ????????????????name: ('[{1}]' format: {tempNames at: index + tempNames size - self object basicSize});
> >   ????????????????valueGetter: [:closure | closure basicAt: index];
> >   ????????????????valueSetter: [:closure :value | closure basicAt: index put: value];
> >   ????????????????yourself)].
> >   ????????????
> >   ????????
> >   "See commentary in #doItReceiver. No need to expose the closured 'self' here because there is already the #receiver instVar.
> >   
> >   fieldReceiver
> >   ????^ (self newFieldForType: #tempVar key: #receiver)
> >   ????????name: '[self]';
> >   ????????valueGetter: [:obj | obj receiver];
> >   ????????valueSetter: [:obj :value | obj receiver: value];
> >   ????????yourself
> >   "!
> > 
> > Item was added:
> > + ----- Method: BlockClosureInspector>>tempVarNames (in category 'private') -----
> > + tempVarNames
> > + 
> > + ????^ tempVarNames ifNil: [tempVarNames := self debuggerMap tempNamesForContext: self object outerContext]!
> > 
> > 
> -------------- next part --------------
> An HTML attachment was scrubbed...
> URL: <http://lists.squeakfoundation.org/pipermail/squeak-dev/attachments/20220920/aed2a297/attachment.html>
> 
> 
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.squeakfoundation.org/pipermail/squeak-dev/attachments/20220920/779597a1/attachment.html>