Skip to content

Commit

Permalink
Warn when assign a concatenating string expression in a loop while ta…
Browse files Browse the repository at this point in the history
…rget variable is outside the scope
  • Loading branch information
ChAoSUnItY committed Mar 12, 2022
1 parent 66ad61e commit 53d5890
Show file tree
Hide file tree
Showing 6 changed files with 45 additions and 21 deletions.
12 changes: 12 additions & 0 deletions src/main/kotlin/org/casc/lang/ast/Expression.kt
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,18 @@ import org.casc.lang.table.Reference
import org.casc.lang.table.Type

sealed class Expression {
companion object {
fun flattenExpressionTree(expressions: Expression): List<Expression?> {
val flattenedExpressions = mutableListOf<Expression?>()

for (expr in expressions.getExpressions())
if (expr is BinaryExpression) flattenedExpressions += flattenExpressionTree(expr)
else flattenedExpressions += expr

return flattenedExpressions
}
}

abstract val pos: Position?

/* Provided by Checker Unit */
Expand Down
13 changes: 12 additions & 1 deletion src/main/kotlin/org/casc/lang/checker/Checker.kt
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import org.casc.lang.ast.Function
import org.casc.lang.compilation.AbstractPreference
import org.casc.lang.compilation.Error
import org.casc.lang.compilation.Report
import org.casc.lang.compilation.Warning
import org.casc.lang.table.*
import org.casc.lang.utils.getOrElse
import org.casc.lang.utils.pforEach
Expand Down Expand Up @@ -458,7 +459,7 @@ class Checker(private val preference: AbstractPreference) {
}
}
is JForStatement -> {
val innerScope = Scope(scope)
val innerScope = Scope(scope, isLoopScope = true)

checkStatement(statement.initStatement, innerScope, returnType, useSameScope = true)

Expand Down Expand Up @@ -622,6 +623,16 @@ class Checker(private val preference: AbstractPreference) {
variable.type = PrimitiveType.Null
}

if (scope.scopeDepth > variable.declaredScopeDepth &&
expression.rightExpression is BinaryExpression &&
expression.rightExpression.isConcatExpression) {
reports += Warning(
expression.pos,
"Potential unnecessary performance cost while concatenating string through `+` and assign to variable in parent context in loop",
"Consider use `java::lang::StringBuilder`"
)
}

expression.leftExpression.isAssignedBy = true
}
}
Expand Down
14 changes: 2 additions & 12 deletions src/main/kotlin/org/casc/lang/emitter/Emitter.kt
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ class Emitter(private val preference: AbstractPreference) {
}
}
is JForStatement -> {
if (statement.initStatement != null)
if (statement.initStatement != null && statement.initStatement is ExpressionStatement && statement.initStatement.expression != null)
emitStatement(methodVisitor, statement.initStatement)

val startLabel = Label()
Expand Down Expand Up @@ -529,17 +529,7 @@ class Emitter(private val preference: AbstractPreference) {
methodVisitor: MethodVisitor,
expression: BinaryExpression
) {
fun flattenExpressionTree(expressions: BinaryExpression): List<Expression?> {
val flattenedExpressions = mutableListOf<Expression?>()

for (expr in expressions.getExpressions())
if (expr is BinaryExpression) flattenedExpressions += flattenExpressionTree(expr)
else flattenedExpressions += expr

return flattenedExpressions
}

val flattenedExpressions = flattenExpressionTree(expression)
val flattenedExpressions = Expression.flattenExpressionTree(expression)

if (flattenedExpressions.all { it is LiteralExpression }) {
val stringLiteral = flattenedExpressions.map { (it as LiteralExpression).getLiteral() }.joinToString("")
Expand Down
18 changes: 12 additions & 6 deletions src/main/kotlin/org/casc/lang/table/Scope.kt
Original file line number Diff line number Diff line change
Expand Up @@ -22,15 +22,18 @@ data class Scope(
var fields: MutableSet<ClassField> = mutableSetOf(),
var signatures: MutableSet<FunctionSignature> = mutableSetOf(),
var variables: MutableList<Variable> = mutableListOf(),
var isCompScope: Boolean = false
var scopeDepth: Int = 0, // Scope always starts from global scope
var isCompScope: Boolean = false,
var isLoopScope: Boolean = false,
) {
constructor(
parent: Scope,
companion: Boolean? = null,
mutable: Boolean? = null,
accessor: Accessor? = null,
classPath: Reference? = null,
isCompScope: Boolean = false
isCompScope: Boolean = false,
isLoopScope: Boolean = false,
) : this(
parent.preference,
companion ?: parent.companion,
Expand All @@ -42,7 +45,9 @@ data class Scope(
parent.fields.toMutableSet(),
parent.signatures.toMutableSet(),
parent.variables.toMutableList(),
isCompScope
parent.scopeDepth + 1,
isCompScope,
isLoopScope
) {
if (variables.isEmpty() && !isCompScope) {
// Insert a dummy variable
Expand Down Expand Up @@ -222,7 +227,8 @@ data class Scope(
mutable,
variableName,
type,
index
index,
scopeDepth
)

if (variables.contains(variable)) return false
Expand All @@ -233,8 +239,8 @@ data class Scope(
fun findVariable(variableName: String): Variable? =
if (!isCompScope && (variableName == "self" || variableName == "super")) {
when (variableName) {
"self" -> Variable(true, "self", findType(classReference), 0)
"super" -> Variable(true, "super", findType(parentClassPath), 0)
"self" -> Variable(true, "self", findType(classReference), 0, scopeDepth)
"super" -> Variable(true, "super", findType(parentClassPath), 0, scopeDepth)
else -> null // Should not happen
}
} else variables.find { it.name == variableName }
Expand Down
2 changes: 1 addition & 1 deletion src/main/kotlin/org/casc/lang/table/Variable.kt
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ package org.casc.lang.table
/**
* Refers to function's local variable, its type might be null since its value could be null.
*/
data class Variable(val mutable: Boolean, val name: String, var type: Type?, val index: Int) {
data class Variable(val mutable: Boolean, val name: String, var type: Type?, val index: Int, val declaredScopeDepth: Int) {
override fun equals(other: Any?): Boolean =
if (other !is Variable) false
else other.name == name
Expand Down
7 changes: 6 additions & 1 deletion test/Main.casc
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,12 @@ impl Main : Rock {
self.a = k
}

new(): self(1) {}
new(): self(1) {
mut a := ""
for ;; {
a = a + 1
}
}

ovrd fn lol(): i32 {
mut a := "ab"
Expand Down

0 comments on commit 53d5890

Please sign in to comment.