Add shift operations for numeric values#995
Draft
abh80 wants to merge 18 commits intonasa:feature/shift-opfrom
Draft
Add shift operations for numeric values#995abh80 wants to merge 18 commits intonasa:feature/shift-opfrom
abh80 wants to merge 18 commits intonasa:feature/shift-opfrom
Conversation
Contributor
Author
/** For shifting operations make sure both values are integer */
private def convertToInteger(loc: Location, t: Type): Result.Result[Type] =
if (t.isInt) Right(t)
else if (t.isConvertibleTo(Type.Integer)) Right(Type.Integer)
else {
val error = SemanticError.InvalidType(loc, s"cannot convert $t to an integer type")
Left(error)
} The current implementation of int value check is too much permissive, since float is convertible to int. I will completely remove the conditional check. This causes the failure: https://github.com/abh80/fpp/blob/564f25468d2be477626cae34c49a0d5d354327f5/compiler/tools/fpp-check/test/numeric_shift/shift_float_value_error.ref.txt#L1 Edit: Looks like there is another bug t <- a.commonType(e.e1.id, e.e2.id, loc)
_ <- e.op match {
case Ast.Binop.Shift(_) => convertToInteger(loc, t)
...
}The common type inference automatically promotes to Float even if one value is int which causes misleading error information. However this dosent affect the intended behavior since it's only applicable if both values are integer |
Contributor
Author
|
@bocchino I have completed the implementation and testing. Can you give the initial review before I start writing the documentation? |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Added:
Bit shift operations to numeric values
Closes #922
Tasks
Implementation
Token.scala: addedLSHIFTandRSHIFTtokens.Lexer.scala: added token parsing for new tokens.Ast.scala:ShiftTypeto encapsulate shift typesLShift extends ShiftType .....case class Shift(op: ShiftType) extends Binopto Binop.Parser.scala:expr binopnode creation for new shift tokens.analysis/CheckSemantics/CheckExprTypes.scala:convertToIntegerfor shift operations (ignoring float values).Ast.Binop.Shift(_).Value.scala:intShiftOp = (BigInt, Int) => BigIntfor shifting.private[analysis] def intShiftOp(op: Value.Binop.intShiftOp)(v: Value)(since generic binop does not support(BigInt, Int) => Bigint))Value:isNegativeandisValidShiftAmountto help inAnalysis.scala.intShiftOpmethod inPrimitiveInt,IntegerandEnumConstant.<<and>>methods toValueto perform shift operations.Analysis.scala: addedlshiftandrshiftmethod, using the new methods added toValue.CheckSemantics/EvalConstantExprs.scala: add the new caseAst.Binop.Shift(op)using the new methods added toAnalysis.util/Error.scala: Added a newInvalidShiftAmounterror toSemanticError.Tests
compiler/tools/fpp-check/test/numeric_shift: added robust test cases including edge cases and error checks.compiler/lib/src/test/input/syntax/lexer/ok/symbols.fpp: added the new<</>>symbols.fpp-syntaxtests.fpp-to-cpptests.ValueSpectests.Spec: