Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Introduce OneToManyValueExpression and OneToOneValueExpression #247

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions core/src/main/java/io/parsingdata/metal/Shorthand.java
Original file line number Diff line number Diff line change
Expand Up @@ -44,14 +44,14 @@
import io.parsingdata.metal.expression.value.Cat;
import io.parsingdata.metal.expression.value.Const;
import io.parsingdata.metal.expression.value.ConstantFactory;
import io.parsingdata.metal.expression.value.OneToManyValueExpression;
import io.parsingdata.metal.expression.value.reference.CurrentIteration;
import io.parsingdata.metal.expression.value.Elvis;
import io.parsingdata.metal.expression.value.Expand;
import io.parsingdata.metal.expression.value.FoldLeft;
import io.parsingdata.metal.expression.value.FoldRight;
import io.parsingdata.metal.expression.value.FoldCat;
import io.parsingdata.metal.expression.value.Reverse;
import io.parsingdata.metal.expression.value.UnaryValueExpression;
import io.parsingdata.metal.expression.value.Value;
import io.parsingdata.metal.expression.value.ValueExpression;
import io.parsingdata.metal.expression.value.arithmetic.Add;
Expand Down Expand Up @@ -164,10 +164,10 @@ private Shorthand() {}
public static BinaryValueExpression mul(final ValueExpression left, final ValueExpression right) { return new Mul(left, right); }
public static BinaryValueExpression sub(final ValueExpression left, final ValueExpression right) { return new Sub(left, right); }
public static BinaryValueExpression mod(final ValueExpression left, final ValueExpression right) { return new Mod(left, right); }
public static UnaryValueExpression neg(final ValueExpression operand) { return new Neg(operand); }
public static OneToManyValueExpression neg(final ValueExpression operand) { return new Neg(operand); }
public static BinaryValueExpression and(final ValueExpression left, final ValueExpression right) { return new io.parsingdata.metal.expression.value.bitwise.And(left, right); }
public static BinaryValueExpression or(final ValueExpression left, final ValueExpression right) { return new io.parsingdata.metal.expression.value.bitwise.Or(left, right); }
public static UnaryValueExpression not(final ValueExpression operand) { return new io.parsingdata.metal.expression.value.bitwise.Not(operand); }
public static OneToManyValueExpression not(final ValueExpression operand) { return new io.parsingdata.metal.expression.value.bitwise.Not(operand); }
Copy link
Collaborator

@ccreeten ccreeten Aug 30, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Referring to your naming debate, reading this does make it seem a bit funny in that it now reads like negate/not have a different cardinality between input and output (consider an identity function, it would be even more weird in my opinion), although both are simply (unbounded) ValueExpressions. Before, Unary meant a single operand, with implicitly a single output value (as always). Now I think it is a mix of two concepts.

But thinking of a good name is hard. UnaryManyToMany to me feels like I would know better what it all is about. But there might be better names to be found. Or I am just thinking about it wrong. 🆘

EDIT: as another example, last being a OneToOne just does not quite feel right to me (many to one?).

Copy link
Contributor

@jvdb jvdb Aug 30, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree that because of our list semantics, the notion of what is a single operand is a bit complex... We should probably approach this in a more fundamental way: think about the possible types, then the ones that actually occur, then which ones we should implement/support and after all that, how to name them.

Copy link
Collaborator

@ccreeten ccreeten Aug 31, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good :) It is also interesting for expressions with more than one operand, where e.g. mul can have two lists, but exp can have a list as the first operand, but requires a single element as the second.

It would probably be cool to encode that information in the operand types, i.e. BinaryExpr&ToManyExpr exp(ToManyExpr value, ToOneExpr second). It can then allow us to statically check for correctness as well, as per #70 (although there are few cases where it is necessary?). Downside is that we can not pass a list expression where we know that it will only ever contain a single value, so in the exp case, it would need a wrap with a last for example.

And there are probably other things to consider.

public static BinaryValueExpression shl(final ValueExpression left, final ValueExpression right) { return new ShiftLeft(left, right); }
public static BinaryValueExpression shr(final ValueExpression left, final ValueExpression right) { return new ShiftRight(left, right); }
public static ValueExpression con(final long value) { return con(value, new Encoding()); }
Expand Down
4 changes: 2 additions & 2 deletions core/src/main/java/io/parsingdata/metal/Util.java
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
import io.parsingdata.metal.data.ParseState;
import io.parsingdata.metal.data.Slice;
import io.parsingdata.metal.encoding.Encoding;
import io.parsingdata.metal.expression.value.UnaryValueExpression;
import io.parsingdata.metal.expression.value.OneToManyValueExpression;
import io.parsingdata.metal.expression.value.Value;
import io.parsingdata.metal.expression.value.ValueExpression;

Expand Down Expand Up @@ -85,7 +85,7 @@ public static String bytesToHexString(final byte[] bytes) {
}

public static ValueExpression inflate(final ValueExpression target) {
return new UnaryValueExpression(target) {
return new OneToManyValueExpression(target) {
@Override
public Optional<Value> eval(final Value value, final ParseState parseState, final Encoding encoding) {
final Inflater inf = new Inflater(true);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@
* handling the case of evaluating two values. This base class takes care of
* evaluating the operands and handling list semantics.
*
* @see UnaryValueExpression
* @see OneToManyValueExpression
*/
public abstract class BinaryValueExpression implements ValueExpression {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,8 @@

import static io.parsingdata.metal.data.Slice.createFromSource;

import java.util.Objects;
import java.util.Optional;

import io.parsingdata.metal.Util;
import io.parsingdata.metal.data.ConcatenatedValueSource;
import io.parsingdata.metal.data.ImmutableList;
import io.parsingdata.metal.data.ParseState;
Expand All @@ -36,36 +34,17 @@
* @see FoldLeft
* @see Cat
*/
public class FoldCat implements ValueExpression {

public final ValueExpression operand;
public class FoldCat extends OneToOneValueExpression {

public FoldCat(final ValueExpression operand) {
this.operand = operand;
}

@Override
public ImmutableList<Optional<Value>> eval(final ParseState parseState, final Encoding encoding) {
return ConcatenatedValueSource.create(operand.eval(parseState, encoding))
.flatMap(source -> createFromSource(source, ZERO, source.length))
.map(slice -> new ImmutableList<Optional<Value>>().add(Optional.of(new Value(slice, encoding))))
.orElseGet(() -> ImmutableList.create(Optional.empty()));
}

@Override
public String toString() {
return getClass().getSimpleName() + "(" + operand + ")";
}

@Override
public boolean equals(final Object obj) {
return Util.notNullAndSameClass(this, obj)
&& Objects.equals(operand, ((FoldCat)obj).operand);
super(operand);
}

@Override
public int hashCode() {
return Objects.hash(getClass(), operand);
public Optional<Value> eval(final ImmutableList<Optional<Value>> list, final ParseState parseState, final Encoding encoding) {
return ConcatenatedValueSource.create(list)
.flatMap(source -> createFromSource(source, ZERO, source.length))
.map(slice -> new Value(slice, encoding));
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -31,25 +31,26 @@
import io.parsingdata.metal.encoding.Encoding;

/**
* Base class for {@link ValueExpression}s with one operand.
* Base class for {@link ValueExpression}s with one operand that evaluates
* to multiple values.
* <p>
* A UnaryValueExpression implements a ValueExpression that has one
* A OneToManyValueExpression implements a ValueExpression that has one
* <code>operand</code> (a {@link ValueExpression}). The operand is first
* evaluated. If it evaluates to {@link Optional#empty()}, the result of the
* ValueExpression itself will be that as well.
* <p>
* To implement a UnaryValueExpression, only the
* To implement a OneToManyValueExpression, only the
* {@link #eval(Value, ParseState, Encoding)} must be implemented, handling
* the case of evaluating one value. This base class takes care of evaluating
* the operand and handling list semantics.
*
* @see BinaryValueExpression
*/
public abstract class UnaryValueExpression implements ValueExpression {
public abstract class OneToManyValueExpression implements ValueExpression {

public final ValueExpression operand;

public UnaryValueExpression(final ValueExpression operand) {
public OneToManyValueExpression(final ValueExpression operand) {
this.operand = checkNotNull(operand, "operand");
}

Expand All @@ -75,7 +76,7 @@ public String toString() {
@Override
public boolean equals(final Object obj) {
return Util.notNullAndSameClass(this, obj)
&& Objects.equals(operand, ((UnaryValueExpression)obj).operand);
&& Objects.equals(operand, ((OneToManyValueExpression)obj).operand);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
/*
* Copyright 2013-2016 Netherlands Forensic Institute
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package io.parsingdata.metal.expression.value;

import static io.parsingdata.metal.Util.checkNotNull;

import java.util.Objects;
import java.util.Optional;

import io.parsingdata.metal.Util;
import io.parsingdata.metal.data.ImmutableList;
import io.parsingdata.metal.data.ParseState;
import io.parsingdata.metal.encoding.Encoding;

/**
* Base class for {@link ValueExpression}s with one operand that evaluates
* to a single value.
* <p>
* A OneToOneValueExpression implements a ValueExpression that has one
* <code>operand</code> (a {@link ValueExpression}). The operand is first
* evaluated. If it evaluates to {@link Optional#empty()}, the result of the
* ValueExpression itself will be that as well.
* <p>
* To implement a OneToOneValueExpression, only the
* {@link #eval(ImmutableList, ParseState, Encoding)} must
* be implemented, handling the case of evaluating a list of values. This
* base class takes care of evaluating the operand and handling list
* semantics.
*
* @see BinaryValueExpression
*/
public abstract class OneToOneValueExpression implements ValueExpression {

public final ValueExpression operand;

public OneToOneValueExpression(final ValueExpression operand) {
this.operand = checkNotNull(operand, "operand");
}

@Override
public ImmutableList<Optional<Value>> eval(final ParseState parseState, final Encoding encoding) {
return ImmutableList.create(eval(operand.eval(parseState, encoding), parseState, encoding));
}

public abstract Optional<Value> eval(final ImmutableList<Optional<Value>> list, final ParseState parseState, final Encoding encoding);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Self-review: I think we do not need to pass ParseState into this method, we do not use it anywhere.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jvdb @mvanaken I think that in both this new class as in OneToManyValueExpression, there is no need to have the ParseState argument. We never access it anywhere. Even the Encoding is rarely accessed; I found only a single use of it in the code I refactored here.


@Override
public String toString() {
return getClass().getSimpleName() + "(" + operand + ")";
}

@Override
public boolean equals(final Object obj) {
return Util.notNullAndSameClass(this, obj)
&& Objects.equals(operand, ((OneToOneValueExpression)obj).operand);
}

@Override
public int hashCode() {
return Objects.hash(getClass(), operand);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,14 @@
import io.parsingdata.metal.data.ParseState;
import io.parsingdata.metal.encoding.Encoding;
import io.parsingdata.metal.expression.value.ConstantFactory;
import io.parsingdata.metal.expression.value.UnaryValueExpression;
import io.parsingdata.metal.expression.value.OneToManyValueExpression;
import io.parsingdata.metal.expression.value.Value;
import io.parsingdata.metal.expression.value.ValueExpression;

/**
* A {@link UnaryValueExpression} that implements integer negation.
* A {@link OneToManyValueExpression} that implements integer negation.
*/
public class Neg extends UnaryValueExpression {
public class Neg extends OneToManyValueExpression {

public Neg(final ValueExpression operand) {
super(operand);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,14 @@
import io.parsingdata.metal.data.ParseState;
import io.parsingdata.metal.encoding.Encoding;
import io.parsingdata.metal.expression.value.ConstantFactory;
import io.parsingdata.metal.expression.value.UnaryValueExpression;
import io.parsingdata.metal.expression.value.OneToManyValueExpression;
import io.parsingdata.metal.expression.value.Value;
import io.parsingdata.metal.expression.value.ValueExpression;

/**
* A {@link UnaryValueExpression} that implements the bitwise NOT operator.
* A {@link OneToManyValueExpression} that implements the bitwise NOT operator.
*/
public class Not extends UnaryValueExpression {
public class Not extends OneToManyValueExpression {

public Not(final ValueExpression operand) {
super(operand);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,56 +16,34 @@

package io.parsingdata.metal.expression.value.reference;

import static io.parsingdata.metal.Util.checkNotNull;

import java.util.Objects;
import java.util.Optional;

import io.parsingdata.metal.Util;
import io.parsingdata.metal.data.ImmutableList;
import io.parsingdata.metal.data.ParseState;
import io.parsingdata.metal.encoding.Encoding;
import io.parsingdata.metal.encoding.Sign;
import io.parsingdata.metal.expression.value.ConstantFactory;
import io.parsingdata.metal.expression.value.OneToOneValueExpression;
import io.parsingdata.metal.expression.value.Value;
import io.parsingdata.metal.expression.value.ValueExpression;

/**
* A {@link ValueExpression} that represents the amount of {@link Value}s
* returned by evaluating its <code>operand</code>.
*/
public class Count implements ValueExpression {

public final ValueExpression operand;
public class Count extends OneToOneValueExpression {

public Count(final ValueExpression operand) {
this.operand = checkNotNull(operand, "operand");
super(operand);
}

@Override
public ImmutableList<Optional<Value>> eval(final ParseState parseState, final Encoding encoding) {
final ImmutableList<Optional<Value>> values = operand.eval(parseState, encoding);
return ImmutableList.create(Optional.of(fromNumeric(values.size)));
public Optional<Value> eval(final ImmutableList<Optional<Value>> list, final ParseState parseState, final Encoding encoding) {
return Optional.of(fromNumeric(list.size));
}

private static Value fromNumeric(final long length) {
return ConstantFactory.createFromNumeric(length, new Encoding(Sign.SIGNED));
}

@Override
public String toString() {
return getClass().getSimpleName() + "(" + operand + ")";
}

@Override
public boolean equals(final Object obj) {
return Util.notNullAndSameClass(this, obj)
&& Objects.equals(operand, ((Count)obj).operand);
}

@Override
public int hashCode() {
return Objects.hash(getClass(), operand);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -18,55 +18,34 @@

import static io.parsingdata.metal.Trampoline.complete;
import static io.parsingdata.metal.Trampoline.intermediate;
import static io.parsingdata.metal.Util.checkNotNull;

import java.util.Objects;
import java.util.Optional;

import io.parsingdata.metal.Trampoline;
import io.parsingdata.metal.Util;
import io.parsingdata.metal.data.ImmutableList;
import io.parsingdata.metal.data.ParseState;
import io.parsingdata.metal.encoding.Encoding;
import io.parsingdata.metal.expression.value.OneToOneValueExpression;
import io.parsingdata.metal.expression.value.Value;
import io.parsingdata.metal.expression.value.ValueExpression;

/**
* A {@link ValueExpression} that represents the first {@link Value} returned
* by evaluating its <code>operand</code>.
*/
public class First implements ValueExpression {

public final ValueExpression operand;
public class First extends OneToOneValueExpression {

public First(final ValueExpression operand) {
this.operand = checkNotNull(operand, "operand");
super(operand);
}

@Override
public ImmutableList<Optional<Value>> eval(final ParseState parseState, final Encoding encoding) {
final ImmutableList<Optional<Value>> list = operand.eval(parseState, encoding);
return list.isEmpty() ? list : ImmutableList.create(getFirst(list).computeResult());
public Optional<Value> eval(final ImmutableList<Optional<Value>> list, final ParseState parseState, final Encoding encoding) {
return list.isEmpty() ? Optional.empty() : getFirst(list).computeResult();
}

private Trampoline<Optional<Value>> getFirst(final ImmutableList<Optional<Value>> values) {
return values.tail.isEmpty() ? complete(() -> values.head) : intermediate(() -> getFirst(values.tail));
}

@Override
public String toString() {
return getClass().getSimpleName() + "(" + operand + ")";
}

@Override
public boolean equals(final Object obj) {
return Util.notNullAndSameClass(this, obj)
&& Objects.equals(operand, ((First)obj).operand);
}

@Override
public int hashCode() {
return Objects.hash(getClass(), operand);
}

}
Loading