Fix panic in setproduct when one of the arguments is an empty list

[mantoine96]

Hi!

This PR fixes: #102

When one of the argument of setproduct is an empty list, we would get this panic:

Running tool: /usr/local/bin/go test -timeout 30s -run ^TestSetproduct$ github.com/zclconf/go-cty/cty/function/stdlib

--- FAIL: TestSetproduct (0.00s)
    --- FAIL: TestSetproduct/Setproduct([]cty.Value{cty.ListValEmpty(cty.EmptyObject),_cty.ListVal([]cty.Value{cty.StringVal("quick"),_cty.StringVal("fox")})}) (0.00s)
        /Users/matthieu/Documents/projects/oss/go-cty/cty/function/stdlib/collection_test.go:2187: unexpected error: panic in function implementation: runtime error: hash of unhashable type cty.ValueMarks
            goroutine 8 [running]:
            runtime/debug.Stack(0xc0000746c8, 0x12b5120, 0xc00015d3c0)
            	/usr/local/Cellar/go/1.15.2/libexec/src/runtime/debug/stack.go:24 +0x9f
            github.com/zclconf/go-cty/cty/function.errorForPanic(...)
            	/Users/matthieu/Documents/projects/oss/go-cty/cty/function/error.go:44
            github.com/zclconf/go-cty/cty/function.Function.Call.func1(0xc000074e98, 0xc000074eb8)
            	/Users/matthieu/Documents/projects/oss/go-cty/cty/function/function.go:291 +0x95
            panic(0x12b5120, 0xc00015d3c0)
            	/usr/local/Cellar/go/1.15.2/libexec/src/runtime/panic.go:969 +0x175
            github.com/zclconf/go-cty/cty.Value.Mark(0x13a5ee0, 0xc00015d3b0, 0x12a4260, 0xc000164e60, 0x12c03e0, 0x0, 0x12e8ea0, 0xc000163980, 0x0, 0xc000074c38)
            	/Users/matthieu/Documents/projects/oss/go-cty/cty/marks.go:208 +0x1ce
            github.com/zclconf/go-cty/cty/function/stdlib.glob..func33(0xc00015f100, 0x2, 0x2, 0x13a5ee0, 0xc00015d3a0, 0xc00015d3a0, 0x0, 0x0, 0x868cf, 0x868cf, ...)
            	/Users/matthieu/Documents/projects/oss/go-cty/cty/function/stdlib/collection.go:956 +0xf05
            github.com/zclconf/go-cty/cty/function.Function.Call(0xc00006d2c0, 0xc00015f100, 0x2, 0x2, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0)
            	/Users/matthieu/Documents/projects/oss/go-cty/cty/function/function.go:295 +0x51a
            github.com/zclconf/go-cty/cty/function/stdlib.SetProduct(...)
            	/Users/matthieu/Documents/projects/oss/go-cty/cty/function/stdlib/collection.go:1411
            github.com/zclconf/go-cty/cty/function/stdlib.TestSetproduct.func1(0xc000001980)
            	/Users/matthieu/Documents/projects/oss/go-cty/cty/function/stdlib/collection_test.go:2177 +0x88
            testing.tRunner(0xc000001980, 0xc00015d350)
            	/usr/local/Cellar/go/1.15.2/libexec/src/testing/testing.go:1127 +0xef
            created by testing.(*T).Run
            	/usr/local/Cellar/go/1.15.2/libexec/src/testing/testing.go:1178 +0x386
FAIL
FAIL	github.com/zclconf/go-cty/cty/function/stdlib	0.423s
FAIL

I think the reason for this is that in function/stdlib/collection.go, in the case of an argument being an empty collection, an empty list was returned. However that list was first run through .Mark which caused the panic since there are no marks.

I've also taken the liberty of adding a test which helped me exercise this panic. The short-circuit codepath wasn't exercised previously.

I've made sure that all the tests are still passing after my change.

Thanks!

[alisdair]

Thanks for working on this! I left a comment inline about the exact solution to the problem.

[alisdair]

This is not the correct change here, as it will result in dropping marks from input values.

Instead, I think the crash is caused by calling Mark with a cty.ValueMarks value, instead of calling WithMarks. Unfortunately the compiler didn't catch this because Mark accepts interface{}.

Would you mind fixing this up to call WithMarks for both the empty list and empty set branches? Adding tests cases to cover this would be great too. Input of two empty lists marked "a" and "b" respectively should result in an empty list marked with both "a" and "b", and the same for sets.

[mantoine96]

Hey @alisdair thanks for your input! I think you're right. I'll fix this :)

[apparentlymart]

Thanks for working on this @thehunt33r, and thanks @alisdair for reviewing!

This also looks good to me, with the updates Alisdair suggested, and so I'm going to merge it now. Thanks again!