Marcel Taeumel uploaded a new version of Tests to project The Trunk:
http://source.squeak.org/trunk/Tests-mt.449.mcz ==================== Summary ==================== Name: Tests-mt.449 Author: mt Time: 13 April 2021, 10:51:15.223811 am UUID: 23984a3a-fc1c-2a44-a023-9862b3d2ba8d Ancestors: Tests-nice.448 Simplify #testUnknownSelector. Avoid interfering with garbage collection and symbol tables. (Note that #lookup: is the preferred interface, not #findInterned: or #hasInterned:ifTrue:. The concept of internalization should not leak into client code.) =============== Diff against Tests-nice.448 =============== Item was changed: TestCase subclass: #CompilerExceptionsTest + instanceVariableNames: 'text selectionInterval originalText previousSelection originalSelection' - instanceVariableNames: 'text selectionInterval originalText previousSelection originalSelection tearDowns' classVariableNames: '' poolDictionaries: '' category: 'Tests-Compiler'! Item was added: + ----- Method: CompilerExceptionsTest>>generateUnknownSelector (in category 'private') ----- + generateUnknownSelector + + | selector num | + selector := 'yourself'. + num := 0. + [(Symbol lookup: selector, num) notNil] whileTrue: [num := num + 1]. + ^ selector, num! Item was removed: - ----- Method: CompilerExceptionsTest>>setUp (in category 'running') ----- - setUp - - super setUp. - tearDowns := OrderedCollection new. - Symbol hasInterned: self unknownSelector ifTrue: [:symbol | - tearDowns add: [Symbol intern: symbol]]. - Symbol extern: self unknownSelector.! Item was changed: ----- Method: CompilerExceptionsTest>>tearDown (in category 'running') ----- tearDown self removeGeneratedMethods. - Symbol extern: self unknownSelector. - tearDowns do: #value. super tearDown.! Item was changed: ----- Method: CompilerExceptionsTest>>testUnknownSelector (in category 'tests') ----- testUnknownSelector + | unknownSelector | + self flag: #flaky. "mt: This test depends on a list of suggestions that is derived from the system's known selectors. Any desired replacement might not be present in the actual list. Maybe we could use an index-based choice instead." + unknownSelector := self generateUnknownSelector. self + compiling: 'griffle self ' , unknownSelector - compiling: 'griffle self ' , self unknownSelector shouldRaise: UnknownSelector + andSelect: unknownSelector - andSelect: self unknownSelector testing: { + false -> [ + self assertCanceled. + self assert: (Symbol lookup: unknownSelector) isNil]. + 'yourself' -> [ + self assertSucceeded: 'griffle self yourself'. + self assert: (Symbol lookup: unknownSelector) isNil]. + unknownSelector -> [ - false -> [self assertCanceled]. - 'yourself' -> [self assertSucceeded: 'griffle self yourself']. - self unknownSelector -> [ self assertSucceeded. + self assert: (Symbol lookup: unknownSelector) notNil] }. + unknownSelector := self generateUnknownSelector. - self assert: (Symbol hasInterned: self unknownSelector ifTrue: [:symbol |]). - Symbol extern: self unknownSelector] }. self + compiling: 'griffle [ self ' , unknownSelector , ' ] value' - compiling: 'griffle [ self ' , self unknownSelector , ' ] value' shouldRaise: UnknownSelector + andSelect: unknownSelector - andSelect: self unknownSelector testing: { + false -> [ + self assertCanceled. + self assert: (Symbol lookup: unknownSelector) isNil]. + 'yourself' -> [ + self assertSucceeded: 'griffle [ self yourself ] value'. + self assert: (Symbol lookup: unknownSelector) isNil]. + unknownSelector -> [ - false -> [self assertCanceled]. - 'yourself' -> [self assertSucceeded: 'griffle [ self yourself ] value']. - self unknownSelector -> [ self assertSucceeded. + self assert: (Symbol lookup: unknownSelector) notNil] }.! - self assert: (Symbol hasInterned: self unknownSelector ifTrue: [:symbol |])] }.! Item was removed: - ----- Method: CompilerExceptionsTest>>unknownSelector (in category 'private') ----- - unknownSelector - - ^ 'yourrsellff'! Item was removed: - ----- Method: Symbol class>>extern: (in category '*Tests-release') ----- - extern: aStringOrSymbol - - {NewSymbols. SymbolTable} do: [:table | - table remove: aStringOrSymbol ifAbsent: []].! |
Le mar. 13 avr. 2021 à 10:51, <[hidden email]> a écrit :
> > Marcel Taeumel uploaded a new version of Tests to project The Trunk: > http://source.squeak.org/trunk/Tests-mt.449.mcz > > ==================== Summary ==================== > > Name: Tests-mt.449 > Author: mt > Time: 13 April 2021, 10:51:15.223811 am > UUID: 23984a3a-fc1c-2a44-a023-9862b3d2ba8d > Ancestors: Tests-nice.448 > > Simplify #testUnknownSelector. Avoid interfering with garbage collection and symbol tables. > > (Note that #lookup: is the preferred interface, not #findInterned: or #hasInterned:ifTrue:. The concept of internalization should not leak into client code.) May I suggest to classify them as private then? > > =============== Diff against Tests-nice.448 =============== > > Item was changed: > TestCase subclass: #CompilerExceptionsTest > + instanceVariableNames: 'text selectionInterval originalText previousSelection originalSelection' > - instanceVariableNames: 'text selectionInterval originalText previousSelection originalSelection tearDowns' > classVariableNames: '' > poolDictionaries: '' > category: 'Tests-Compiler'! > > Item was added: > + ----- Method: CompilerExceptionsTest>>generateUnknownSelector (in category 'private') ----- > + generateUnknownSelector > + > + | selector num | > + selector := 'yourself'. > + num := 0. > + [(Symbol lookup: selector, num) notNil] whileTrue: [num := num + 1]. > + ^ selector, num! > > Item was removed: > - ----- Method: CompilerExceptionsTest>>setUp (in category 'running') ----- > - setUp > - > - super setUp. > - tearDowns := OrderedCollection new. > - Symbol hasInterned: self unknownSelector ifTrue: [:symbol | > - tearDowns add: [Symbol intern: symbol]]. > - Symbol extern: self unknownSelector.! > > Item was changed: > ----- Method: CompilerExceptionsTest>>tearDown (in category 'running') ----- > tearDown > > self removeGeneratedMethods. > - Symbol extern: self unknownSelector. > - tearDowns do: #value. > super tearDown.! > > Item was changed: > ----- Method: CompilerExceptionsTest>>testUnknownSelector (in category 'tests') ----- > testUnknownSelector > > + | unknownSelector | > + self flag: #flaky. "mt: This test depends on a list of suggestions that is derived from the system's known selectors. Any desired replacement might not be present in the actual list. Maybe we could use an index-based choice instead." > + unknownSelector := self generateUnknownSelector. > self > + compiling: 'griffle self ' , unknownSelector > - compiling: 'griffle self ' , self unknownSelector > shouldRaise: UnknownSelector > + andSelect: unknownSelector > - andSelect: self unknownSelector > testing: { > + false -> [ > + self assertCanceled. > + self assert: (Symbol lookup: unknownSelector) isNil]. > + 'yourself' -> [ > + self assertSucceeded: 'griffle self yourself'. > + self assert: (Symbol lookup: unknownSelector) isNil]. > + unknownSelector -> [ > - false -> [self assertCanceled]. > - 'yourself' -> [self assertSucceeded: 'griffle self yourself']. > - self unknownSelector -> [ > self assertSucceeded. > + self assert: (Symbol lookup: unknownSelector) notNil] }. > + unknownSelector := self generateUnknownSelector. > - self assert: (Symbol hasInterned: self unknownSelector ifTrue: [:symbol |]). > - Symbol extern: self unknownSelector] }. > self > + compiling: 'griffle [ self ' , unknownSelector , ' ] value' > - compiling: 'griffle [ self ' , self unknownSelector , ' ] value' > shouldRaise: UnknownSelector > + andSelect: unknownSelector > - andSelect: self unknownSelector > testing: { > + false -> [ > + self assertCanceled. > + self assert: (Symbol lookup: unknownSelector) isNil]. > + 'yourself' -> [ > + self assertSucceeded: 'griffle [ self yourself ] value'. > + self assert: (Symbol lookup: unknownSelector) isNil]. > + unknownSelector -> [ > - false -> [self assertCanceled]. > - 'yourself' -> [self assertSucceeded: 'griffle [ self yourself ] value']. > - self unknownSelector -> [ > self assertSucceeded. > + self assert: (Symbol lookup: unknownSelector) notNil] }.! > - self assert: (Symbol hasInterned: self unknownSelector ifTrue: [:symbol |])] }.! > > Item was removed: > - ----- Method: CompilerExceptionsTest>>unknownSelector (in category 'private') ----- > - unknownSelector > - > - ^ 'yourrsellff'! > > Item was removed: > - ----- Method: Symbol class>>extern: (in category '*Tests-release') ----- > - extern: aStringOrSymbol > - > - {NewSymbols. SymbolTable} do: [:table | > - table remove: aStringOrSymbol ifAbsent: []].! > > |
> May I suggest to classify them as private then? Hmm... #hasInterned:ifTrue: is "private" already. ;-) Maybe #findInterned: should be, too. Yet, soft-deprecation via "self flag: #deprecated" might be even better since both are actually implemented through #lookup:. Best, Marcel
|
Hey Eliot, recently, you chose #findInterned: over #lookup:. Can you recall how this happened? What is your opinion on this matter? :-) Here is a remark from Levente about this from July 2019: Best, Marcel
|
In reply to this post by marcel.taeumel
findInterned: could be deprecated because completely equivalent to lookup:.
I removed its usage in core image, but there are other in external packages (refactoring browser for example). Le mar. 13 avr. 2021 à 11:44, Marcel Taeumel <[hidden email]> a écrit : > > > May I suggest to classify them as private then? > > Hmm... #hasInterned:ifTrue: is "private" already. ;-) Maybe #findInterned: should be, too. Yet, soft-deprecation via "self flag: #deprecated" might be even better since both are actually implemented through #lookup:. > > Best, > Marcel > > Am 13.04.2021 11:25:27 schrieb Nicolas Cellier <[hidden email]>: > > Le mar. 13 avr. 2021 à 10:51, a écrit : > > > > Marcel Taeumel uploaded a new version of Tests to project The Trunk: > > http://source.squeak.org/trunk/Tests-mt.449.mcz > > > > ==================== Summary ==================== > > > > Name: Tests-mt.449 > > Author: mt > > Time: 13 April 2021, 10:51:15.223811 am > > UUID: 23984a3a-fc1c-2a44-a023-9862b3d2ba8d > > Ancestors: Tests-nice.448 > > > > Simplify #testUnknownSelector. Avoid interfering with garbage collection and symbol tables. > > > > (Note that #lookup: is the preferred interface, not #findInterned: or #hasInterned:ifTrue:. The concept of internalization should not leak into client code.) > > May I suggest to classify them as private then? > > > > > =============== Diff against Tests-nice.448 =============== > > > > Item was changed: > > TestCase subclass: #CompilerExceptionsTest > > + instanceVariableNames: 'text selectionInterval originalText previousSelection originalSelection' > > - instanceVariableNames: 'text selectionInterval originalText previousSelection originalSelection tearDowns' > > classVariableNames: '' > > poolDictionaries: '' > > category: 'Tests-Compiler'! > > > > Item was added: > > + ----- Method: CompilerExceptionsTest>>generateUnknownSelector (in category 'private') ----- > > + generateUnknownSelector > > + > > + | selector num | > > + selector := 'yourself'. > > + num := 0. > > + [(Symbol lookup: selector, num) notNil] whileTrue: [num := num + 1]. > > + ^ selector, num! > > > > Item was removed: > > - ----- Method: CompilerExceptionsTest>>setUp (in category 'running') ----- > > - setUp > > - > > - super setUp. > > - tearDowns := OrderedCollection new. > > - Symbol hasInterned: self unknownSelector ifTrue: [:symbol | > > - tearDowns add: [Symbol intern: symbol]]. > > - Symbol extern: self unknownSelector.! > > > > Item was changed: > > ----- Method: CompilerExceptionsTest>>tearDown (in category 'running') ----- > > tearDown > > > > self removeGeneratedMethods. > > - Symbol extern: self unknownSelector. > > - tearDowns do: #value. > > super tearDown.! > > > > Item was changed: > > ----- Method: CompilerExceptionsTest>>testUnknownSelector (in category 'tests') ----- > > testUnknownSelector > > > > + | unknownSelector | > > + self flag: #flaky. "mt: This test depends on a list of suggestions that is derived from the system's known selectors. Any desired replacement might not be present in the actual list. Maybe we could use an index-based choice instead." > > + unknownSelector := self generateUnknownSelector. > > self > > + compiling: 'griffle self ' , unknownSelector > > - compiling: 'griffle self ' , self unknownSelector > > shouldRaise: UnknownSelector > > + andSelect: unknownSelector > > - andSelect: self unknownSelector > > testing: { > > + false -> [ > > + self assertCanceled. > > + self assert: (Symbol lookup: unknownSelector) isNil]. > > + 'yourself' -> [ > > + self assertSucceeded: 'griffle self yourself'. > > + self assert: (Symbol lookup: unknownSelector) isNil]. > > + unknownSelector -> [ > > - false -> [self assertCanceled]. > > - 'yourself' -> [self assertSucceeded: 'griffle self yourself']. > > - self unknownSelector -> [ > > self assertSucceeded. > > + self assert: (Symbol lookup: unknownSelector) notNil] }. > > + unknownSelector := self generateUnknownSelector. > > - self assert: (Symbol hasInterned: self unknownSelector ifTrue: [:symbol |]). > > - Symbol extern: self unknownSelector] }. > > self > > + compiling: 'griffle [ self ' , unknownSelector , ' ] value' > > - compiling: 'griffle [ self ' , self unknownSelector , ' ] value' > > shouldRaise: UnknownSelector > > + andSelect: unknownSelector > > - andSelect: self unknownSelector > > testing: { > > + false -> [ > > + self assertCanceled. > > + self assert: (Symbol lookup: unknownSelector) isNil]. > > + 'yourself' -> [ > > + self assertSucceeded: 'griffle [ self yourself ] value'. > > + self assert: (Symbol lookup: unknownSelector) isNil]. > > + unknownSelector -> [ > > - false -> [self assertCanceled]. > > - 'yourself' -> [self assertSucceeded: 'griffle [ self yourself ] value']. > > - self unknownSelector -> [ > > self assertSucceeded. > > + self assert: (Symbol lookup: unknownSelector) notNil] }.! > > - self assert: (Symbol hasInterned: self unknownSelector ifTrue: [:symbol |])] }.! > > > > Item was removed: > > - ----- Method: CompilerExceptionsTest>>unknownSelector (in category 'private') ----- > > - unknownSelector > > - > > - ^ 'yourrsellff'! > > > > Item was removed: > > - ----- Method: Symbol class>>extern: (in category '*Tests-release') ----- > > - extern: aStringOrSymbol > > - > > - {NewSymbols. SymbolTable} do: [:table | > > - table remove: aStringOrSymbol ifAbsent: []].! > > > > > > |
In reply to this post by marcel.taeumel
I think that findInterned: is old, probably from st80 epoch...
hasInterned:ifTrue: also, it may be cross-dialect. Strange that we did put it in private... a massive rewrite of (Symbol hasInterned: aString ifTrue: [:aSymbol | ...]) into ((Symbol lookup: aString) ifNotNil: [:aSymbol | ...]) is possible, except in the rare cases when we use the returned value of the send, which is a Boolean. But it is questionable... Le mar. 13 avr. 2021 à 11:53, Marcel Taeumel <[hidden email]> a écrit : > > Hey Eliot, > > recently, you chose #findInterned: over #lookup:. Can you recall how this happened? What is your opinion on this matter? :-) > > Here is a remark from Levente about this from July 2019: > http://forum.world.st/The-Trunk-Morphic-mt-1491-mcz-tp5101075p5101089.html > > Best, > Marcel > > Am 13.04.2021 11:44:24 schrieb Marcel Taeumel <[hidden email]>: > > > May I suggest to classify them as private then? > > Hmm... #hasInterned:ifTrue: is "private" already. ;-) Maybe #findInterned: should be, too. Yet, soft-deprecation via "self flag: #deprecated" might be even better since both are actually implemented through #lookup:. > > Best, > Marcel > > Am 13.04.2021 11:25:27 schrieb Nicolas Cellier <[hidden email]>: > > Le mar. 13 avr. 2021 à 10:51, a écrit : > > > > Marcel Taeumel uploaded a new version of Tests to project The Trunk: > > http://source.squeak.org/trunk/Tests-mt.449.mcz > > > > ==================== Summary ==================== > > > > Name: Tests-mt.449 > > Author: mt > > Time: 13 April 2021, 10:51:15.223811 am > > UUID: 23984a3a-fc1c-2a44-a023-9862b3d2ba8d > > Ancestors: Tests-nice.448 > > > > Simplify #testUnknownSelector. Avoid interfering with garbage collection and symbol tables. > > > > (Note that #lookup: is the preferred interface, not #findInterned: or #hasInterned:ifTrue:. The concept of internalization should not leak into client code.) > > May I suggest to classify them as private then? > > > > > =============== Diff against Tests-nice.448 =============== > > > > Item was changed: > > TestCase subclass: #CompilerExceptionsTest > > + instanceVariableNames: 'text selectionInterval originalText previousSelection originalSelection' > > - instanceVariableNames: 'text selectionInterval originalText previousSelection originalSelection tearDowns' > > classVariableNames: '' > > poolDictionaries: '' > > category: 'Tests-Compiler'! > > > > Item was added: > > + ----- Method: CompilerExceptionsTest>>generateUnknownSelector (in category 'private') ----- > > + generateUnknownSelector > > + > > + | selector num | > > + selector := 'yourself'. > > + num := 0. > > + [(Symbol lookup: selector, num) notNil] whileTrue: [num := num + 1]. > > + ^ selector, num! > > > > Item was removed: > > - ----- Method: CompilerExceptionsTest>>setUp (in category 'running') ----- > > - setUp > > - > > - super setUp. > > - tearDowns := OrderedCollection new. > > - Symbol hasInterned: self unknownSelector ifTrue: [:symbol | > > - tearDowns add: [Symbol intern: symbol]]. > > - Symbol extern: self unknownSelector.! > > > > Item was changed: > > ----- Method: CompilerExceptionsTest>>tearDown (in category 'running') ----- > > tearDown > > > > self removeGeneratedMethods. > > - Symbol extern: self unknownSelector. > > - tearDowns do: #value. > > super tearDown.! > > > > Item was changed: > > ----- Method: CompilerExceptionsTest>>testUnknownSelector (in category 'tests') ----- > > testUnknownSelector > > > > + | unknownSelector | > > + self flag: #flaky. "mt: This test depends on a list of suggestions that is derived from the system's known selectors. Any desired replacement might not be present in the actual list. Maybe we could use an index-based choice instead." > > + unknownSelector := self generateUnknownSelector. > > self > > + compiling: 'griffle self ' , unknownSelector > > - compiling: 'griffle self ' , self unknownSelector > > shouldRaise: UnknownSelector > > + andSelect: unknownSelector > > - andSelect: self unknownSelector > > testing: { > > + false -> [ > > + self assertCanceled. > > + self assert: (Symbol lookup: unknownSelector) isNil]. > > + 'yourself' -> [ > > + self assertSucceeded: 'griffle self yourself'. > > + self assert: (Symbol lookup: unknownSelector) isNil]. > > + unknownSelector -> [ > > - false -> [self assertCanceled]. > > - 'yourself' -> [self assertSucceeded: 'griffle self yourself']. > > - self unknownSelector -> [ > > self assertSucceeded. > > + self assert: (Symbol lookup: unknownSelector) notNil] }. > > + unknownSelector := self generateUnknownSelector. > > - self assert: (Symbol hasInterned: self unknownSelector ifTrue: [:symbol |]). > > - Symbol extern: self unknownSelector] }. > > self > > + compiling: 'griffle [ self ' , unknownSelector , ' ] value' > > - compiling: 'griffle [ self ' , self unknownSelector , ' ] value' > > shouldRaise: UnknownSelector > > + andSelect: unknownSelector > > - andSelect: self unknownSelector > > testing: { > > + false -> [ > > + self assertCanceled. > > + self assert: (Symbol lookup: unknownSelector) isNil]. > > + 'yourself' -> [ > > + self assertSucceeded: 'griffle [ self yourself ] value'. > > + self assert: (Symbol lookup: unknownSelector) isNil]. > > + unknownSelector -> [ > > - false -> [self assertCanceled]. > > - 'yourself' -> [self assertSucceeded: 'griffle [ self yourself ] value']. > > - self unknownSelector -> [ > > self assertSucceeded. > > + self assert: (Symbol lookup: unknownSelector) notNil] }.! > > - self assert: (Symbol hasInterned: self unknownSelector ifTrue: [:symbol |])] }.! > > > > Item was removed: > > - ----- Method: CompilerExceptionsTest>>unknownSelector (in category 'private') ----- > > - unknownSelector > > - > > - ^ 'yourrsellff'! > > > > Item was removed: > > - ----- Method: Symbol class>>extern: (in category '*Tests-release') ----- > > - extern: aStringOrSymbol > > - > > - {NewSymbols. SymbolTable} do: [:table | > > - table remove: aStringOrSymbol ifAbsent: []].! > > > > > > |
> a massive rewrite of (Symbol hasInterned: aString ifTrue: [:aSymbol | ...]) into ((Symbol lookup: aString) ifNotNil: [:aSymbol | ...]) is possible Hmm... only 34 senders. Would me take about 2 hours max, I suppose. I volunteer. Should I do it? Best, Marcel
|
Free forum by Nabble | Edit this page |