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

[operators][integrate]feat!: Integrate OperationExpr into GeneralForStatement #355

Merged
merged 49 commits into from
Oct 8, 2020
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
49 commits
Select commit Hold shift + click to select a range
aeb972b
Integrate OperationExpr into GeneralForStatement
summer-ji-eng Sep 29, 2020
5ed201c
make exprs optional
summer-ji-eng Sep 29, 2020
91741f3
format tests
summer-ji-eng Sep 29, 2020
17c9959
Add comments
summer-ji-eng Sep 29, 2020
7c5269f
Merge branch 'master' into integrate_GeneralForStatement
summer-ji-eng Sep 29, 2020
1cd43c6
Merge branch 'master' into integrate_GeneralForStatement
summer-ji-eng Oct 1, 2020
ade35c7
remove null case for expressions
summer-ji-eng Oct 1, 2020
18cd6d9
Merge branch 'master' into integrate_GeneralForStatement
summer-ji-eng Oct 2, 2020
c5f32ee
fix comments
summer-ji-eng Oct 2, 2020
25c8eca
Merge branch 'master' into integrate_GeneralForStatement
summer-ji-eng Oct 2, 2020
1e75624
Update second version design
summer-ji-eng Oct 4, 2020
7698a95
Merge branch 'integrate_GeneralForStatement' of github.com:googleapis…
summer-ji-eng Oct 4, 2020
b52cf21
Merge branch 'master' into integrate_GeneralForStatement
summer-ji-eng Oct 4, 2020
30b4ff4
fix: add final keyword checking in post increment operation expr
summer-ji-eng Oct 4, 2020
5a5ec4e
fix: add final keyword check on assignment expr
summer-ji-eng Oct 4, 2020
741edca
Integrate OperationExpr into GeneralForStatement
summer-ji-eng Sep 29, 2020
b681720
make exprs optional
summer-ji-eng Sep 29, 2020
5aeae69
format tests
summer-ji-eng Sep 29, 2020
c04297d
Add comments
summer-ji-eng Sep 29, 2020
9375d9b
remove null case for expressions
summer-ji-eng Oct 1, 2020
dbfb6ed
fix comments
summer-ji-eng Oct 2, 2020
d899782
Update second version design
summer-ji-eng Oct 4, 2020
8d647cd
rebase fix in assignment and increment expr
summer-ji-eng Oct 4, 2020
e5c54ef
Merge branch 'master' into integrate_GeneralForStatement
summer-ji-eng Oct 6, 2020
aabd0a6
Merge branch 'master' into integrate_GeneralForStatement
summer-ji-eng Oct 6, 2020
976c9c1
Merge branch 'integrate_GeneralForStatement' of github.com:googleapis…
summer-ji-eng Oct 6, 2020
c5b02b4
fix: add final keyword checking in post increment operation expr
summer-ji-eng Oct 4, 2020
9ac46dd
fix: add final keyword check on assignment expr
summer-ji-eng Oct 4, 2020
097ca65
Integrate OperationExpr into GeneralForStatement
summer-ji-eng Sep 29, 2020
c958696
make exprs optional
summer-ji-eng Sep 29, 2020
747b3b4
format tests
summer-ji-eng Sep 29, 2020
2d7bbf4
Add comments
summer-ji-eng Sep 29, 2020
08f4b2d
remove null case for expressions
summer-ji-eng Oct 1, 2020
83176f9
fix comments
summer-ji-eng Oct 2, 2020
2a6fd72
Update second version design
summer-ji-eng Oct 4, 2020
d7931b0
rebase fix in assignment and increment expr
summer-ji-eng Oct 4, 2020
3581aa3
Merge branch 'integrate_GeneralForStatement' of github.com:googleapis…
summer-ji-eng Oct 6, 2020
ac9904f
remove .circleci/config.yml
summer-ji-eng Oct 6, 2020
1124549
checkout master with files
summer-ji-eng Oct 6, 2020
ea276a0
git batching
summer-ji-eng Oct 6, 2020
93a1fc2
Merge branch 'master' into integrate_GeneralForStatement
summer-ji-eng Oct 7, 2020
33bdbca
Merge branch 'master' into integrate_GeneralForStatement
summer-ji-eng Oct 7, 2020
bd8e2f8
Merge branch 'master' into integrate_GeneralForStatement
summer-ji-eng Oct 7, 2020
4e20a14
Merge branch 'master' into integrate_GeneralForStatement
summer-ji-eng Oct 7, 2020
bc40948
Merge branch 'master' into integrate_GeneralForStatement
summer-ji-eng Oct 7, 2020
1e7c838
delete comment code
summer-ji-eng Oct 7, 2020
06dd281
Initialization is expr
summer-ji-eng Oct 8, 2020
7816b01
Merge branch 'master' into integrate_GeneralForStatement
summer-ji-eng Oct 8, 2020
6da3962
Merge branch 'master' into integrate_GeneralForStatement
summer-ji-eng Oct 8, 2020
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
Original file line number Diff line number Diff line change
Expand Up @@ -22,19 +22,11 @@

@AutoValue
public abstract class GeneralForStatement implements Statement {
public abstract Expr initializationExpr();
public abstract AssignmentExpr initializationExpr();
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you please confirm if this must be an AssignmentExpr (e.g. what about passing foobar() here)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually, it should not be limited to assignmentExpr, it should be expr which belongs to statmentExprList(https://docs.oracle.com/javase/specs/jls/se10/html/jls-14.html#jls).
My original though is to simplify the way to get variable. After consideration of back compatible, it should change back to expr. I will make change on this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@miraleung changed the interface use Expr for initialization. Before I merge the code, could you take one more look. Thanks.


// TODO(summerji): Integrate OperationExpr here. Start by uncommenting the following section to
// replace the localVariableExpr and maxSizeExpr getters after it.
/*
// Uses the same terminology as https://docs.oracle.com/javase/tutorial/java/nutsandbolts/for.html.
public abstract Expr terminationExpr();
public abstract Expr incrementExpr();
*/

public abstract VariableExpr localVariableExpr();

public abstract Expr maxSizeExpr();
public abstract Expr incrementExpr();

public abstract ImmutableList<Statement> body();

Expand All @@ -43,14 +35,26 @@ public void accept(AstNodeVisitor visitor) {
visitor.visit(this);
}

// Convenience wrapper.
// incrementWith is convenience wrapper to generate index-base for-loop with lower and upper bound
// and post increment on variable, like code in ```for (int i = 0; i < getMax(); i++) {..}```
// TODO (developer): Add more convenience wrapper for the future generation needs.
public static GeneralForStatement incrementWith(
VariableExpr variableExpr, Expr maxSizeExpr, List<Statement> body) {
// TODO(summerji): Do some integration here, in JavaWriterVisitor, in ImportWriterVisitor, and
// add more tests.
VariableExpr localVariableExpr,
ValueExpr initialValueExpr,
Expr maxSizeExpr,
List<Statement> body) {
return builder()
.setLocalVariableExpr(variableExpr.toBuilder().setIsDecl(false).build())
.setMaxSizeExpr(maxSizeExpr)
.setInitializationExpr(
AssignmentExpr.builder()
.setVariableExpr(localVariableExpr)
.setValueExpr(initialValueExpr)
.build())
.setTerminationExpr(
RelationalOperationExpr.lessThanWithExprs(
localVariableExpr.toBuilder().setIsDecl(false).build(), maxSizeExpr))
.setIncrementExpr(
UnaryOperationExpr.postfixIncrementWithExpr(
localVariableExpr.toBuilder().setIsDecl(false).build()))
.setBody(body)
.build();
}
Expand All @@ -61,22 +65,24 @@ public static Builder builder() {

@AutoValue.Builder
public abstract static class Builder {
public abstract Builder setInitializationExpr(Expr initializationExpr);

public abstract Builder setBody(List<Statement> body);

// Private.
abstract Builder setLocalVariableExpr(VariableExpr variableExpr);

abstract Builder setMaxSizeExpr(Expr maxSizeExpr);

abstract VariableExpr localVariableExpr();
// Private setter.
abstract Builder setInitializationExpr(AssignmentExpr initializationExpr);
// Private setter.
abstract Builder setTerminationExpr(Expr terminationExpr);
// Private setter.
abstract Builder setIncrementExpr(Expr incrementExpr);
// Private setter.
abstract Builder setBody(List<Statement> body);

abstract GeneralForStatement autoBuild();

// Type-checking will be done in the sub-expressions.
public GeneralForStatement build() {
VariableExpr varExpr = localVariableExpr();
GeneralForStatement generalForStatement = autoBuild();
AssignmentExpr initializationExpr = generalForStatement.initializationExpr();
Expr terminationExpr = generalForStatement.terminationExpr();
Expr incrementExpr = generalForStatement.incrementExpr();
VariableExpr varExpr = initializationExpr.variableExpr();
Preconditions.checkState(
varExpr.scope().equals(ScopeNode.LOCAL),
String.format(
Expand All @@ -87,16 +93,18 @@ public GeneralForStatement build() {
String.format(
"Variable %s in a general for-loop cannot be static or final",
varExpr.variable().identifier().name()));
setInitializationExpr(
AssignmentExpr.builder()
.setVariableExpr(varExpr.toBuilder().setIsDecl(true).build())
.setValueExpr(
ValueExpr.withValue(
PrimitiveValue.builder().setValue("0").setType(TypeNode.INT).build()))
.build());
// TODO(summerji): Remove the following two lines.
// This temporary workaround will be removed soon, so it doesn't need a test.
setLocalVariableExpr(varExpr.toBuilder().setIsDecl(false).build());
Preconditions.checkState(
terminationExpr.type().equals(TypeNode.BOOLEAN),
"Terminal expression %s must be boolean-type expression.");
Preconditions.checkState(
(incrementExpr instanceof MethodInvocationExpr)
|| (incrementExpr instanceof AssignmentExpr)
|| (incrementExpr instanceof AssignmentOperationExpr)
// TODO(developer): Currently we only support postIncrement (i++), please add
// postDecrement, prefixIncrement, prefixIncrement if needs.
|| (incrementExpr instanceof UnaryOperationExpr
&& ((UnaryOperationExpr) incrementExpr).isPostfixIncrement()),
"Increment expression %s must be either a method invocation, assignment, or unary post-fix operation expression.");
return autoBuild();
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -281,8 +281,8 @@ public void visit(ForStatement forStatement) {
@Override
public void visit(GeneralForStatement generalForStatement) {
generalForStatement.initializationExpr().accept(this);
generalForStatement.localVariableExpr().accept(this);
generalForStatement.maxSizeExpr().accept(this);
generalForStatement.terminationExpr().accept(this);
generalForStatement.incrementExpr().accept(this);
statements(generalForStatement.body());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -540,17 +540,11 @@ public void visit(GeneralForStatement generalForStatement) {
semicolon();
space();

generalForStatement.localVariableExpr().accept(this);
space();
buffer.append(LEFT_ANGLE);
space();
generalForStatement.maxSizeExpr().accept(this);
generalForStatement.terminationExpr().accept(this);
semicolon();
space();

generalForStatement.localVariableExpr().accept(this);
// TODO(summerji): Remove the following temporary workaround.
buffer.append("++");
generalForStatement.incrementExpr().accept(this);
rightParen();
space();
leftBrace();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -354,10 +354,16 @@ private static MethodDefinition createSplitResponseMethod(
// TODO(miraleung): Increment batchMessageIndexVarExpr.

VariableExpr forIndexVarExpr =
VariableExpr.withVariable(Variable.builder().setType(TypeNode.INT).setName("i").build());
VariableExpr.builder()
.setIsDecl(true)
.setVariable(Variable.builder().setType(TypeNode.INT).setName("i").build())
.build();
ValueExpr initValueExpr =
ValueExpr.withValue(PrimitiveValue.builder().setValue("0").setType(TypeNode.INT).build());
GeneralForStatement innerSubresponseForStatement =
GeneralForStatement.incrementWith(
forIndexVarExpr,
initValueExpr,
subresponseCountVarExpr,
innerSubresponseForExprs.stream()
.map(e -> ExprStatement.withExpr(e))
Expand Down
Loading