removed chained comparisons again, because they caused invalid expression evaluations due to changed semantics.

This commit is contained in:
Irmen de Jong 2024-01-01 16:00:11 +01:00
parent 498841d45d
commit 0e086d788b
6 changed files with 14 additions and 59 deletions

View File

@ -142,11 +142,9 @@ class ConstantFoldingOptimizer(private val program: Program, private val errors:
// const fold when both operands are a const. // const fold when both operands are a const.
// if in a chained comparison, that one has to be desugared first though. // if in a chained comparison, that one has to be desugared first though.
if(leftconst != null && rightconst != null) { if(leftconst != null && rightconst != null) {
if((expr.parent as? BinaryExpression)?.isChainedComparison()!=true) {
val result = evaluator.evaluate(leftconst, expr.operator, rightconst) val result = evaluator.evaluate(leftconst, expr.operator, rightconst)
modifications += IAstModification.ReplaceNode(expr, result, parent) modifications += IAstModification.ReplaceNode(expr, result, parent)
} }
}
if(leftconst==null && rightconst!=null && rightconst.number<0.0) { if(leftconst==null && rightconst!=null && rightconst.number<0.0) {
if (expr.operator == "-") { if (expr.operator == "-") {

View File

@ -6,9 +6,11 @@ import prog8.ast.Program
import prog8.ast.base.AstException import prog8.ast.base.AstException
import prog8.ast.expressions.Expression import prog8.ast.expressions.Expression
import prog8.ast.expressions.NumericLiteral import prog8.ast.expressions.NumericLiteral
import prog8.ast.printProgram
import prog8.ast.statements.Directive import prog8.ast.statements.Directive
import prog8.code.SymbolTableMaker import prog8.code.SymbolTableMaker
import prog8.code.ast.PtProgram import prog8.code.ast.PtProgram
import prog8.code.ast.printAst
import prog8.code.core.* import prog8.code.core.*
import prog8.code.optimize.optimizeIntermediateAst import prog8.code.optimize.optimizeIntermediateAst
import prog8.code.target.* import prog8.code.target.*
@ -94,10 +96,10 @@ fun compileProgram(args: CompilerArguments): CompilationResult? {
importedFiles = imported importedFiles = imported
processAst(program, args.errors, compilationOptions) processAst(program, args.errors, compilationOptions)
if (compilationOptions.optimize) {
// println("*********** COMPILER AST RIGHT BEFORE OPTIMIZING *************") // println("*********** COMPILER AST RIGHT BEFORE OPTIMIZING *************")
// printProgram(program) // printProgram(program)
if (compilationOptions.optimize) {
optimizeAst( optimizeAst(
program, program,
compilationOptions, compilationOptions,
@ -105,6 +107,7 @@ fun compileProgram(args: CompilerArguments): CompilationResult? {
BuiltinFunctionsFacade(BuiltinFunctions), BuiltinFunctionsFacade(BuiltinFunctions),
) )
} }
postprocessAst(program, args.errors, compilationOptions) postprocessAst(program, args.errors, compilationOptions)
// println("*********** COMPILER AST BEFORE ASSEMBLYGEN *************") // println("*********** COMPILER AST BEFORE ASSEMBLYGEN *************")
@ -120,16 +123,15 @@ fun compileProgram(args: CompilerArguments): CompilationResult? {
program.processAstBeforeAsmGeneration(compilationOptions, args.errors) program.processAstBeforeAsmGeneration(compilationOptions, args.errors)
args.errors.report() args.errors.report()
// println("*********** COMPILER AST RIGHT BEFORE ASM GENERATION *************") println("*********** COMPILER AST RIGHT BEFORE ASM GENERATION *************")
// printProgram(program) printProgram(program)
val intermediateAst = IntermediateAstMaker(program, args.errors).transform() val intermediateAst = IntermediateAstMaker(program, args.errors).transform()
// printAst(intermediateAst, true) { println(it) }
optimizeIntermediateAst(intermediateAst, compilationOptions, args.errors) optimizeIntermediateAst(intermediateAst, compilationOptions, args.errors)
args.errors.report() args.errors.report()
// println("*********** AST RIGHT BEFORE ASM GENERATION *************") println("*********** AST RIGHT BEFORE ASM GENERATION *************")
// printAst(intermediateAst, true, ::println) printAst(intermediateAst, true, ::println)
if(!createAssemblyAndAssemble(intermediateAst, args.errors, compilationOptions)) { if(!createAssemblyAndAssemble(intermediateAst, args.errors, compilationOptions)) {
System.err.println("Error in codegeneration or assembler") System.err.println("Error in codegeneration or assembler")

View File

@ -26,7 +26,6 @@ internal class CodeDesugarer(val program: Program, private val errors: IErrorRep
// - pointer[word] replaced by @(pointer+word) // - pointer[word] replaced by @(pointer+word)
// - @(&var) and @(&var+1) replaced by lsb(var) and msb(var) if var is a word // - @(&var) and @(&var+1) replaced by lsb(var) and msb(var) if var is a word
// - flatten chained assignments // - flatten chained assignments
// - rewrite chained comparisons like i<x<j into i<x and x<j
override fun before(breakStmt: Break, parent: Node): Iterable<IAstModification> { override fun before(breakStmt: Break, parent: Node): Iterable<IAstModification> {
fun jumpAfter(stmt: Statement): Iterable<IAstModification> { fun jumpAfter(stmt: Statement): Iterable<IAstModification> {
@ -260,29 +259,6 @@ _after:
return listOf(IAstModification.ReplaceNode(expr, squareCall, parent)) return listOf(IAstModification.ReplaceNode(expr, squareCall, parent))
} }
// desugar chained comparisons: i < x < j ---> i<x and x<j
// only if i<x or x<j was not written in parentheses! (i<x) < y, i < (x<y) -> leave untouched
if(expr.isChainedComparison()) {
val leftBinExpr = expr.left as? BinaryExpression
if(leftBinExpr!=null) {
if(!leftBinExpr.right.isSimple) {
errors.warn("possible multiple evaluation of subexpression in chained comparison, consider using a temporary variable", leftBinExpr.right.position)
}
val right = BinaryExpression(leftBinExpr.right.copy(), expr.operator, expr.right, leftBinExpr.right.position)
val desugar = BinaryExpression(leftBinExpr, "and", right, expr.position)
return listOf(IAstModification.ReplaceNode(expr, desugar, parent))
}
val rightBinExpr = expr.right as? BinaryExpression
if(rightBinExpr!=null) {
if(!rightBinExpr.left.isSimple) {
errors.warn("possible multiple evaluation of subexpression in chained comparison, consider using a temporary variable", rightBinExpr.left.position)
}
val left = BinaryExpression(expr.left, expr.operator, rightBinExpr.left.copy(), rightBinExpr.left.position)
val desugar = BinaryExpression(left, "and", rightBinExpr, expr.position)
return listOf(IAstModification.ReplaceNode(expr, desugar, parent))
}
}
return noModifications return noModifications
} }

View File

@ -35,7 +35,7 @@ sealed class Expression: Node {
is PrefixExpression -> is PrefixExpression ->
(other is PrefixExpression && other.operator==operator && other.expression isSameAs expression) (other is PrefixExpression && other.operator==operator && other.expression isSameAs expression)
is BinaryExpression -> { is BinaryExpression -> {
if(other !is BinaryExpression || other.operator!=operator || other.isChainedComparison()!=isChainedComparison()) if(other !is BinaryExpression || other.operator!=operator)
false false
else if(operator in AssociativeOperators) else if(operator in AssociativeOperators)
(other.left isSameAs left && other.right isSameAs right) || (other.left isSameAs right && other.right isSameAs left) (other.left isSameAs left && other.right isSameAs right) || (other.left isSameAs right && other.right isSameAs left)
@ -182,18 +182,6 @@ class BinaryExpression(
override val isSimple = false override val isSimple = false
fun isChainedComparison(): Boolean {
if(operator in ComparisonOperators) {
val leftBinExpr = left as? BinaryExpression
if (leftBinExpr != null && !leftBinExpr.insideParentheses && leftBinExpr.operator in ComparisonOperators)
return true
val rightBinExpr = right as? BinaryExpression
if (rightBinExpr != null && !rightBinExpr.insideParentheses && rightBinExpr.operator in ComparisonOperators)
return true
}
return false
}
// binary expression should actually have been optimized away into a single value, before const value was requested... // binary expression should actually have been optimized away into a single value, before const value was requested...
override fun constValue(program: Program): NumericLiteral? = null override fun constValue(program: Program): NumericLiteral? = null

View File

@ -592,7 +592,6 @@ postfix increment and decrement: ``++`` ``--``
comparison: ``==`` ``!=`` ``<`` ``>`` ``<=`` ``>=`` comparison: ``==`` ``!=`` ``<`` ``>`` ``<=`` ``>=``
Equality, Inequality, Less-than, Greater-than, Less-or-Equal-than, Greater-or-Equal-than comparisons. Equality, Inequality, Less-than, Greater-than, Less-or-Equal-than, Greater-or-Equal-than comparisons.
The result is a boolean value 'true' or 'false' (1 or 0). The result is a boolean value 'true' or 'false' (1 or 0).
Note that you can chain comparisons like so: ``i < x < j``, which is the same as ``i<x and x<j``.
logical: ``not`` ``and`` ``or`` ``xor`` logical: ``not`` ``and`` ``or`` ``xor``
These operators are the usual logical operations that are part of a logical expression to reason These operators are the usual logical operations that are part of a logical expression to reason

View File

@ -1,17 +1,6 @@
TODO TODO
==== ====
- Revert or fix current "desugar chained comparisons" it causes problems with if statements (in optimizer).
ubyte @shared n=20
ubyte @shared L1=10
ubyte @shared L2=100
if n < L1 {
;txt.print("bing")
} else {
txt.print("boom") ; no longer triggers
}
... ...
@ -87,3 +76,6 @@ Other language/syntax features to think about
- add (rom/ram)bank support to romsub. A call will then automatically switch banks, use callfar and something else when in banked ram. - add (rom/ram)bank support to romsub. A call will then automatically switch banks, use callfar and something else when in banked ram.
challenges: how to not make this too X16 specific? How does the compiler know what bank to switch (ram/rom)? challenges: how to not make this too X16 specific? How does the compiler know what bank to switch (ram/rom)?
How to make it performant when we want to (i.e. NOT have it use callfar/auto bank switching) ? How to make it performant when we want to (i.e. NOT have it use callfar/auto bank switching) ?
- chained comparisons `10<x<20` , `x==y==z` (desugars to `10<x and x<20`, `x==y and y==z`)
BUT this needs a new AST node type and rewritten parser rules, because otherwise it changes the semantics
of existing expressions such as if x<y==0 ...