From 65ba91411d21723dc769adaacd64525996ad62f1 Mon Sep 17 00:00:00 2001 From: Irmen de Jong Date: Thu, 20 Aug 2020 12:18:42 +0200 Subject: [PATCH] improved function arg type checking and error message --- compiler/res/version.txt | 2 +- compiler/src/prog8/ast/AstToplevel.kt | 4 +- compiler/src/prog8/ast/antlr/Antr2Kotlin.kt | 2 +- .../src/prog8/ast/processing/AstChecker.kt | 94 ++++++------------- .../ast/processing/VerifyFunctionArgTypes.kt | 40 ++++---- .../src/prog8/functions/BuiltinFunctions.kt | 6 +- examples/test.p8 | 65 ++++++++----- 7 files changed, 97 insertions(+), 116 deletions(-) diff --git a/compiler/res/version.txt b/compiler/res/version.txt index 8c50098d8..5606f1c30 100644 --- a/compiler/res/version.txt +++ b/compiler/res/version.txt @@ -1 +1 @@ -3.1 +3.2-SNAPSHOT diff --git a/compiler/src/prog8/ast/AstToplevel.kt b/compiler/src/prog8/ast/AstToplevel.kt index d737e8252..602890722 100644 --- a/compiler/src/prog8/ast/AstToplevel.kt +++ b/compiler/src/prog8/ast/AstToplevel.kt @@ -314,9 +314,9 @@ class GlobalNamespace(val modules: List): Node, INameScope { } // lookup something from the module. return when (val stmt = localContext.definingModule().lookup(scopedName, localContext)) { - is Label, is VarDecl, is Block, is Subroutine -> stmt + is Label, is VarDecl, is Block, is Subroutine, is StructDecl -> stmt null -> null - else -> throw SyntaxError("wrong identifier target for $scopedName: $stmt", stmt.position) + else -> throw SyntaxError("invalid identifier target type", stmt.position) } } } diff --git a/compiler/src/prog8/ast/antlr/Antr2Kotlin.kt b/compiler/src/prog8/ast/antlr/Antr2Kotlin.kt index ebec76c0e..5e690f74b 100644 --- a/compiler/src/prog8/ast/antlr/Antr2Kotlin.kt +++ b/compiler/src/prog8/ast/antlr/Antr2Kotlin.kt @@ -301,7 +301,7 @@ private fun prog8Parser.Asmsub_paramsContext.toAst(): List registerorpair = RegisterOrPair.valueOf(name) in Statusflag.names -> statusregister = Statusflag.valueOf(name) - else -> throw FatalAstException("invalid register or status flag in $it") + else -> throw FatalAstException("invalid register or status flag '$name'") } } AsmSubroutineParameter(vardecl.varname.text, datatype, registerorpair, statusregister, diff --git a/compiler/src/prog8/ast/processing/AstChecker.kt b/compiler/src/prog8/ast/processing/AstChecker.kt index e8e4893cc..1f27f377f 100644 --- a/compiler/src/prog8/ast/processing/AstChecker.kt +++ b/compiler/src/prog8/ast/processing/AstChecker.kt @@ -304,7 +304,7 @@ internal class AstChecker(private val program: Program, } else { // Pass-by-reference datatypes can not occur as parameters to a subroutine directly // Instead, their reference (address) should be passed (as an UWORD). - // TODO The language has no typed pointers at this time! This should be handled better. + // TODO The language has no (typed) pointers at this time. Should this be handled better? if(subroutine.parameters.any{it.type in PassByReferenceDatatypes }) { err("Pass-by-reference types (str, array) cannot occur as a parameter type directly. Instead, use an uword to receive their address, or access the variable from the outer scope directly.") } @@ -863,8 +863,9 @@ internal class AstChecker(private val program: Program, } val error = VerifyFunctionArgTypes.checkTypes(functionCallStatement, functionCallStatement.definingScope(), program) - if(error!=null) - errors.err(error, functionCallStatement.args.first().position) + if(error!=null) { + errors.err(error, functionCallStatement.args.firstOrNull()?.position ?: functionCallStatement.position) + } super.visit(functionCallStatement) } @@ -874,79 +875,38 @@ internal class AstChecker(private val program: Program, errors.err("cannot use arguments when calling a label", position) if(target is BuiltinFunctionStatementPlaceholder) { - // it's a call to a builtin function. - val func = BuiltinFunctions.getValue(target.name) - if(args.size!=func.parameters.size) - errors.err("invalid number of arguments", position) - else { - val paramTypesForAddressOf = PassByReferenceDatatypes + DataType.UWORD - for (arg in args.withIndex().zip(func.parameters)) { - val argDt=arg.first.value.inferType(program) - if (argDt.isKnown - && !(argDt.typeOrElse(DataType.STRUCT) isAssignableTo arg.second.possibleDatatypes) - && (argDt.typeOrElse(DataType.STRUCT) != DataType.UWORD || arg.second.possibleDatatypes.intersect(paramTypesForAddressOf).isEmpty())) { - errors.err("builtin function '${target.name}' argument ${arg.first.index + 1} has invalid type $argDt, expected ${arg.second.possibleDatatypes}", position) - } + if(target.name=="swap") { + // swap() is a bit weird because this one is translated into a operations directly, instead of being a function call + val dt1 = args[0].inferType(program) + val dt2 = args[1].inferType(program) + if (dt1 != dt2) + errors.err("swap requires 2 args of identical type", position) + else if (args[0].constValue(program) != null || args[1].constValue(program) != null) + errors.err("swap requires 2 variables, not constant value(s)", position) + else if(args[0] isSameAs args[1]) + errors.err("swap should have 2 different args", position) + else if(dt1.typeOrElse(DataType.STRUCT) !in NumericDatatypes) + errors.err("swap requires args of numerical type", position) + } + else if(target.name=="all" || target.name=="any") { + if((args[0] as? AddressOf)?.identifier?.targetVarDecl(program.namespace)?.datatype == DataType.STR) { + errors.err("any/all on a string is useless (is always true unless the string is empty)", position) } - if(target.name=="swap") { - // swap() is a bit weird because this one is translated into a operations directly, instead of being a function call - val dt1 = args[0].inferType(program) - val dt2 = args[1].inferType(program) - if (dt1 != dt2) - errors.err("swap requires 2 args of identical type", position) - else if (args[0].constValue(program) != null || args[1].constValue(program) != null) - errors.err("swap requires 2 variables, not constant value(s)", position) - else if(args[0] isSameAs args[1]) - errors.err("swap should have 2 different args", position) - else if(dt1.typeOrElse(DataType.STRUCT) !in NumericDatatypes) - errors.err("swap requires args of numerical type", position) - } - else if(target.name=="all" || target.name=="any") { - if((args[0] as? AddressOf)?.identifier?.targetVarDecl(program.namespace)?.datatype == DataType.STR) { - errors.err("any/all on a string is useless (is always true unless the string is empty)", position) - } - if(args[0].inferType(program).typeOrElse(DataType.STR) == DataType.STR) { - errors.err("any/all on a string is useless (is always true unless the string is empty)", position) - } + if(args[0].inferType(program).typeOrElse(DataType.STR) == DataType.STR) { + errors.err("any/all on a string is useless (is always true unless the string is empty)", position) } } } else if(target is Subroutine) { if(target.regXasResult()) errors.warn("subroutine call return value in X register is discarded and replaced by 0", position) - if(args.size!=target.parameters.size) - errors.err("invalid number of arguments", position) - else { + if(target.isAsmSubroutine) { for (arg in args.withIndex().zip(target.parameters)) { val argIDt = arg.first.value.inferType(program) - if(!argIDt.isKnown) { + if (!argIDt.isKnown) return - } - val argDt=argIDt.typeOrElse(DataType.STRUCT) - if(!(argDt isAssignableTo arg.second.type)) { - // for asm subroutines having STR param it's okay to provide a UWORD (address value) - if(!(target.isAsmSubroutine && arg.second.type == DataType.STR && argDt == DataType.UWORD)) - errors.err("subroutine '${target.name}' argument ${arg.first.index + 1} has invalid type $argDt, expected ${arg.second.type}", position) - } - - if(target.isAsmSubroutine) { - if (target.asmParameterRegisters[arg.first.index].registerOrPair in setOf(RegisterOrPair.AX, RegisterOrPair.XY, RegisterOrPair.X)) { - if (arg.first.value !is NumericLiteralValue && arg.first.value !is IdentifierReference) - errors.warn("calling a subroutine that expects X as a parameter is problematic. If you see a compiler error/crash about this later, try to change this call", position) - } - - // check if the argument types match the register(pairs) - val asmParamReg = target.asmParameterRegisters[arg.first.index] - if(asmParamReg.statusflag!=null) { - if(argDt !in ByteDatatypes) - errors.err("subroutine '${target.name}' argument ${arg.first.index + 1} must be byte type for statusflag", position) - } else if(asmParamReg.registerOrPair in setOf(RegisterOrPair.A, RegisterOrPair.X, RegisterOrPair.Y)) { - if(argDt !in ByteDatatypes) - errors.err("subroutine '${target.name}' argument ${arg.first.index + 1} must be byte type for single register", position) - } else if(asmParamReg.registerOrPair in setOf(RegisterOrPair.AX, RegisterOrPair.AY, RegisterOrPair.XY)) { - if(argDt !in WordDatatypes + IterableDatatypes) - errors.err("subroutine '${target.name}' argument ${arg.first.index + 1} must be word type for register pair", position) - } - } + if (target.asmParameterRegisters[arg.first.index].registerOrPair in setOf(RegisterOrPair.AX, RegisterOrPair.XY, RegisterOrPair.X) + && arg.first.value !is NumericLiteralValue && arg.first.value !is IdentifierReference) + errors.warn("calling a subroutine that expects X as a parameter is problematic. If you see a compiler error/crash about this later, try to change this call", position) } } } diff --git a/compiler/src/prog8/ast/processing/VerifyFunctionArgTypes.kt b/compiler/src/prog8/ast/processing/VerifyFunctionArgTypes.kt index 58f5778a6..5bd69758e 100644 --- a/compiler/src/prog8/ast/processing/VerifyFunctionArgTypes.kt +++ b/compiler/src/prog8/ast/processing/VerifyFunctionArgTypes.kt @@ -29,24 +29,32 @@ class VerifyFunctionArgTypes(val program: Program) : IAstVisitor { fun checkTypes(call: IFunctionCall, scope: INameScope, program: Program): String? { val argtypes = call.args.map { it.inferType(program).typeOrElse(DataType.STRUCT) } val target = call.target.targetStatement(scope) - when (target) { - is Subroutine -> { - val paramtypes = target.parameters.map { it.type } - val mismatch = argtypes.zip(paramtypes).indexOfFirst { it.first != it.second} - if(mismatch>=0) - return "argument ${mismatch+1} type mismatch" - } - is BuiltinFunctionStatementPlaceholder -> { - val func = BuiltinFunctions.getValue(target.name) - val paramtypes = func.parameters.map { it.possibleDatatypes } - for (x in argtypes.zip(paramtypes).withIndex()) { - if (x.value.first !in x.value.second) - return "argument ${x.index+1} type mismatch" - } - } - else -> { + if (target is Subroutine) { + // asmsub types are not checked specifically at this time + if(call.args.size != target.parameters.size) + return "invalid number of arguments" + val paramtypes = target.parameters.map { it.type } + val mismatch = argtypes.zip(paramtypes).indexOfFirst { it.first != it.second} + if(mismatch>=0) { + val actual = argtypes[mismatch].toString() + val expected = paramtypes[mismatch].toString() + return "argument ${mismatch + 1} type mismatch, was: $actual expected: $expected" } } + else if (target is BuiltinFunctionStatementPlaceholder) { + val func = BuiltinFunctions.getValue(target.name) + if(call.args.size != func.parameters.size) + return "invalid number of arguments" + val paramtypes = func.parameters.map { it.possibleDatatypes } + for (x in argtypes.zip(paramtypes).withIndex()) { + if (x.value.first !in x.value.second) { + val actual = x.value.first.toString() + val expected = x.value.second.toString() + return "argument ${x.index + 1} type mismatch, was: $actual expected: $expected" + } + } + } + return null } } diff --git a/compiler/src/prog8/functions/BuiltinFunctions.kt b/compiler/src/prog8/functions/BuiltinFunctions.kt index da8cf55c2..764c4c7ed 100644 --- a/compiler/src/prog8/functions/BuiltinFunctions.kt +++ b/compiler/src/prog8/functions/BuiltinFunctions.kt @@ -254,9 +254,6 @@ private fun builtinLen(args: List, position: Position, program: Prog // note: in some cases the length is > 255 and then we have to return a UWORD type instead of a UBYTE. if(args.size!=1) throw SyntaxError("len requires one argument", position) - val constArg = args[0].constValue(program) - if(constArg!=null) - throw SyntaxError("len of weird argument ${args[0]}", position) val directMemVar = ((args[0] as? DirectMemoryRead)?.addressExpression as? IdentifierReference)?.targetVarDecl(program.namespace) var arraySize = directMemVar?.arraysize?.size() @@ -286,7 +283,8 @@ private fun builtinLen(args: List, position: Position, program: Prog val refLv = target.value as StringLiteralValue NumericLiteralValue.optimalInteger(refLv.value.length, args[0].position) } - in NumericDatatypes -> throw SyntaxError("len of weird argument ${args[0]}", position) + DataType.STRUCT -> throw SyntaxError("cannot use len on struct, did you mean sizeof?", args[0].position) + in NumericDatatypes -> throw SyntaxError("cannot use len on numeric value, did you mean sizeof?", args[0].position) else -> throw CompilerException("weird datatype") } } diff --git a/examples/test.p8 b/examples/test.p8 index 42a3336b6..66a7f353c 100644 --- a/examples/test.p8 +++ b/examples/test.p8 @@ -6,36 +6,51 @@ main { sub start() { - uword count=0 - - count=0 - repeat 10 { - count++ + struct Color { + ubyte red + uword green + float blue } - c64scr.print_uw(count) - c64.CHROUT('\n') - count=0 - repeat 255 { - count++ - } - c64scr.print_uw(count) - c64.CHROUT('\n') + Color c = [11,22222,3.1234] - count=0 - repeat 256 { - count++ - } - c64scr.print_uw(count) - c64.CHROUT('\n') +; c64scr.print_ub(c.red) +; c64.CHROUT('\n') +; c64scr.print_uw(c.green) +; c64.CHROUT('\n') +; c64flt.print_f(c.blue) +; c64.CHROUT('\n') - count=0 - repeat 40000 { - count++ - } - c64scr.print_uw(count) - c64.CHROUT('\n') + uword xx = 4.5678 + ubyte bb = 33 + foo2(-33) + foo2(bb) + foo2(4.55) ; TODO truncation warning + + ;foo("zzz", 8.777) + ;len(13) + +; uword size = len(Color) +; c64scr.print_uw(size) +; c64.CHROUT('\n') + +; c64scr.print_ub(len(Color)) +; c64.CHROUT('\n') +; c64scr.print_ub(len(c)) +; c64.CHROUT('\n') +; c64scr.print_ub(len(c.green)) +; c64.CHROUT('\n') + } + + sub foo(ubyte aa, word ww) { + ww += aa + } + + asmsub foo2(ubyte aa @Pc) { + %asm {{ + rts + }} } }