[Help-smalltalk] fix error management in gst-remote

[Paolo Bonzini]

Aha, the crash when loading a nonexistent package was actually a crash 
on any error, even --eval='self halt'.  Here is a nice cleanup in the 
Parser package that also happens to fix the bug.

A couple years ago I added a hack to handle the parsing of comments in 
gst-convert.  The problem is that comment offsets are stored relative to 
the input stream of the scanner, but the source code was stored as a 
string in the RBMethodNode, so it was simply 1-based.

Since all that was done on the source code was #copyFrom:to: I decided 
to wrap the source code with a MappedCollection to get the right 
indices.  This works marvellously, but has several drawbacks.  First of 
all, #do: does not work on the MappedCollection, because the 
MappedCollection's domain includes out-of-range indices (the 
MappedCollection is sequenceable -- so, in order to map file-offset 3 to 
source-offset 1, the file-offset 1 has to be mapped to the invalid 
source-offset -1).  For this reason, for example, 
LoadedMethod>>#asString was accessing the MappedCollection's #domain 
method directly, thus assuming that the RBMethodNode's source was stored 
in a MappedCollection.  Not so nice.

In the case at hand, in addition, the MappedCollection percolated down 
to the CompiledMethod, and the backtrace-printing was unable to cope 
with it.  Just making sure that something sensible is used as the 
method's source code would fix the gst-remote crash.  In other words, 
the way to fix this, is to cleanup the entire mess.

Here is what I did.  I kept the MappedCollection hack, but I use a 
specific subclass of MappedCollection instead, MappedSourceCode.  This 
allows me to do some things:

1) I can add a #asSourceCode method to MappedSourceCode, which is used 
to reconstruct a FileSegment from the source code when the code is 
placed in a method.

I use it in two place.  First to fix the bug in gst-remote, secondly to 
allow LoadedMethod objects to answer a FileSegment for #methodSourceCode

2) I can implement #asString in MappedSourceCode (so that it is 
polymorphic with FileSegment) -- this is the right place to move the 
#domain ugliness to, because here it is not an ugliness anymore (just an 
implementation detail).

The remaining bit of ugliness is that I'm not yet implementing #do: on 
MappedSourceCode, but that can come later.

3) Since I was at it, I noticed that a message with the nice comment 
"needed to do the documentation" in kernel/StreamOps.st is not needed 
anymore (definitely not with the new implementation, maybe even before).

Paolo

commit c98e29426a83b57eed9f09f9dd3df815467f5c11
Author: Paolo Bonzini <address@hidden>
Date:   Sun Jun 7 18:13:44 2009 +0200

    cleanup source code management and FileSegment generation in STFileParser
    
    2009-06-07  Paolo Bonzini  <address@hidden>
    
        * kernel/StreamOps.st: Remove #segmentFrom:to:.
    
    packages/stinst/parser:
    2009-06-07  Paolo Bonzini  <address@hidden>
    
        * STCompiler.st: Send #asSourceCode to the method's source code.
        * STFileParser.st: Define a MappedSourceCode class and use it instead
        of MappedCollection.  Define #segmentFrom:to: for Stream.
        Define #asSourceCode for any object.
        * STLoaderObjs.st: Remove #methodSourceString hack involving
        MappedCollection.

diff --git a/ChangeLog b/ChangeLog
index beecd9e..6c51bb6 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,7 @@
+2009-06-07  Paolo Bonzini  <address@hidden>
+
+       * kernel/StreamOps.st: Remove #segmentFrom:to:.
+
 2009-04-27  Paolo Bonzini  <address@hidden>
 
        * main.c: Fix for real.
diff --git a/kernel/StreamOps.st b/kernel/StreamOps.st
index 09052e9..c12cb50 100644
--- a/kernel/StreamOps.st
+++ b/kernel/StreamOps.st
@@ -166,19 +166,6 @@ Stream subclass: ConcatenatedStream [
        ^stream copyFrom: (start - adjust max: 0) to: end - adjust
     ]
 
-    segmentFrom: start to: end [
-       "needed to do the documentation"
-
-       <category: 'all'>
-       | adjust stream |
-       stream := self stream.
-       end + 1 = start ifTrue: [^''].
-       adjust := end <= startPos 
-           ifTrue: [stream := last. lastStart]
-           ifFalse: [adjust := startPos].
-       ^stream segmentFrom: (start - adjust max: 0) to: end - adjust
-    ]
-
     addStream: stream [
        <category: 'initializing'>
        streams addLast: stream
diff --git a/packages/stinst/parser/ChangeLog b/packages/stinst/parser/ChangeLog
index dab3df7..831d07d 100644
--- a/packages/stinst/parser/ChangeLog
+++ b/packages/stinst/parser/ChangeLog
@@ -1,5 +1,14 @@
 2009-06-07  Paolo Bonzini  <address@hidden>
 
+       * STCompiler.st: Send #asSourceCode to the method's source code.
+       * STFileParser.st: Define a MappedSourceCode class and use it instead
+       of MappedCollection.  Define #segmentFrom:to: for Stream.
+       Define #asSourceCode for any object.
+       * STLoaderObjs.st: Remove #methodSourceString hack involving
+       MappedCollection.
+
+2009-06-07  Paolo Bonzini  <address@hidden>
+
        * GSTParser.st: Fix compilation of class variables.
 
 2009-06-07  Paolo Bonzini  <address@hidden>
diff --git a/packages/stinst/parser/STCompiler.st 
b/packages/stinst/parser/STCompiler.st
index d10d580..d4cb721 100644
--- a/packages/stinst/parser/STCompiler.st
+++ b/packages/stinst/parser/STCompiler.st
@@ -441,7 +441,7 @@ indexed'' bytecode. The resulting stream is
                    bytecodes: bytecodes contents
                    depth: maxDepth + symTable numTemps.
        (method descriptor)
-           setSourceCode: node source;
+           setSourceCode: node source asSourceCode;
            methodClass: UndefinedObject;
            selector: #executeStatements.
        ^method
@@ -465,7 +465,7 @@ indexed'' bytecode. The resulting stream is
                    bytecodes: bytecodes contents
                    depth: maxDepth + node body temporaries size + node 
arguments size.
        (method descriptor)
-           setSourceCode: node source;
+           setSourceCode: node source asSourceCode;
            methodClass: symTable environment;
            selector: node selector.
        method attributesDo: 
diff --git a/packages/stinst/parser/STFileParser.st 
b/packages/stinst/parser/STFileParser.st
index f8e53c2..9b4f8ff 100644
--- a/packages/stinst/parser/STFileParser.st
+++ b/packages/stinst/parser/STFileParser.st
@@ -27,7 +27,6 @@
 |
  ======================================================================"
 
-
 
 RBParser subclass: STFileParser [
     | driver |
@@ -149,8 +148,7 @@ RBParser subclass: STFileParser [
            ifFalse: 
                [method := RBMethodNode selectorParts: #() arguments: #().
                node parent: method].
-       source := scanner stream copyFrom: start to: stop.
-       source := MappedCollection collection: source map: (1 - start to: stop).
+       source := MappedSourceCode on: scanner from: start to: stop.
        method source: source.
        ^node
     ]
@@ -377,6 +375,7 @@ PositionableStream extend [
                self species name}
     ]
 
+
     segmentFrom: startPos to: endPos [
        "Answer an object that, when sent #asString, will yield the result
         of sending `copyFrom: startPos to: endPos' to the receiver"
@@ -388,7 +387,18 @@ PositionableStream extend [
 ]
 
 
-
+Stream extend [
+
+    segmentFrom: startPos to: endPos [
+       "Answer an object that, when sent #asString, will yield the result
+        of sending `copyFrom: startPos to: endPos' to the receiver"
+
+       <category: 'compiling'>
+       ^nil
+    ]
+
+]
+
 FileStream extend [
 
     segmentFrom: startPos to: endPos [
@@ -396,6 +406,7 @@ FileStream extend [
         of sending `copyFrom: startPos to: endPos' to the receiver"
 
        <category: 'compiling'>
+       self isPipe ifTrue: [^nil].
        ^FileSegment 
            on: self file
            startingAt: startPos
@@ -403,4 +414,46 @@ FileStream extend [
     ]
 
 ]
+
+MappedCollection subclass: MappedSourceCode [
+    <comment: 'This class is a hack.  It allows the positions in the tokens
+and in the comments to be file-based, while at the same time only the source
+code of the method is kept in memory.'>
+    <category: 'Refactory-Parser'>
+
+    | sourceCode |
+
+    MappedSourceCode class >> on: aScanner from: start to: stop [
+       <category: 'instance creation'>
+       | collection coll sourceCode |
+       collection := aScanner stream copyFrom: start to: stop.
+       coll := self collection: collection map: (1 - start to: stop).
+       sourceCode := (aScanner stream segmentFrom: start to: stop)
+                       ifNil: [collection].
+       coll sourceCode: sourceCode.
+       ^coll
+    ]
+
+    asString [
+       <category: 'conversion'>
+       ^self domain asString
+    ]
+
+    asSourceCode [
+       <category: 'conversion'>
+       ^sourceCode
+    ]
+
+    sourceCode: anObject [
+       <category: 'private - initialization'>
+       sourceCode := anObject
+    ]
+]
+
+Object extend [
+    asSourceCode [
+       <category: 'private-compilation'>
+       ^self
+    ]
+]
 
diff --git a/packages/stinst/parser/STLoaderObjs.st 
b/packages/stinst/parser/STLoaderObjs.st
index 1509457..1bc1813 100644
--- a/packages/stinst/parser/STLoaderObjs.st
+++ b/packages/stinst/parser/STLoaderObjs.st
@@ -1197,7 +1197,7 @@ methodCategory
 !
 
 methodSourceCode
-    ^node source
+    ^node source asSourceCode
 !
 
 selector
@@ -1205,8 +1205,7 @@ selector
 !
 
 methodSourceString
-    "FIXME: this relies on the source code being a MappedCollection."
-    ^node source domain asString
+    ^node source asString
 ! !
 
 !LoadedMethod methodsFor: 'empty stubs'!