Minor changes on code review

This commit is contained in:
Valentin Kipyatkov
2016-04-27 21:50:22 +03:00
parent c2103e2200
commit 89dab52a14
7 changed files with 16 additions and 18 deletions
@@ -52,6 +52,8 @@ interface Transformation {
presentation presentation
} }
fun mergeWithPrevious(previousTransformation: SequenceTransformation): Transformation?
fun generateCode(chainedCallGenerator: ChainedCallGenerator): KtExpression fun generateCode(chainedCallGenerator: ChainedCallGenerator): KtExpression
val chainCallCount: Int val chainCallCount: Int
@@ -65,7 +67,7 @@ interface Transformation {
* Represents a transformation of input sequence into another sequence * Represents a transformation of input sequence into another sequence
*/ */
interface SequenceTransformation : Transformation { interface SequenceTransformation : Transformation {
fun mergeWithPrevious(previousTransformation: SequenceTransformation): SequenceTransformation? = null override fun mergeWithPrevious(previousTransformation: SequenceTransformation): SequenceTransformation? = null
val affectsIndex: Boolean val affectsIndex: Boolean
} }
@@ -74,7 +76,7 @@ interface SequenceTransformation : Transformation {
* Represents a final transformation of sequence which produces the result of the whole loop (for example, assigning a found value into a variable). * Represents a final transformation of sequence which produces the result of the whole loop (for example, assigning a found value into a variable).
*/ */
interface ResultTransformation : Transformation { interface ResultTransformation : Transformation {
fun mergeWithPrevious(previousTransformation: SequenceTransformation): ResultTransformation? = null override fun mergeWithPrevious(previousTransformation: SequenceTransformation): ResultTransformation? = null
val commentSavingRange: PsiChildRange val commentSavingRange: PsiChildRange
@@ -287,7 +287,7 @@ private fun MatchResult.generateCallChain(loop: KtForExpression): KtExpression {
} }
private fun mergeTransformations(match: TransformationMatch.Result): TransformationMatch.Result { private fun mergeTransformations(match: TransformationMatch.Result): TransformationMatch.Result {
val transformations = ArrayList<Transformation>().apply { addAll(match.sequenceTransformations); add(match.resultTransformation) } val transformations = (match.sequenceTransformations + match.resultTransformation).toMutableList()
var anyChange: Boolean var anyChange: Boolean
do { do {
@@ -295,12 +295,7 @@ private fun mergeTransformations(match: TransformationMatch.Result): Transformat
for (index in 0..transformations.lastIndex - 1) { for (index in 0..transformations.lastIndex - 1) {
val transformation = transformations[index] as SequenceTransformation val transformation = transformations[index] as SequenceTransformation
val next = transformations[index + 1] val next = transformations[index + 1]
val merged = when (next) { val merged = next.mergeWithPrevious(transformation) ?: continue
is SequenceTransformation -> next.mergeWithPrevious(transformation)
is ResultTransformation -> next.mergeWithPrevious(transformation)
else -> error("Unknown transformation type: $next")
} ?: continue
transformations[index] = merged transformations[index] = merged
transformations.removeAt(index + 1) transformations.removeAt(index + 1)
anyChange = true anyChange = true
@@ -122,7 +122,7 @@ class AddToCollectionTransformation(
targetCollection: KtExpression, targetCollection: KtExpression,
addOperationArgument: KtExpression addOperationArgument: KtExpression
): TransformationMatch.Result? { ): TransformationMatch.Result? {
val collectionInitialization = targetCollection.detectInitializationBeforeLoop(state.outerLoop, checkNoOtherUsagesInLoop = true) ?: return null val collectionInitialization = targetCollection.isVariableInitializedBeforeLoop(state.outerLoop, checkNoOtherUsagesInLoop = true) ?: return null
val collectionKind = collectionInitialization.initializer.isSimpleCollectionInstantiation() ?: return null val collectionKind = collectionInitialization.initializer.isSimpleCollectionInstantiation() ?: return null
val argumentIsInputVariable = addOperationArgument.isVariableReference(state.inputVariable) val argumentIsInputVariable = addOperationArgument.isVariableReference(state.inputVariable)
when (collectionKind) { when (collectionKind) {
@@ -223,7 +223,7 @@ class FilterToTransformation private constructor(
condition: KtExpression, condition: KtExpression,
isInverse: Boolean isInverse: Boolean
): ResultTransformation { ): ResultTransformation {
val initialization = targetCollection.detectInitializationBeforeLoop(loop, checkNoOtherUsagesInLoop = true) val initialization = targetCollection.isVariableInitializedBeforeLoop(loop, checkNoOtherUsagesInLoop = true)
if (initialization != null && initialization.initializer.hasNoSideEffect()) { if (initialization != null && initialization.initializer.hasNoSideEffect()) {
val transformation = FilterToTransformation(loop, inputVariable, indexVariable, initialization.initializer, condition, isInverse) val transformation = FilterToTransformation(loop, inputVariable, indexVariable, initialization.initializer, condition, isInverse)
return AssignToVariableResultTransformation.createDelegated(transformation, initialization) return AssignToVariableResultTransformation.createDelegated(transformation, initialization)
@@ -252,7 +252,7 @@ class FilterNotNullToTransformation private constructor(
loop: KtForExpression, loop: KtForExpression,
targetCollection: KtExpression targetCollection: KtExpression
): ResultTransformation { ): ResultTransformation {
val initialization = targetCollection.detectInitializationBeforeLoop(loop, checkNoOtherUsagesInLoop = true) val initialization = targetCollection.isVariableInitializedBeforeLoop(loop, checkNoOtherUsagesInLoop = true)
if (initialization != null && initialization.initializer.hasNoSideEffect()) { if (initialization != null && initialization.initializer.hasNoSideEffect()) {
val transformation = FilterNotNullToTransformation(loop, initialization.initializer) val transformation = FilterNotNullToTransformation(loop, initialization.initializer)
return AssignToVariableResultTransformation.createDelegated(transformation, initialization) return AssignToVariableResultTransformation.createDelegated(transformation, initialization)
@@ -298,7 +298,7 @@ class MapToTransformation private constructor(
mapping: KtExpression, mapping: KtExpression,
mapNotNull: Boolean mapNotNull: Boolean
): ResultTransformation { ): ResultTransformation {
val initialization = targetCollection.detectInitializationBeforeLoop(loop, checkNoOtherUsagesInLoop = true) val initialization = targetCollection.isVariableInitializedBeforeLoop(loop, checkNoOtherUsagesInLoop = true)
if (initialization != null && initialization.initializer.hasNoSideEffect()) { if (initialization != null && initialization.initializer.hasNoSideEffect()) {
val transformation = MapToTransformation(loop, inputVariable, indexVariable, initialization.initializer, mapping, mapNotNull) val transformation = MapToTransformation(loop, inputVariable, indexVariable, initialization.initializer, mapping, mapNotNull)
return AssignToVariableResultTransformation.createDelegated(transformation, initialization) return AssignToVariableResultTransformation.createDelegated(transformation, initialization)
@@ -332,7 +332,7 @@ class FlatMapToTransformation private constructor(
targetCollection: KtExpression, targetCollection: KtExpression,
transform: KtExpression transform: KtExpression
): ResultTransformation { ): ResultTransformation {
val initialization = targetCollection.detectInitializationBeforeLoop(loop, checkNoOtherUsagesInLoop = true) val initialization = targetCollection.isVariableInitializedBeforeLoop(loop, checkNoOtherUsagesInLoop = true)
if (initialization != null && initialization.initializer.hasNoSideEffect()) { if (initialization != null && initialization.initializer.hasNoSideEffect()) {
val transformation = FlatMapToTransformation(loop, inputVariable, initialization.initializer, transform) val transformation = FlatMapToTransformation(loop, inputVariable, initialization.initializer, transform)
return AssignToVariableResultTransformation.createDelegated(transformation, initialization) return AssignToVariableResultTransformation.createDelegated(transformation, initialization)
@@ -74,7 +74,7 @@ class CountTransformation(
override fun match(state: MatchingState): TransformationMatch.Result? { override fun match(state: MatchingState): TransformationMatch.Result? {
val operand = state.statements.singleOrNull()?.isPlusPlusOf() ?: return null val operand = state.statements.singleOrNull()?.isPlusPlusOf() ?: return null
val initialization = operand.detectInitializationBeforeLoop(state.outerLoop, checkNoOtherUsagesInLoop = true) ?: return null val initialization = operand.isVariableInitializedBeforeLoop(state.outerLoop, checkNoOtherUsagesInLoop = true) ?: return null
if (initialization.variable.countUsages(state.outerLoop) != 1) return null // this should be the only usage of this variable inside the loop if (initialization.variable.countUsages(state.outerLoop) != 1) return null // this should be the only usage of this variable inside the loop
@@ -78,7 +78,7 @@ object FindTransformationMatcher : TransformationMatcher {
val left = binaryExpression.left ?: return null val left = binaryExpression.left ?: return null
val right = binaryExpression.right ?: return null val right = binaryExpression.right ?: return null
val initialization = left.detectInitializationBeforeLoop(state.outerLoop, checkNoOtherUsagesInLoop = true) ?: return null val initialization = left.isVariableInitializedBeforeLoop(state.outerLoop, checkNoOtherUsagesInLoop = true) ?: return null
if (initialization.variable.countUsages(state.outerLoop) != 1) return null // this should be the only usage of this variable inside the loop if (initialization.variable.countUsages(state.outerLoop) != 1) return null // this should be the only usage of this variable inside the loop
@@ -39,7 +39,7 @@ object IntroduceIndexMatcher : TransformationMatcher {
// there should be no continuation of the loop in statements before index increment // there should be no continuation of the loop in statements before index increment
if (restStatements.any { statement -> statement.anyDescendantOfType<KtContinueExpression>(::isContinueOfThisLoopOrOuter) }) return null if (restStatements.any { statement -> statement.anyDescendantOfType<KtContinueExpression>(::isContinueOfThisLoopOrOuter) }) return null
val variableInitialization = operand.detectInitializationBeforeLoop(state.outerLoop, checkNoOtherUsagesInLoop = false) val variableInitialization = operand.isVariableInitializedBeforeLoop(state.outerLoop, checkNoOtherUsagesInLoop = false)
?: return null ?: return null
if ((variableInitialization.initializer as? KtConstantExpression)?.text != "0") return null if ((variableInitialization.initializer as? KtConstantExpression)?.text != "0") return null
@@ -97,7 +97,7 @@ data class VariableInitialization(
val initializer: KtExpression) val initializer: KtExpression)
//TODO: we need more correctness checks (if variable is non-local or is local but can be changed by some local functions) //TODO: we need more correctness checks (if variable is non-local or is local but can be changed by some local functions)
fun KtExpression.detectInitializationBeforeLoop( fun KtExpression.isVariableInitializedBeforeLoop(
loop: KtForExpression, loop: KtForExpression,
checkNoOtherUsagesInLoop: Boolean checkNoOtherUsagesInLoop: Boolean
): VariableInitialization? { ): VariableInitialization? {
@@ -147,6 +147,7 @@ enum class CollectionKind {
} }
fun KtExpression.isSimpleCollectionInstantiation(): CollectionKind? { fun KtExpression.isSimpleCollectionInstantiation(): CollectionKind? {
//TODO: support mutableListOf() etc
val callExpression = this as? KtCallExpression ?: return null //TODO: it can be qualified too val callExpression = this as? KtCallExpression ?: return null //TODO: it can be qualified too
if (callExpression.valueArguments.isNotEmpty()) return null if (callExpression.valueArguments.isNotEmpty()) return null
val bindingContext = callExpression.analyze(BodyResolveMode.PARTIAL) val bindingContext = callExpression.analyze(BodyResolveMode.PARTIAL)