Skip to content

Commit

Permalink
Merge pull request #633 from hashicorp/jbardin/conditional-length-ref…
Browse files Browse the repository at this point in the history 
…inements

hclsyntax: conditional refinements of collections must use Range()
  • Loading branch information
@jbardin
jbardin committed Oct 12, 2023
2 parents 63067e8 + bad33d5 commit 4945193
Show file tree
Hide file tree
Showing 2 changed files with 203 additions and 49 deletions.
124 changes: 78 additions & 46 deletions hclsyntax/expression.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -696,64 +696,95 @@ func (e *ConditionalExpr) Value(ctx *hcl.EvalContext) (cty.Value, hcl.Diagnostic
return cty.UnknownVal(resultType), diags
}
if !condResult.IsKnown() {
// we use the unmarked values throughout the unknown branch
_, condResultMarks := condResult.Unmark()
trueResult, trueResultMarks := trueResult.Unmark()
falseResult, falseResultMarks := falseResult.Unmark()

// use a value to merge marks
_, resMarks := cty.DynamicVal.WithMarks(condResultMarks, trueResultMarks, falseResultMarks).Unmark()

trueRange := trueResult.Range()
falseRange := falseResult.Range()

// if both branches are known to be null, then the result must still be null
if trueResult.IsNull() && falseResult.IsNull() {
return cty.NullVal(resultType).WithMarks(resMarks), diags
}

// We might be able to offer a refined range for the result based on
// the two possible outcomes.
if trueResult.Type() == cty.Number && falseResult.Type() == cty.Number {
// This case deals with the common case of (predicate ? 1 : 0) and
// significantly decreases the range of the result in that case.
if !(trueResult.IsNull() || falseResult.IsNull()) {
if gt := trueResult.GreaterThan(falseResult); gt.IsKnown() {
b := cty.UnknownVal(cty.Number).Refine()
if gt.True() {
b = b.
NumberRangeLowerBound(falseResult, true).
NumberRangeUpperBound(trueResult, true)
} else {
b = b.
NumberRangeLowerBound(trueResult, true).
NumberRangeUpperBound(falseResult, true)
}
b = b.NotNull() // If neither of the results is null then the result can't be either
return b.NewValue().WithSameMarks(condResult).WithSameMarks(trueResult).WithSameMarks(falseResult), diags
ref := cty.UnknownVal(cty.Number).Refine()
if trueRange.DefinitelyNotNull() && falseRange.DefinitelyNotNull() {
ref = ref.NotNull()
}

falseLo, falseLoInc := falseRange.NumberLowerBound()
falseHi, falseHiInc := falseRange.NumberUpperBound()
trueLo, trueLoInc := trueRange.NumberLowerBound()
trueHi, trueHiInc := trueRange.NumberUpperBound()

if falseLo.IsKnown() && trueLo.IsKnown() {
lo, loInc := falseLo, falseLoInc
switch {
case trueLo.LessThan(falseLo).True():
lo, loInc = trueLo, trueLoInc
case trueLo.Equals(falseLo).True():
loInc = trueLoInc || falseLoInc
}

ref = ref.NumberRangeLowerBound(lo, loInc)
}

if falseHi.IsKnown() && trueHi.IsKnown() {
hi, hiInc := falseHi, falseHiInc
switch {
case trueHi.GreaterThan(falseHi).True():
hi, hiInc = trueHi, trueHiInc
case trueHi.Equals(falseHi).True():
hiInc = trueHiInc || falseHiInc
}
ref = ref.NumberRangeUpperBound(hi, hiInc)
}

return ref.NewValue().WithMarks(resMarks), diags
}

if trueResult.Type().IsCollectionType() && falseResult.Type().IsCollectionType() {
if trueResult.Type().Equals(falseResult.Type()) {
if !(trueResult.IsNull() || falseResult.IsNull()) {
trueLen := trueResult.Length()
falseLen := falseResult.Length()
if gt := trueLen.GreaterThan(falseLen); gt.IsKnown() {
b := cty.UnknownVal(resultType).Refine()
trueLen, _ := trueLen.AsBigFloat().Int64()
falseLen, _ := falseLen.AsBigFloat().Int64()
if gt.True() {
b = b.
CollectionLengthLowerBound(int(falseLen)).
CollectionLengthUpperBound(int(trueLen))
} else {
b = b.
CollectionLengthLowerBound(int(trueLen)).
CollectionLengthUpperBound(int(falseLen))
}
b = b.NotNull() // If neither of the results is null then the result can't be either
return b.NewValue().WithSameMarks(condResult).WithSameMarks(trueResult).WithSameMarks(falseResult), diags
}
ref := cty.UnknownVal(resultType).Refine()
if trueRange.DefinitelyNotNull() && falseRange.DefinitelyNotNull() {
ref = ref.NotNull()
}

falseLo := falseRange.LengthLowerBound()
falseHi := falseRange.LengthUpperBound()
trueLo := trueRange.LengthLowerBound()
trueHi := trueRange.LengthUpperBound()

lo := falseLo
if trueLo < falseLo {
lo = trueLo
}

hi := falseHi
if trueHi > falseHi {
hi = trueHi
}

ref = ref.CollectionLengthLowerBound(lo).CollectionLengthUpperBound(hi)
return ref.NewValue().WithMarks(resMarks), diags
}
}
_, condResultMarks := condResult.Unmark()
trueResult, trueResultMarks := trueResult.Unmark()
falseResult, falseResultMarks := falseResult.Unmark()

trueRng := trueResult.Range()
falseRng := falseResult.Range()
ret := cty.UnknownVal(resultType)
if trueRng.DefinitelyNotNull() && falseRng.DefinitelyNotNull() {
if trueRange.DefinitelyNotNull() && falseRange.DefinitelyNotNull() {
ret = ret.RefineNotNull()
}
return ret.WithMarks(condResultMarks, trueResultMarks, falseResultMarks), diags
return ret.WithMarks(resMarks), diags
}

condResult, err := convert.Convert(condResult, cty.Bool)
if err != nil {
diags = append(diags, &hcl.Diagnostic{
Expand Down Expand Up @@ -1244,9 +1275,9 @@ func (e *ObjectConsKeyExpr) UnwrapExpression() Expression {

// ForExpr represents iteration constructs:
//
// tuple = [for i, v in list: upper(v) if i > 2]
// object = {for k, v in map: k => upper(v)}
// object_of_tuples = {for v in list: v.key: v...}
// tuple = [for i, v in list: upper(v) if i > 2]
// object = {for k, v in map: k => upper(v)}
// object_of_tuples = {for v in list: v.key: v...}
type ForExpr struct {
KeyVar string // empty if ignoring the key
ValVar string
Expand Down Expand Up @@ -1742,7 +1773,8 @@ func (e *SplatExpr) Value(ctx *hcl.EvalContext) (cty.Value, hcl.Diagnostics) {
if ty.IsListType() && sourceVal.Type().IsCollectionType() {
// We can refine the length of an unknown list result based on
// the source collection's own length.
sourceRng := sourceVal.Range()
sv, _ := sourceVal.Unmark()
sourceRng := sv.Range()
ret = ret.Refine().
CollectionLengthLowerBound(sourceRng.LengthLowerBound()).
CollectionLengthUpperBound(sourceRng.LengthUpperBound()).
Expand Down
128 changes: 125 additions & 3 deletions hclsyntax/expression_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1933,6 +1933,74 @@ EOT
NewValue(),
0,
},
{
`unknown ? i : j`,
&hcl.EvalContext{
Variables: map[string]cty.Value{
"unknown": cty.UnknownVal(cty.Bool),
"i": cty.NullVal(cty.Number),
"j": cty.NullVal(cty.Number),
},
},
cty.NullVal(cty.Number),
0,
},
{
`unknown ? im : jm`,
&hcl.EvalContext{
Variables: map[string]cty.Value{
"unknown": cty.UnknownVal(cty.Bool),
"im": cty.NullVal(cty.Number).Mark("a"),
"jm": cty.NullVal(cty.Number).Mark("b"),
},
},
cty.NullVal(cty.Number).Mark("a").Mark("b"),
0,
},
{
`unknown ? im : jm`,
&hcl.EvalContext{
Variables: map[string]cty.Value{
"unknown": cty.UnknownVal(cty.Bool).Mark("a"),
"im": cty.UnknownVal(cty.Number),
"jm": cty.UnknownVal(cty.Number).Mark("b"),
},
},
// the empty refinement may eventually be removed, but does nothing here
cty.UnknownVal(cty.Number).Refine().NewValue().Mark("a").Mark("b"),
0,
},
{
`unknown ? ix : jx`,
&hcl.EvalContext{
Variables: map[string]cty.Value{
"unknown": cty.UnknownVal(cty.Bool),
"ix": cty.UnknownVal(cty.Number),
"jx": cty.UnknownVal(cty.Number),
},
},
// the empty refinement may eventually be removed, but does nothing here
cty.UnknownVal(cty.Number).Refine().NewValue(),
0,
},
{
`unknown ? ir : jr`,
&hcl.EvalContext{
Variables: map[string]cty.Value{
"unknown": cty.UnknownVal(cty.Bool),
"ir": cty.UnknownVal(cty.Number).Refine().
NumberRangeLowerBound(cty.NumberIntVal(1), false).
NumberRangeUpperBound(cty.NumberIntVal(3), false).NewValue(),
"jr": cty.UnknownVal(cty.Number).Refine().
NumberRangeLowerBound(cty.NumberIntVal(2), true).
NumberRangeUpperBound(cty.NumberIntVal(4), true).NewValue(),
},
},
cty.UnknownVal(cty.Number).Refine().
NumberRangeLowerBound(cty.NumberIntVal(1), false).
NumberRangeUpperBound(cty.NumberIntVal(4), true).NewValue(),
0,
},
{
`unknown ? a : b`,
&hcl.EvalContext{
Expand All @@ -1946,17 +2014,71 @@ EOT
0,
},
{
`unknown ? a : b`,
`unknown ? al : bl`,
&hcl.EvalContext{
Variables: map[string]cty.Value{
"unknown": cty.UnknownVal(cty.Bool),
"a": cty.ListValEmpty(cty.String),
"b": cty.ListValEmpty(cty.String),
"al": cty.ListValEmpty(cty.String),
"bl": cty.ListValEmpty(cty.String),
},
},
cty.ListValEmpty(cty.String), // deduced through refinements
0,
},
{
`unknown ? am : bm`,
&hcl.EvalContext{
Variables: map[string]cty.Value{
"unknown": cty.UnknownVal(cty.Bool),
"am": cty.MapValEmpty(cty.String),
"bm": cty.MapValEmpty(cty.String).Mark("test"),
},
},
cty.MapValEmpty(cty.String).Mark("test"), // deduced through refinements
0,
},
{
`unknown ? ar : br`,
&hcl.EvalContext{
Variables: map[string]cty.Value{
"unknown": cty.UnknownVal(cty.Bool),
"ar": cty.UnknownVal(cty.Set(cty.String)).Refine().
CollectionLengthLowerBound(1).CollectionLengthUpperBound(3).NewValue(),
"br": cty.UnknownVal(cty.Set(cty.String)).Refine().
CollectionLengthLowerBound(2).CollectionLengthUpperBound(4).NewValue(),
},
},
cty.UnknownVal(cty.Set(cty.String)).Refine().CollectionLengthLowerBound(1).CollectionLengthUpperBound(4).NewValue(), // deduced through refinements
0,
},
{
`unknown ? arn : brn`,
&hcl.EvalContext{
Variables: map[string]cty.Value{
"unknown": cty.UnknownVal(cty.Bool),
"arn": cty.UnknownVal(cty.Set(cty.String)).Refine().NotNull().
CollectionLengthLowerBound(1).CollectionLengthUpperBound(2).NewValue(),
"brn": cty.UnknownVal(cty.Set(cty.String)).Refine().NotNull().
CollectionLengthLowerBound(3).CollectionLengthUpperBound(4).NewValue(),
},
},
cty.UnknownVal(cty.Set(cty.String)).Refine().NotNull().CollectionLengthLowerBound(1).CollectionLengthUpperBound(4).NewValue(), // deduced through refinements
0,
},
{
`unknown ? amr : bmr`,
&hcl.EvalContext{
Variables: map[string]cty.Value{
"unknown": cty.UnknownVal(cty.Bool),
"amr": cty.UnknownVal(cty.Set(cty.String)).Mark("test").Refine().
CollectionLengthLowerBound(1).CollectionLengthUpperBound(2).NewValue(),
"bmr": cty.UnknownVal(cty.Set(cty.String)).Mark("test").Refine().
CollectionLengthLowerBound(3).CollectionLengthUpperBound(4).NewValue(),
},
},
cty.UnknownVal(cty.Set(cty.String)).Refine().CollectionLengthLowerBound(1).CollectionLengthUpperBound(4).NewValue().Mark("test"), // deduced through refinements
0,
},
{
`unknown ? a : b`,
&hcl.EvalContext{
Expand Down

0 comments on commit 4945193

Please sign in to comment.