Skip to content

Commit bad33d5

Browse files
jbardinjbardin
committed
further refine refinement handling
Correct mark handling for some conditional values. Find correct refinement for overlapping ranges which could not have been compared with `GreaterThan`. Also map inclusive flags for numeric ranges. Correct handling of DefinitelyNotNull collections. Return a known null early when both conditional branches are null.
1 parent e4589e3 commit bad33d5

File tree

2 files changed

+172
-51
lines changed

2 files changed

+172
-51
lines changed

‎hclsyntax/expression.go‎

Lines changed: 73 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -696,68 +696,95 @@ func (e *ConditionalExpr) Value(ctx *hcl.EvalContext) (cty.Value, hcl.Diagnostic
696696
return cty.UnknownVal(resultType), diags
697697
}
698698
if !condResult.IsKnown() {
699+
// we use the unmarked values throughout the unknown branch
700+
_, condResultMarks := condResult.Unmark()
701+
trueResult, trueResultMarks := trueResult.Unmark()
702+
falseResult, falseResultMarks := falseResult.Unmark()
703+
704+
// use a value to merge marks
705+
_, resMarks := cty.DynamicVal.WithMarks(condResultMarks, trueResultMarks, falseResultMarks).Unmark()
706+
707+
trueRange := trueResult.Range()
708+
falseRange := falseResult.Range()
709+
710+
// if both branches are known to be null, then the result must still be null
711+
if trueResult.IsNull() && falseResult.IsNull() {
712+
return cty.NullVal(resultType).WithMarks(resMarks), diags
713+
}
714+
699715
// We might be able to offer a refined range for the result based on
700716
// the two possible outcomes.
701717
if trueResult.Type() == cty.Number && falseResult.Type() == cty.Number {
702-
// This case deals with the common case of (predicate ? 1 : 0) and
703-
// significantly decreases the range of the result in that case.
704-
if !(trueResult.IsNull() || falseResult.IsNull()) {
705-
if gt := trueResult.GreaterThan(falseResult); gt.IsKnown() {
706-
b := cty.UnknownVal(cty.Number).Refine()
707-
if gt.True() {
708-
b = b.
709-
NumberRangeLowerBound(falseResult, true).
710-
NumberRangeUpperBound(trueResult, true)
711-
} else {
712-
b = b.
713-
NumberRangeLowerBound(trueResult, true).
714-
NumberRangeUpperBound(falseResult, true)
715-
}
716-
b = b.NotNull() // If neither of the results is null then the result can't be either
717-
return b.NewValue().WithSameMarks(condResult).WithSameMarks(trueResult).WithSameMarks(falseResult), diags
718+
ref := cty.UnknownVal(cty.Number).Refine()
719+
if trueRange.DefinitelyNotNull() && falseRange.DefinitelyNotNull() {
720+
ref = ref.NotNull()
721+
}
722+
723+
falseLo, falseLoInc := falseRange.NumberLowerBound()
724+
falseHi, falseHiInc := falseRange.NumberUpperBound()
725+
trueLo, trueLoInc := trueRange.NumberLowerBound()
726+
trueHi, trueHiInc := trueRange.NumberUpperBound()
727+
728+
if falseLo.IsKnown() && trueLo.IsKnown() {
729+
lo, loInc := falseLo, falseLoInc
730+
switch {
731+
case trueLo.LessThan(falseLo).True():
732+
lo, loInc = trueLo, trueLoInc
733+
case trueLo.Equals(falseLo).True():
734+
loInc = trueLoInc || falseLoInc
718735
}
736+
737+
ref = ref.NumberRangeLowerBound(lo, loInc)
719738
}
739+
740+
if falseHi.IsKnown() && trueHi.IsKnown() {
741+
hi, hiInc := falseHi, falseHiInc
742+
switch {
743+
case trueHi.GreaterThan(falseHi).True():
744+
hi, hiInc = trueHi, trueHiInc
745+
case trueHi.Equals(falseHi).True():
746+
hiInc = trueHiInc || falseHiInc
747+
}
748+
ref = ref.NumberRangeUpperBound(hi, hiInc)
749+
}
750+
751+
return ref.NewValue().WithMarks(resMarks), diags
720752
}
753+
721754
if trueResult.Type().IsCollectionType() && falseResult.Type().IsCollectionType() {
722755
if trueResult.Type().Equals(falseResult.Type()) {
723-
if !(trueResult.IsNull() || falseResult.IsNull()) {
724-
// the bounds are not part of the final result value, so
725-
// the marks are not needed
726-
tr, _ := trueResult.Unmark()
727-
fr, _ := falseResult.Unmark()
728-
trueRange := tr.Range()
729-
falseRange := fr.Range()
730-
731-
if gt := trueResult.Length().GreaterThan(falseResult.Length()); gt.IsKnown() {
732-
gt, _ := gt.Unmark()
733-
b := cty.UnknownVal(resultType).Refine()
734-
if gt.True() {
735-
b = b.
736-
CollectionLengthLowerBound(falseRange.LengthLowerBound()).
737-
CollectionLengthUpperBound(trueRange.LengthUpperBound())
738-
} else {
739-
b = b.
740-
CollectionLengthLowerBound(trueRange.LengthLowerBound()).
741-
CollectionLengthUpperBound(falseRange.LengthUpperBound())
742-
}
743-
b = b.NotNull() // If neither of the results is null then the result can't be either
744-
return b.NewValue().WithSameMarks(condResult).WithSameMarks(trueResult).WithSameMarks(falseResult), diags
745-
}
756+
ref := cty.UnknownVal(resultType).Refine()
757+
if trueRange.DefinitelyNotNull() && falseRange.DefinitelyNotNull() {
758+
ref = ref.NotNull()
759+
}
760+
761+
falseLo := falseRange.LengthLowerBound()
762+
falseHi := falseRange.LengthUpperBound()
763+
trueLo := trueRange.LengthLowerBound()
764+
trueHi := trueRange.LengthUpperBound()
765+
766+
lo := falseLo
767+
if trueLo < falseLo {
768+
lo = trueLo
746769
}
770+
771+
hi := falseHi
772+
if trueHi > falseHi {
773+
hi = trueHi
774+
}
775+
776+
ref = ref.CollectionLengthLowerBound(lo).CollectionLengthUpperBound(hi)
777+
return ref.NewValue().WithMarks(resMarks), diags
747778
}
748779
}
749-
_, condResultMarks := condResult.Unmark()
750-
trueResult, trueResultMarks := trueResult.Unmark()
751-
falseResult, falseResultMarks := falseResult.Unmark()
752780

753-
trueRng := trueResult.Range()
754-
falseRng := falseResult.Range()
755781
ret := cty.UnknownVal(resultType)
756-
if trueRng.DefinitelyNotNull() && falseRng.DefinitelyNotNull() {
782+
if trueRange.DefinitelyNotNull() && falseRange.DefinitelyNotNull() {
757783
ret = ret.RefineNotNull()
758784
}
759-
return ret.WithMarks(condResultMarks, trueResultMarks, falseResultMarks), diags
785+
return ret.WithMarks(resMarks), diags
760786
}
787+
761788
condResult, err := convert.Convert(condResult, cty.Bool)
762789
if err != nil {
763790
diags = append(diags, &hcl.Diagnostic{

‎hclsyntax/expression_test.go‎

Lines changed: 99 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1933,6 +1933,74 @@ EOT
19331933
NewValue(),
19341934
0,
19351935
},
1936+
{
1937+
`unknown ? i : j`,
1938+
&hcl.EvalContext{
1939+
Variables: map[string]cty.Value{
1940+
"unknown": cty.UnknownVal(cty.Bool),
1941+
"i": cty.NullVal(cty.Number),
1942+
"j": cty.NullVal(cty.Number),
1943+
},
1944+
},
1945+
cty.NullVal(cty.Number),
1946+
0,
1947+
},
1948+
{
1949+
`unknown ? im : jm`,
1950+
&hcl.EvalContext{
1951+
Variables: map[string]cty.Value{
1952+
"unknown": cty.UnknownVal(cty.Bool),
1953+
"im": cty.NullVal(cty.Number).Mark("a"),
1954+
"jm": cty.NullVal(cty.Number).Mark("b"),
1955+
},
1956+
},
1957+
cty.NullVal(cty.Number).Mark("a").Mark("b"),
1958+
0,
1959+
},
1960+
{
1961+
`unknown ? im : jm`,
1962+
&hcl.EvalContext{
1963+
Variables: map[string]cty.Value{
1964+
"unknown": cty.UnknownVal(cty.Bool).Mark("a"),
1965+
"im": cty.UnknownVal(cty.Number),
1966+
"jm": cty.UnknownVal(cty.Number).Mark("b"),
1967+
},
1968+
},
1969+
// the empty refinement may eventually be removed, but does nothing here
1970+
cty.UnknownVal(cty.Number).Refine().NewValue().Mark("a").Mark("b"),
1971+
0,
1972+
},
1973+
{
1974+
`unknown ? ix : jx`,
1975+
&hcl.EvalContext{
1976+
Variables: map[string]cty.Value{
1977+
"unknown": cty.UnknownVal(cty.Bool),
1978+
"ix": cty.UnknownVal(cty.Number),
1979+
"jx": cty.UnknownVal(cty.Number),
1980+
},
1981+
},
1982+
// the empty refinement may eventually be removed, but does nothing here
1983+
cty.UnknownVal(cty.Number).Refine().NewValue(),
1984+
0,
1985+
},
1986+
{
1987+
`unknown ? ir : jr`,
1988+
&hcl.EvalContext{
1989+
Variables: map[string]cty.Value{
1990+
"unknown": cty.UnknownVal(cty.Bool),
1991+
"ir": cty.UnknownVal(cty.Number).Refine().
1992+
NumberRangeLowerBound(cty.NumberIntVal(1), false).
1993+
NumberRangeUpperBound(cty.NumberIntVal(3), false).NewValue(),
1994+
"jr": cty.UnknownVal(cty.Number).Refine().
1995+
NumberRangeLowerBound(cty.NumberIntVal(2), true).
1996+
NumberRangeUpperBound(cty.NumberIntVal(4), true).NewValue(),
1997+
},
1998+
},
1999+
cty.UnknownVal(cty.Number).Refine().
2000+
NumberRangeLowerBound(cty.NumberIntVal(1), false).
2001+
NumberRangeUpperBound(cty.NumberIntVal(4), true).NewValue(),
2002+
0,
2003+
},
19362004
{
19372005
`unknown ? a : b`,
19382006
&hcl.EvalContext{
@@ -1946,25 +2014,51 @@ EOT
19462014
0,
19472015
},
19482016
{
1949-
`unknown ? a : b`,
2017+
`unknown ? al : bl`,
19502018
&hcl.EvalContext{
19512019
Variables: map[string]cty.Value{
19522020
"unknown": cty.UnknownVal(cty.Bool),
1953-
"a": cty.ListValEmpty(cty.String),
1954-
"b": cty.ListValEmpty(cty.String),
2021+
"al": cty.ListValEmpty(cty.String),
2022+
"bl": cty.ListValEmpty(cty.String),
19552023
},
19562024
},
19572025
cty.ListValEmpty(cty.String), // deduced through refinements
19582026
0,
19592027
},
2028+
{
2029+
`unknown ? am : bm`,
2030+
&hcl.EvalContext{
2031+
Variables: map[string]cty.Value{
2032+
"unknown": cty.UnknownVal(cty.Bool),
2033+
"am": cty.MapValEmpty(cty.String),
2034+
"bm": cty.MapValEmpty(cty.String).Mark("test"),
2035+
},
2036+
},
2037+
cty.MapValEmpty(cty.String).Mark("test"), // deduced through refinements
2038+
0,
2039+
},
19602040
{
19612041
`unknown ? ar : br`,
19622042
&hcl.EvalContext{
19632043
Variables: map[string]cty.Value{
19642044
"unknown": cty.UnknownVal(cty.Bool),
19652045
"ar": cty.UnknownVal(cty.Set(cty.String)).Refine().
1966-
CollectionLengthLowerBound(1).CollectionLengthUpperBound(2).NewValue(),
2046+
CollectionLengthLowerBound(1).CollectionLengthUpperBound(3).NewValue(),
19672047
"br": cty.UnknownVal(cty.Set(cty.String)).Refine().
2048+
CollectionLengthLowerBound(2).CollectionLengthUpperBound(4).NewValue(),
2049+
},
2050+
},
2051+
cty.UnknownVal(cty.Set(cty.String)).Refine().CollectionLengthLowerBound(1).CollectionLengthUpperBound(4).NewValue(), // deduced through refinements
2052+
0,
2053+
},
2054+
{
2055+
`unknown ? arn : brn`,
2056+
&hcl.EvalContext{
2057+
Variables: map[string]cty.Value{
2058+
"unknown": cty.UnknownVal(cty.Bool),
2059+
"arn": cty.UnknownVal(cty.Set(cty.String)).Refine().NotNull().
2060+
CollectionLengthLowerBound(1).CollectionLengthUpperBound(2).NewValue(),
2061+
"brn": cty.UnknownVal(cty.Set(cty.String)).Refine().NotNull().
19682062
CollectionLengthLowerBound(3).CollectionLengthUpperBound(4).NewValue(),
19692063
},
19702064
},
@@ -1982,7 +2076,7 @@ EOT
19822076
CollectionLengthLowerBound(3).CollectionLengthUpperBound(4).NewValue(),
19832077
},
19842078
},
1985-
cty.UnknownVal(cty.Set(cty.String)).Refine().NotNull().CollectionLengthLowerBound(1).CollectionLengthUpperBound(4).NewValue().Mark("test"), // deduced through refinements
2079+
cty.UnknownVal(cty.Set(cty.String)).Refine().CollectionLengthLowerBound(1).CollectionLengthUpperBound(4).NewValue().Mark("test"), // deduced through refinements
19862080
0,
19872081
},
19882082
{

0 commit comments

Comments
 (0)