0007570: CompiledMethod asString fails with an improperStore

(0013932 - 165 - 165 - 165 - 165 - 165 - 165)
leves
11-15-10 04:23

I doubt the sender of #asString would expect a string with random characters. If we want to implement #asString, then [^self printString] is a better candidate IMHO.

(0013933 - 230 - 270 - 270 - 270 - 270 - 270)
FrankShearar
11-15-10 09:39

It bugs me more that ByteArray does one thing when a subclass, CompiledMethod, doesn't.

I'd be happy with anything that worked the same for both.

If we want "^ self printString" it'd be simpler to remove ByteArray>>asString.

(0013934 - 452 - 458 - 458 - 458 - 458 - 458)
leves
11-15-10 10:02

It's sometimes useful to convert a ByteArray to a ByteString the way #asString does it (binary-ascii buffer changes are done this way in some streams IIRC). But it's hardly useful to do the same with a CompiledMethod. This wouldn't be an issue if CompiledMethod were a simple variableByteSubclass of Object instead of ByteArray, but that way some methods would be duplicated.
Another option is to do the same what Pharo does: ^self getSource asString.

(0013971 - 662 - 726 - 726 - 726 - 726 - 726)
leves
12-08-10 02:47

I implemented the method in Kernel-ul.521 as Object does it with the following reasoning:

"Why doesn't it return a string with the bytes as characters like ByteArray does it?
Because it's not very useful to get an unreadable string with random looking characters. Also CompiledMethod is a subclass of ByteArray only to avoid code
duplication, it could be a variableByteSubclass of Object.

Why doesn't it return the source code like Pharo does it?
Because that requires several changes in the way the sources and changes files are used (concurrency issues). And it's a lot slower."

I set the status to feedback, because I'm interested in your opinion.

(0013974 - 328 - 340 - 340 - 340 - 340 - 340)
FrankShearar
12-10-10 09:26

First, I'm happy that it doesn't break, like the old behaviour.

Second, I'm happy, as long as we understand that a CompiledMethod is-a ByteArray simply for purposes of code reuse - as an implementation detail, in other words - not because it is of necessity a ByteArray. (Maybe it'd be useful to say that in a class comment.)

(0013978 - 932 - 1869 - 1869 - 1869 - 1869 - 1869)
leves
12-12-10 04:09

Here's a small hack (really just a hack) to change CompiledMethod to be a subclass of Object. Use it with care:

| class |
class := CompiledMethod superclass.
[ class == Object ] whileFalse: [
    class selectorsAndMethodsDo: [ :selector :method |
        (CompiledMethod methodDictionary includesKey: selector) ifFalse: [
            CompiledMethod
                compile: method getSource asString
                classified: class name ] ].
    class class selectorsAndMethodsDo: [ :selector :method |
        (CompiledMethod class methodDictionary includesKey: selector) ifFalse: [
            CompiledMethod class
                compile: method getSource asString
                classified: class name ] ].
    class := class superclass ].
[ Object variableByteSubclass: #CompiledMethod
    instanceVariableNames: ''
    classVariableNames: 'MutexForPicking RandomForPicking LargeFrame SmallFrame'
    poolDictionaries: ''
    category: 'Kernel-Methods' ]
    on: Error, Warning
    do: [ :ex | ex resume ]

(0013989 - 164 - 174 - 174 - 174 - 174 - 174)
FrankShearar
12-17-10 14:44

I'm happy with the class comment saying "We subclass ByteArray to avoid unnecessarily duplicating ByteArray's methods, not because a CompiledMethod is-a ByteArray."

(0014033 - 37 - 37 - 37 - 37 - 37 - 37)
leves
01-29-11 12:21

I added the comment in Kernel-ul.540.

(0014034 - 18 - 18 - 18 - 18 - 18 - 18)
FrankShearar
01-29-11 16:16

Ah, great. Thanks!

Issue History
Date Modified	Username	Field	Change
11-13-10 17:41	FrankShearar	New Issue
11-15-10 04:23	leves	Note Added: 0013932
11-15-10 09:39	FrankShearar	Note Added: 0013933
11-15-10 10:02	leves	Note Added: 0013934
12-08-10 02:47	leves	Note Added: 0013971
12-08-10 02:47	leves	Assigned To	=> leves
12-08-10 02:47	leves	Status	new => feedback
12-10-10 02:37	leves	Status	feedback => assigned
12-10-10 02:37	leves	Assigned To	leves => FrankShearar
12-10-10 09:26	FrankShearar	Note Added: 0013974
12-12-10 04:09	leves	Note Added: 0013978
12-17-10 14:44	FrankShearar	Note Added: 0013989
01-29-11 12:21	leves	Status	assigned => resolved
01-29-11 12:21	leves	Fixed in Version	=> trunk
01-29-11 12:21	leves	Resolution	open => fixed
01-29-11 12:21	leves	Note Added: 0014033
01-29-11 16:16	FrankShearar	Note Added: 0014034
02-06-11 23:48	leves	Status	resolved => closed

Notes
(0013932 - 165 - 165 - 165 - 165 - 165 - 165) leves 11-15-10 04:23	I doubt the sender of #asString would expect a string with random characters. If we want to implement #asString, then [^self printString] is a better candidate IMHO.

(0013933 - 230 - 270 - 270 - 270 - 270 - 270) FrankShearar 11-15-10 09:39	It bugs me more that ByteArray does one thing when a subclass, CompiledMethod, doesn't. I'd be happy with anything that worked the same for both. If we want "^ self printString" it'd be simpler to remove ByteArray>>asString.

(0013934 - 452 - 458 - 458 - 458 - 458 - 458) leves 11-15-10 10:02	It's sometimes useful to convert a ByteArray to a ByteString the way #asString does it (binary-ascii buffer changes are done this way in some streams IIRC). But it's hardly useful to do the same with a CompiledMethod. This wouldn't be an issue if CompiledMethod were a simple variableByteSubclass of Object instead of ByteArray, but that way some methods would be duplicated. Another option is to do the same what Pharo does: ^self getSource asString.

(0013971 - 662 - 726 - 726 - 726 - 726 - 726) leves 12-08-10 02:47	I implemented the method in Kernel-ul.521 as Object does it with the following reasoning: "Why doesn't it return a string with the bytes as characters like ByteArray does it? Because it's not very useful to get an unreadable string with random looking characters. Also CompiledMethod is a subclass of ByteArray only to avoid code duplication, it could be a variableByteSubclass of Object. Why doesn't it return the source code like Pharo does it? Because that requires several changes in the way the sources and changes files are used (concurrency issues). And it's a lot slower." I set the status to feedback, because I'm interested in your opinion.

(0013974 - 328 - 340 - 340 - 340 - 340 - 340) FrankShearar 12-10-10 09:26	First, I'm happy that it doesn't break, like the old behaviour. Second, I'm happy, as long as we understand that a CompiledMethod is-a ByteArray simply for purposes of code reuse - as an implementation detail, in other words - not because it is of necessity a ByteArray. (Maybe it'd be useful to say that in a class comment.)

(0013978 - 932 - 1869 - 1869 - 1869 - 1869 - 1869) leves 12-12-10 04:09	Here's a small hack (really just a hack) to change CompiledMethod to be a subclass of Object. Use it with care: \| class \| class := CompiledMethod superclass. [ class == Object ] whileFalse: [ class selectorsAndMethodsDo: [ :selector :method \| (CompiledMethod methodDictionary includesKey: selector) ifFalse: [ CompiledMethod compile: method getSource asString classified: class name ] ]. class class selectorsAndMethodsDo: [ :selector :method \| (CompiledMethod class methodDictionary includesKey: selector) ifFalse: [ CompiledMethod class compile: method getSource asString classified: class name ] ]. class := class superclass ]. [ Object variableByteSubclass: #CompiledMethod instanceVariableNames: '' classVariableNames: 'MutexForPicking RandomForPicking LargeFrame SmallFrame' poolDictionaries: '' category: 'Kernel-Methods' ] on: Error, Warning do: [ :ex \| ex resume ]

(0013989 - 164 - 174 - 174 - 174 - 174 - 174) FrankShearar 12-17-10 14:44	I'm happy with the class comment saying "We subclass ByteArray to avoid unnecessarily duplicating ByteArray's methods, not because a CompiledMethod is-a ByteArray."

(0014033 - 37 - 37 - 37 - 37 - 37 - 37) leves 01-29-11 12:21	I added the comment in Kernel-ul.540.

(0014034 - 18 - 18 - 18 - 18 - 18 - 18) FrankShearar 01-29-11 16:16	Ah, great. Thanks!

Relationships