next step:

- re-categorize methods a bit - use #streamContents to simplify two methods - instead of lookupAndReadVar: name inContext: aContext I added a method to return the context where the variable is defined: lookupDefiningContextFor: name startingFrom: aContext - this way we can fix the write methods, too, to store in the correct context. - this means that we do not need indexFromIRFromScope: anymore, as we now always use the offset of the defintion context
astares · Sep 20, 2019 · cd33ba9 · cd33ba9
1 parent ef20d29
commit cd33ba9
Show file tree

Hide file tree

Showing 6 changed files with 58 additions and 66 deletions.
diff --git a/src/OpalCompiler-Core/OCAbstractMethodScope.class.st b/src/OpalCompiler-Core/OCAbstractMethodScope.class.st
@@ -14,7 +14,7 @@ Class {
 	#category : #'OpalCompiler-Core-Semantics'
 }
 
-{ #category : #'temp vars' }
+{ #category : #'temp vars - copying' }
 OCAbstractMethodScope >> addCopiedTempToAllScopesUpToDefVector: aVariable [
 
 	(self hasCopyingTempNamed: aVariable name)
@@ -23,26 +23,28 @@ OCAbstractMethodScope >> addCopiedTempToAllScopesUpToDefVector: aVariable [
 	^ self outerScope addCopiedTempToAllScopesUpToDefVector: aVariable.
 ]
 
-{ #category : #'temp vars' }
+{ #category : #'temp vars - copying' }
 OCAbstractMethodScope >> addCopyingTemp: aTempVar [
-	^ copiedVars add: (OCCopyingTempVariable new
+	copiedVars add: (OCCopyingTempVariable new
 			originalVar: aTempVar originalVar;
 			name: aTempVar name;
 			escaping: aTempVar escaping;
 			usage: aTempVar usage;
 			scope: self;
-			yourself)
+			yourself).
+	^copiedVars
 ]
 
-{ #category : #'temp vars' }
+{ #category : #'temp vars - copying' }
 OCAbstractMethodScope >> addCopyingTempNamed: name [
-	^ copiedVars add: (OCCopyingTempVariable new
+	copiedVars add: (OCCopyingTempVariable new
 			name: name;
 			scope: self;
-			yourself)
+			yourself).
+	^copiedVars
 ]
 
-{ #category : #'temp vars' }
+{ #category : #'temp vars - copying' }
 OCAbstractMethodScope >> addCopyingTempToAllScopesUpToDefTemp: aVar [
 
 	(self hasCopyingTempNamed: aVar name) ifFalse: [self addCopyingTemp: aVar].
@@ -51,11 +53,11 @@ OCAbstractMethodScope >> addCopyingTempToAllScopesUpToDefTemp: aVar [
 
 ]
 
-{ #category : #'temp vars' }
+{ #category : #'temp vars - copying' }
 OCAbstractMethodScope >> addCopyingTempToAllScopesUpToDefVector: aName [
 
 	(self hasCopyingTempNamed: aName) ifFalse: [self addCopyingTempNamed: aName].
-	self tempVectorName = aName  ifTrue: [^ self].
+	self tempVectorName = aName ifTrue: [^ self].
 	^ self outerScope addCopyingTempToAllScopesUpToDefVector: aName.
 ]
 
@@ -74,7 +76,7 @@ OCAbstractMethodScope >> addTemp: aTempVariable withName: aString [
 				yourself)
 ]
 
-{ #category : #'temp vars' }
+{ #category : #'temp vars - vector' }
 OCAbstractMethodScope >> addVectorTemp: aTemp [
 	^ tempVector 
 		at:  aTemp name
@@ -89,26 +91,23 @@ OCAbstractMethodScope >> addVectorTemp: aTemp [
 
 { #category : #'temp vars' }
 OCAbstractMethodScope >> allTempNames [
-	^self allTemps collect: #name.
+	^self allTemps collect: [: each | each name]
 ]
 
 { #category : #'temp vars' }
 OCAbstractMethodScope >> allTemps [
 	"return all temps defined, even the ones in the outer scope that are not used in the current.
-	 This includes the arguments
-	 We do not need to care about shadowed temps as temp shadowing is not allowed."
-	| temps |
-	temps := OrderedCollection new.
-	self outerScope allTemps do: [ :var |
-	 	(self localTempNames includes: var name) ifFalse: [
-			temps add: var.]
-		].
-	temps addAll: self localTemps.
-	^temps asArray.
-
+	 This includes the arguments. We do not need to care about shadowed temps as temp shadowing is not allowed."
+
+	^ Array streamContents: [ :str | 
+			self outerScope allTemps
+				do: [ :var | 
+					(self localTempNames includes: var name)
+						ifFalse: [ str nextPut: var ] ].
+			str nextPutAll: self localTemps ]
 ]
 
-{ #category : #'temp vars' }
+{ #category : #'temp vars - copying' }
 OCAbstractMethodScope >> copiedVars [
 	^ copiedVars
 ]
@@ -125,17 +124,17 @@ OCAbstractMethodScope >> findVariable: aBlock ifNone: aNotFound [
 		^  outerScope findVariable: aBlock ifNone: aNotFound ]
 ]
 
-{ #category : #'temp vars' }
+{ #category : #'temp vars - copying' }
 OCAbstractMethodScope >> hasCopyingTempNamed: name [
 	^self copiedVars anySatisfy: [:each | each name = name]
 ]
 
-{ #category : #'temp vars' }
+{ #category : #'temp vars - vector' }
 OCAbstractMethodScope >> hasTempVector [
 	^ tempVector isNotEmpty
 ]
 
-{ #category : #initializing }
+{ #category : #accessing }
 OCAbstractMethodScope >> id: int [
 	id := int
 ]
@@ -161,27 +160,24 @@ OCAbstractMethodScope >> localTempNames [
 { #category : #'temp vars' }
 OCAbstractMethodScope >> localTemps [
 	"all temps accessed in the context... for tempVectors, it takes all the vars even those not used here"
-	| localVars |
-	localVars := OrderedCollection new.
-	self copiedVars do:  [:var |
-		var isStoringTempVector 
-			ifTrue: [var tempVectorForTempStoringIt do: [:tempVectorVars | localVars add: tempVectorVars]]
-	].
-	self tempVars do:  [:var | localVars add: var].
-	^localVars asArray
+
+	^ Array streamContents: [ :str | 
+			self copiedVars
+				do: [ :var | 
+					var isStoringTempVector
+						ifTrue: [ var tempVectorForTempStoringIt
+								do: [ :tempVectorVars | str nextPut: tempVectorVars ] ] ].
+			str nextPutAll: self tempVars ]
 ]
 
 { #category : #lookup }
-OCAbstractMethodScope >> lookupAndReadVar: name inContext: aContext [
-	"We lookup a variable in a context. If it not in this context, we look in the outer context using the corresponding outer scope. If found, we return the value"
+OCAbstractMethodScope >> lookupDefiningContextFor: name startingFrom: aContext [
+	"We lookup a variable in a context. If it not in this context, we look in the outer context using the corresponding outer scope. If found, we return the context"
 
-	| variable theValue |
-
-	variable := self
+	self
 		variableNamed: name
-		ifAbsent: [ 	^self outerScope lookupAndReadVar: name inContext: (self nextOuterScopeContextOf: aContext)].
-	theValue := variable readFromContext: aContext.
-	^ theValue
+		ifAbsent: [ ^self outerScope lookupDefiningContextFor: name startingFrom: (self nextOuterScopeContextOf: aContext)].
+	^aContext
 ]
 
 { #category : #lookup }
@@ -214,13 +210,13 @@ OCAbstractMethodScope >> lookupVarForDeclaration: name [
 	^self outerScope lookupVarForDeclaration: name
 ]
 
-{ #category : #debugging }
+{ #category : #scope }
 OCAbstractMethodScope >> methodScope [
 
 	^ self outerScope methodScope
 ]
 
-{ #category : #'temp vars' }
+{ #category : #'temp vars - vector' }
 OCAbstractMethodScope >> moveToVectorTemp: aTempVar [
 
 	self addVectorTemp: aTempVar.
@@ -284,12 +280,12 @@ OCAbstractMethodScope >> tempVars [
 	^ tempVars
 ]
 
-{ #category : #'temp vars' }
+{ #category : #'temp vars - vector' }
 OCAbstractMethodScope >> tempVector [
 	^ tempVector 
 ]
 
-{ #category : #'temp vars' }
+{ #category : #'temp vars - vector' }
 OCAbstractMethodScope >> tempVectorName [
 	"the name of the tempVector is not a valid name of a temp variable
 	 This way we avoid name clashes "

diff --git a/src/OpalCompiler-Core/OCAbstractScope.class.st b/src/OpalCompiler-Core/OCAbstractScope.class.st
@@ -55,12 +55,6 @@ OCAbstractScope >> isMethodScope [
 	^false
 ]
 
-{ #category : #lookup }
-OCAbstractScope >> lookupAndReadVar: name inContext: aContext [
-
-	^ self outerScope lookupAndReadVar: name inContext: aContext outerContext
-]
-
 { #category : #lookup }
 OCAbstractScope >> lookupSelector: name [
 

diff --git a/src/OpalCompiler-Core/OCBlockScope.class.st b/src/OpalCompiler-Core/OCBlockScope.class.st
@@ -14,7 +14,7 @@ OCBlockScope >> hasEscapingVars [
 
 { #category : #accessing }
 OCBlockScope >> inComingCopiedVars [
-	^ copiedVars select: [:each | outerScope copiedVars includes: each].
+	^ copiedVars select: [:each | outerScope copiedVars includes: each]
 ]
 
 { #category : #testing }

diff --git a/src/OpalCompiler-Core/OCCopyingTempVariable.class.st b/src/OpalCompiler-Core/OCCopyingTempVariable.class.st
@@ -52,13 +52,15 @@ OCCopyingTempVariable >> tempVectorForTempStoringIt [
 { #category : #debugging }
 OCCopyingTempVariable >> writeFromContext: aContext scope: contextScope value: aValue [
 
+	| definitionContext |
+	definitionContext := contextScope lookupDefiningContextFor: name startingFrom: aContext.
 
 	originalVar writeFromContext: aContext scope: contextScope value: aValue.
 
 	self flag: #FIXME.
 	"we need to change all the copies and the original, too"
 
-	^aContext 
-		tempAt: (self indexFromIRFromScope: contextScope)
+	^definitionContext 
+		tempAt: self indexFromIR
 		put: aValue
 ]
diff --git a/src/OpalCompiler-Core/OCMethodScope.class.st b/src/OpalCompiler-Core/OCMethodScope.class.st
@@ -12,7 +12,7 @@ OCMethodScope >> isMethodScope [
 	^true
 ]
 
-{ #category : #testing }
+{ #category : #scope }
 OCMethodScope >> methodScope [
 
 	^ self

diff --git a/src/OpalCompiler-Core/OCTempVariable.class.st b/src/OpalCompiler-Core/OCTempVariable.class.st
@@ -60,11 +60,6 @@ OCTempVariable >> indexFromIR [
 	^scope outerNotOptimizedScope node irInstruction indexForVarNamed: name
 ]
 
-{ #category : #debugging }
-OCTempVariable >> indexFromIRFromScope: aScope [
-	^aScope outerNotOptimizedScope node irInstruction indexForVarNamed: name
-]
-
 { #category : #initialization }
 OCTempVariable >> initialize [
 	super initialize.
@@ -145,13 +140,18 @@ OCTempVariable >> readFromContext: aContext [
 
 { #category : #debugging }
 OCTempVariable >> readFromContext: aContext scope: contextScope [
-
-	^ contextScope lookupAndReadVar: name inContext: aContext
+	| definitionContext |
+	definitionContext := contextScope lookupDefiningContextFor: name startingFrom: aContext.
+	^ self readFromContext: definitionContext
 ]
 
 { #category : #debugging }
 OCTempVariable >> writeFromContext: aContext scope: contextScope value: aValue [
-	^aContext 
-		tempAt: (self indexFromIRFromScope: contextScope) 
+
+	| definitionContext |
+	definitionContext := contextScope lookupDefiningContextFor: name startingFrom: aContext.
+
+	^definitionContext 
+		tempAt: self indexFromIR
 		put: aValue
 ]