[squeak-dev] The Inbox: ShoutCore-ct.73.mcz

Levente Uzonyi leves at caesar.elte.hu
Tue Oct 1 11:14:33 UTC 2019


I don't think this is correct. It's clearly an improvement, but still 
not perfect.
Even in your test case, you wrote #methodArg instead of #blockArg, and I 
suppose those tests pass. But those variables are block arguments.
This is because you add all the contexts' variables to the top level,
which is supposed to hold the surrounding method's variables.
In my opinion, the context's block structure should be injected into 
arguments and temporaries as they are written in that context.

Also, please use blocks instead of symbols for arguments of enumeration 
methods (e.g. #reject:, #gather:). Using symbols is fine in scripts. They 
might be okay in tests, but I don't think it's appropriate to use that 
feature in Trunk code, no matter how comfortable it is. (I agree that the 
code may seem more legible when you're used to it, but what about those 
who are not?)

Levente

On Sat, 28 Sep 2019, commits at source.squeak.org wrote:

> A new version of ShoutCore was added to project The Inbox:
> http://source.squeak.org/inbox/ShoutCore-ct.73.mcz
>
> ==================== Summary ====================
>
> Name: ShoutCore-ct.73
> Author: ct
> Time: 28 September 2019, 11:31:09.24777 pm
> UUID: 27e05259-bafa-0749-827d-b54de84bb8d2
> Ancestors: ShoutCore-ct.72
>
> Fixes a bug in context-dependent styling, as described in [1].
>
> Until proved otherwise, we cannot distinguish between args and tempvars via DebuggerMap, so shout-reparse the original context source up to the place where the provided context starts and access the active variables from the nested shout parser.
>
> (Minimal example to reproduce the old bug: Do it:
> [:x| [:z | (x*z) halt] value: 7] value: 6
> And style [z] in the context of #halt - it was wrongly styled as a tempvar before.)
>
> [1] http://forum.world.st/Distinguishing-temp-vars-and-arguments-via-DebuggerMap-td5104614.html
>
> =============== Diff against ShoutCore-ct.72 ===============
>
> Item was added:
> + ----- Method: SHParserST80>>activeArguments (in category 'accessing') -----
> + activeArguments
> + 	"Parsed arguments that are in the active scope"
> + 	^ arguments!
>
> Item was added:
> + ----- Method: SHParserST80>>activeTemporaries (in category 'accessing') -----
> + activeTemporaries
> + 	"Parsed temporaries that are in the active scope"
> + 	^ temporaries!
>
> Item was changed:
>  ----- Method: SHParserST80>>initializeInstanceVariables (in category 'parse support') -----
>  initializeInstanceVariables
>
>  	instanceVariables := classOrMetaClass
>  		ifNil: [ #() ]
>  		ifNotNil: [ classOrMetaClass allInstVarNames asArray ].
>  	allowUnderscoreAssignments := Scanner allowUnderscoreAsAssignment.
>  	allowUnderscoreSelectors := Scanner prefAllowUnderscoreSelectors.
>  	allowBlockArgumentAssignment := Scanner allowBlockArgumentAssignment.
>  	sourcePosition := 1.
>  	arguments
>  		ifNil: [ arguments := OrderedCollection with: nil ]
>  		ifNotNil: [ arguments reset; addLast: nil ].
>  	temporaries
>  		ifNil: [ temporaries := OrderedCollection with: nil ]
>  		ifNotNil: [ temporaries reset; addLast: nil ].
> + 	context ifNotNil: [ self initializeVariablesFromContext ].
> - 	context ifNotNil: [
> - 		| contextArgumentCount contextVariableNames | 
> - 		contextArgumentCount := context numArgs.
> - 		contextVariableNames := context tempNames asOrderedCollection.
> - 		contextArgumentCount > 0 ifTrue: [
> - 			arguments at: 1 put: (contextVariableNames first: contextArgumentCount) ].
> - 		contextArgumentCount < contextVariableNames size ifTrue: [
> - 			temporaries at: 1 put: (contextVariableNames allButFirst: contextArgumentCount) ] ].
>  	bracketDepth := 0.
>  	ranges
>  		ifNil: [ ranges := OrderedCollection new: 40 "Covers over 80% of all methods." ]
>  		ifNotNil: [ ranges reset ]!
>
> Item was added:
> + ----- Method: SHParserST80>>initializeVariablesFromContext (in category 'parse support') -----
> + initializeVariablesFromContext
> + 
> + 	| contextSourcePcIndex contextSourceParser |
> + 	contextSourcePcIndex := (context debuggerMap
> + 		rangeForPC: context pc
> + 		in: context method
> + 		contextIsActiveContext: true "little white lie to work in every situation")
> + 			start.
> + 	contextSourceParser := self class new
> + 		classOrMetaClass: context method methodClass;
> + 		environment: self environment;
> + 		source: (context method getSource first: contextSourcePcIndex);
> + 		yourself.
> + 	contextSourceParser parse.
> + 	arguments at: 1 put: ((contextSourceParser activeArguments
> + 		reject: #isNil) gather: #yourself) asOrderedCollection.
> + 	temporaries at: 1 put: ((contextSourceParser activeTemporaries 
> + 		reject: #isNil) gather: #yourself) asOrderedCollection.!


More information about the Squeak-dev mailing list