Avoid exception for immutable list from folded expression

[livebranch]

• Added check to avoid error thrown when returning a immutable list from a function called by a comprehension expression

This pull request fixes a regression introduced in commit 8cf1503. Specifically, an optimization was made which overrides an initial zero size list with a empty mutable list. The optimization makes an assumption that the result from the first step will always be mutable list, which is not always the case. Hopefully the code is self-explanatory, but I can add further details if required.

[google-cla]

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

[TristonianJones]

/gcbrun

[TristonianJones]

@livebranch Would you mind signing the CLA? Once you have I can merge the PR.

[livebranch]

@livebranch Would you mind signing the CLA? Once you have I can merge the PR.

Sorry, email was private so CLA didn't work. Now fixed.

[TristonianJones]

@livebranch I've merged the change, but could you please attach a repro to the PR for future reference. Ideally, these kinds of repros are added into our tests as well. I ask mostly because the buildingList flag should not be true unless the list is mutable.

[livebranch]

Thanks for the fast response in getting this merged!

Sorry, we don't currently have a concise repro to link to at this point. However, to address your point, consider this simple macro:

func makeProblemMacro(eh parser.ExprHelper, target *expr.Expr, args []*expr.Expr) (*expr.Expr, *common.Error) {
	return eh.Fold(
		"iv",
		target,
		parser.AccumulatorName,
		eh.NewList(),
		eh.LiteralBool(true),
		eh.NewList(eh.LiteralInt(1)), // <---- problem is triggered when a non-mutable list is returned by this expr.
		eh.Ident(parser.AccumulatorName)), nil
}

The key here is the step expression. If it ever returns a non-mutable list, Eval throws the error interface conversion: *types.baseList is not traits.MutableLister..

buildingList being set to true only implies the initial accumulator value is mutable, however, it's the eval result that's asserted of type traits.MutableLister (which it may or may not be, depending on the Eval result).