Skip to content

Commit

Permalink
hclsyntax: && and || operators are right-associative
Browse files Browse the repository at this point in the history 
Previously we just treated all binary operators as being left-associative,
which for many of them didn't make a lot of difference anyway because they
were associative operators.

The logical AND and OR operators are also effectively associative the way
we have them implemented right now, because HCL always evaluates both
sides of a binary operator anyway and so there's no way to depend on
the associativity for either of these operations.

However, we'd like to implement a similar short-circuit behavior as we see
in many other similar languages, at which point the evaluation order of
the operations would become important in order to constrain which
expressions get fully evaluated and which are "short-circuited". Treating
these operators as right-associative instead of left-associative will
make the subsequent implementation more intuitive, since the LHS of the
outermost operation will decide whether to evaluate the RHS, rather than
recursing into the LHS completely first and then unwinding if the innermost
term causes the short-circuit.

Since we expect HCL expression evaluation to always be side-effect-free
it doesn't technically matter what associativity we use -- the result would
be the same either way -- but this approach will make the control flow
during evaluation match intuition about how a short-circuit behaves.

This commit does not yet actually implement the short-circuit behavior,
which will hopefully follow in a subsequent commit.
  • Loading branch information
@apparentlymart
apparentlymart committed Jun 14, 2022
1 parent b440b17 commit 9ba6757
Show file tree
Hide file tree
Showing 3 changed files with 220 additions and 7 deletions.
4 changes: 4 additions & 0 deletions hclsyntax/expression_ops.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,10 @@ var (
)

var binaryOps []map[TokenType]*Operation
var rightAssociativeBinaryOps = map[TokenType]struct{}{
TokenOr: {},
TokenAnd: {},
}

func init() {
// This operation table maps from the operator's token type
Expand Down
31 changes: 24 additions & 7 deletions hclsyntax/parser.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -563,20 +563,25 @@ func (p *parser) parseBinaryOps(ops []map[TokenType]*Operation) (Expression, hcl
return lhs, diags
}

// We'll keep eating up operators until we run out, so that operators
// with the same precedence will combine in a left-associative manner:
// a+b+c => (a+b)+c, not a+(b+c)
// Most of our operators are left-associative:
// a+b+c => (a+b)+c, not a+(b+c)
// For those, we recursively hunt for operators only of lower precedence,
// so that any subsequent operators of the same precedence will be eaten
// up by this loop and gather on the left side.
//
// Should we later want to have right-associative operators, a way
// to achieve that would be to call back up to ParseExpression here
// instead of iteratively parsing only the remaining operators.
// A few operators are instead right-associative:
// a && b && c => a && (b && c), not (a && b) && c.
// For those, we recursively hunt for operators of the same or lower
// precedence, so that the recursive call handling the right hand side will
// eat up all of the operators of the same precedence instead.
for {
next := p.Peek()
var newOp *Operation
var ok bool
if newOp, ok = thisLevel[next.Type]; !ok {
break
}
_, rightAssoc := rightAssociativeBinaryOps[next.Type]

// Are we extending an expression started on the previous iteration?
if operation != nil {
Expand All @@ -592,7 +597,19 @@ func (p *parser) parseBinaryOps(ops []map[TokenType]*Operation) (Expression, hcl
operation = newOp
p.Read() // eat operator token
var rhsDiags hcl.Diagnostics
rhs, rhsDiags = p.parseBinaryOps(remaining)
if rightAssoc {
// For right-associative, we use the same ops we were
// given so that the right-hand side can eat up all
// of the operations of the same precedence.
rhs, rhsDiags = p.parseBinaryOps(ops)
} else {
// For left-associative, we use only operators of
// lower precedence so that this will terminate when
// encountering an operator of the same precedence as
// this loop is handling, allowing this loop to eat
// up those operations instead.
rhs, rhsDiags = p.parseBinaryOps(remaining)
}
diags = append(diags, rhsDiags...)
if p.recovery && rhsDiags.HasErrors() {
return lhs, diags
Expand Down
192 changes: 192 additions & 0 deletions hclsyntax/parser_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2195,6 +2195,198 @@ block "valid" {}
},
},
},
{
// This test is for the associativity of the && operator during
// parsing. Specifically, our evaluator relies on the following
// being parsed as if it were `b && (c && d)` rather than
// `(b && c) && d` in order to correctly implement the short-circuit
// behavior.
`a = b && c && d`,
0,
&Body{
Attributes: Attributes{
"a": {
Name: "a",
Expr: &BinaryOpExpr{
LHS: &ScopeTraversalExpr{
Traversal: hcl.Traversal{
hcl.TraverseRoot{
Name: "b",
SrcRange: hcl.Range{
Start: hcl.Pos{Line: 1, Column: 5, Byte: 4},
End: hcl.Pos{Line: 1, Column: 6, Byte: 5},
},
},
},
SrcRange: hcl.Range{
Start: hcl.Pos{Line: 1, Column: 5, Byte: 4},
End: hcl.Pos{Line: 1, Column: 6, Byte: 5},
},
},
Op: OpLogicalAnd,
RHS: &BinaryOpExpr{
LHS: &ScopeTraversalExpr{
Traversal: hcl.Traversal{
hcl.TraverseRoot{
Name: "c",
SrcRange: hcl.Range{
Start: hcl.Pos{Line: 1, Column: 10, Byte: 9},
End: hcl.Pos{Line: 1, Column: 11, Byte: 10},
},
},
},
SrcRange: hcl.Range{
Start: hcl.Pos{Line: 1, Column: 10, Byte: 9},
End: hcl.Pos{Line: 1, Column: 11, Byte: 10},
},
},
Op: OpLogicalAnd,
RHS: &ScopeTraversalExpr{
Traversal: hcl.Traversal{
hcl.TraverseRoot{
Name: "d",
SrcRange: hcl.Range{
Start: hcl.Pos{Line: 1, Column: 15, Byte: 14},
End: hcl.Pos{Line: 1, Column: 16, Byte: 15},
},
},
},
SrcRange: hcl.Range{
Start: hcl.Pos{Line: 1, Column: 15, Byte: 14},
End: hcl.Pos{Line: 1, Column: 16, Byte: 15},
},
},
SrcRange: hcl.Range{
Start: hcl.Pos{Line: 1, Column: 10, Byte: 9},
End: hcl.Pos{Line: 1, Column: 16, Byte: 15},
},
},
SrcRange: hcl.Range{
Start: hcl.Pos{Line: 1, Column: 5, Byte: 4},
End: hcl.Pos{Line: 1, Column: 16, Byte: 15},
},
},
NameRange: hcl.Range{
Start: hcl.Pos{Line: 1, Column: 1, Byte: 0},
End: hcl.Pos{Line: 1, Column: 2, Byte: 1},
},
EqualsRange: hcl.Range{
Start: hcl.Pos{Line: 1, Column: 3, Byte: 2},
End: hcl.Pos{Line: 1, Column: 4, Byte: 3},
},
SrcRange: hcl.Range{
Start: hcl.Pos{Line: 1, Column: 1, Byte: 0},
End: hcl.Pos{Line: 1, Column: 16, Byte: 15},
},
},
},
Blocks: Blocks{},
SrcRange: hcl.Range{
Start: hcl.Pos{Line: 1, Column: 1, Byte: 0},
End: hcl.Pos{Line: 1, Column: 16, Byte: 15},
},
EndRange: hcl.Range{
Start: hcl.Pos{Line: 1, Column: 16, Byte: 15},
End: hcl.Pos{Line: 1, Column: 16, Byte: 15},
},
},
},
{
// This test is for the associativity of the || operator during
// parsing. Specifically, our evaluator relies on the following
// being parsed as if it were `b || (c || d)` rather than
// `(b || c) || d` in order to correctly implement the short-circuit
// behavior.
`a = b || c || d`,
0,
&Body{
Attributes: Attributes{
"a": {
Name: "a",
Expr: &BinaryOpExpr{
LHS: &ScopeTraversalExpr{
Traversal: hcl.Traversal{
hcl.TraverseRoot{
Name: "b",
SrcRange: hcl.Range{
Start: hcl.Pos{Line: 1, Column: 5, Byte: 4},
End: hcl.Pos{Line: 1, Column: 6, Byte: 5},
},
},
},
SrcRange: hcl.Range{
Start: hcl.Pos{Line: 1, Column: 5, Byte: 4},
End: hcl.Pos{Line: 1, Column: 6, Byte: 5},
},
},
Op: OpLogicalOr,
RHS: &BinaryOpExpr{
LHS: &ScopeTraversalExpr{
Traversal: hcl.Traversal{
hcl.TraverseRoot{
Name: "c",
SrcRange: hcl.Range{
Start: hcl.Pos{Line: 1, Column: 10, Byte: 9},
End: hcl.Pos{Line: 1, Column: 11, Byte: 10},
},
},
},
SrcRange: hcl.Range{
Start: hcl.Pos{Line: 1, Column: 10, Byte: 9},
End: hcl.Pos{Line: 1, Column: 11, Byte: 10},
},
},
Op: OpLogicalOr,
RHS: &ScopeTraversalExpr{
Traversal: hcl.Traversal{
hcl.TraverseRoot{
Name: "d",
SrcRange: hcl.Range{
Start: hcl.Pos{Line: 1, Column: 15, Byte: 14},
End: hcl.Pos{Line: 1, Column: 16, Byte: 15},
},
},
},
SrcRange: hcl.Range{
Start: hcl.Pos{Line: 1, Column: 15, Byte: 14},
End: hcl.Pos{Line: 1, Column: 16, Byte: 15},
},
},
SrcRange: hcl.Range{
Start: hcl.Pos{Line: 1, Column: 10, Byte: 9},
End: hcl.Pos{Line: 1, Column: 16, Byte: 15},
},
},
SrcRange: hcl.Range{
Start: hcl.Pos{Line: 1, Column: 5, Byte: 4},
End: hcl.Pos{Line: 1, Column: 16, Byte: 15},
},
},
NameRange: hcl.Range{
Start: hcl.Pos{Line: 1, Column: 1, Byte: 0},
End: hcl.Pos{Line: 1, Column: 2, Byte: 1},
},
EqualsRange: hcl.Range{
Start: hcl.Pos{Line: 1, Column: 3, Byte: 2},
End: hcl.Pos{Line: 1, Column: 4, Byte: 3},
},
SrcRange: hcl.Range{
Start: hcl.Pos{Line: 1, Column: 1, Byte: 0},
End: hcl.Pos{Line: 1, Column: 16, Byte: 15},
},
},
},
Blocks: Blocks{},
SrcRange: hcl.Range{
Start: hcl.Pos{Line: 1, Column: 1, Byte: 0},
End: hcl.Pos{Line: 1, Column: 16, Byte: 15},
},
EndRange: hcl.Range{
Start: hcl.Pos{Line: 1, Column: 16, Byte: 15},
End: hcl.Pos{Line: 1, Column: 16, Byte: 15},
},
},
},

{
` `,
Expand Down

0 comments on commit 9ba6757

Please sign in to comment.