fix error management in gst-remote

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 <[hidden email]>
Date:   Sun Jun 7 18:13:44 2009 +0200

    cleanup source code management and FileSegment generation in STFileParser
   
    2009-06-07  Paolo Bonzini  <[hidden email]>
   
    * kernel/StreamOps.st: Remove #segmentFrom:to:.
   
    packages/stinst/parser:
    2009-06-07  Paolo Bonzini  <[hidden email]>
   
    * 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  <[hidden email]>
+
+ * kernel/StreamOps.st: Remove #segmentFrom:to:.
+
 2009-04-27  Paolo Bonzini  <[hidden email]>
 
  * 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  <[hidden email]>
 
+ * 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  <[hidden email]>
+
  * GSTParser.st: Fix compilation of class variables.
 
 2009-06-07  Paolo Bonzini  <[hidden email]>
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'!

_______________________________________________
help-smalltalk mailing list
[hidden email]
http://lists.gnu.org/mailman/listinfo/help-smalltalk