[
https://issues.apache.org/jira/browse/DAFFODIL-2473?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17338432#comment-17338432
]
Mike Beckerle commented on DAFFODIL-2473:
-----------------------------------------
Pretty close. let's just look at the shifts first.
Best way to get feedback on code is to create a pull request, if you are just
looking for initial feedback just name it with "WIP: ..." for work-in-progress.
The shift operations take two arguments, so should extend FNTwoArgs. The 2nd
argument is the shift count, always an unsignedInt.
I suggest we define the logical shifts, if the shiftCount is greater than the
width of the integer, the result is 0. For arithmetic shift right if the type
is signed, and the value is negative, then arithmetic shift right will
sign-extend the sign bit, so any shift equal to or greater than the integer
width will produce -1 as the value.
In your code in the comment thread above, you don't need the first cases for
NodeInfo.Decimal nor float nor double, as those will be picked up by your final
`case _ => ...`
Your error message should say "leftShift not supported for type".
Outside of the logic to do the operation which you have above, there is also
informing the Daffodil schema compiler about the new functions.
If you look at the definition of FNRound, that is defined for numeric types by
using a class called FNOneArgMathExpr which has member inherentType and method
targetTypeForSubexpression which inform the Daffodil schema compiler what the
constraints are on the argument and result type for the function.
We need to do something similar, but inform the Daffodil schema compiler that
your shifts are defined for two args, the first of which is either
unsignedLong, signed long, or the types derived from those. The second arg is
always an unsignedInt.
So I think you should copy the FNTwoArgsMathExpr case class and modify to
create case class DFDLXShiftExpr
DFDLX is used here not FN, because these functions are daffodil extensions of
the DFDL standard, so will go in a namespace we normally bind the "dfdlx"
prefix to. We will propose these functions for eventual inclusion in the DFDL
specification at which point we'll put them in the regular dfdl namespace.
That case class will extend FunctionCallBase, and must provide methods that
supply type info for the Daffodil schema compiler like this:
{code:java}
case class DFDLXShiftExpr(nameAsParsed: String, fnQName: RefQName,
args: List[Expression], constructor: List[CompiledDPath] => RecipeOp)
extends FunctionCallBase(nameAsParsed, fnQName, args) {
/**
* Gives the inherent type of the result. This should match the type of the
first
* argument, which we check here must be some subtype of unsignedLong or signed
long.
*/
override lazy val inherentType = {
val argInherentType = args(0).inherentType
checkArgCount(2)
schemaDefinitionUnless(
argInherentType.isSubtypeOf(NodeInfo.PrimType.UnsignedLong) ||
argInherentType.isSubtypeOf(Nodenfo.PrimType.Long),
"First argument must be either unsignedLong, signed long, or a subtype of
those, but was %s.",
argInherentType)
argInherentType
}
/**
* Informs the schema compiler what the expressions for args should be required
* to compile into. When this doesn't match the inherent type of the argument
expression
* then daffodil may insert a conversion (if one is defined for the two types).
*/
override def targetTypeForSubexpression(subexp: Expression): NodeInfo.Kind = {
if (subexp == args(0))
inherentType // above that is constrained to be long or unsignedLong or
derived from those
else if (subexp == args(1))
NodeInfo.UnsignedInt // 2nd argument is always unsignedInt.
else
Assert.invariantFailed("subexpression %s is not an
argument.".format(subexp))
}
}{code}
(I can't guarantee that compiles. I just did it in this editor.)
Given the above, these would plug into the expression compilation if you look
for this in Expression.scala:
{code:java}
case (RefQName(_, "round", FUNC), args) =>
FNOneArgMathExpr(functionQNameString, functionQName, args,
FNRound(_, _))
{code}
Your shifts would plug in similarly like:
{code:java}
case (RefQName(_, "leftShift", DFDLX), args) =>
DFDLXShiftExpr(functionQNameString, functionQName, args,
DFDLXLeftShift(_, _))
{code}
Similarly for DFDLXRightShift, DFDLXArithmeticRightShift. (Note here I renamed
your FNLeftShift to DFDLXLeftShift.)
Next comment will be about testing.
> Need bit-wise AND, OR, NOT, and shift operations
> ------------------------------------------------
>
> Key: DAFFODIL-2473
> URL: https://issues.apache.org/jira/browse/DAFFODIL-2473
> Project: Daffodil
> Issue Type: New Feature
> Components: Back End, Front End
> Affects Versions: 3.0.0
> Reporter: Mike Beckerle
> Priority: Critical
>
> There are a number of data representations DFDL cannot handle because it
> cannot manipulate numbers with sufficient flexibility in expressions.
> We need AND, OR, NOT, maybe XOR, and shift left, shift right, and probably
> arithmetic shift right (which does sign-extension of signed numbers). We
> should add functions for the whole set of them.
> On signed numbers with specific sizes (long, int, short, byte) these should
> behave as if the data was binary in 2s complement representation, i.e., as if
> the most-significant-bit was playing the role of 2s complement sign.
> These operations should all be undefined (Schema Def Error) on decimal,
> integer, and non-negative integer values, as well as float and double.
> Specifically, these operations are defined on long and its subtypes, and
> unsignedLong and its subtypes only.
>
>
>
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
