From c59ecac870c5b5bab27e98c024d8fefaf1e56e7c Mon Sep 17 00:00:00 2001
From: James Bardin <j.bardin@gmail.com>
Date: Tue, 7 Apr 2020 17:11:32 -0400
Subject: [PATCH 1/5] rename module variables and remove extra methods

The variable nodes are not only used during plan and apply, so remove
those from there names. The "plan" node is now
`nodeExpandModuleVariable` and the "apply" node is now just
`nodeModuleVariable`.

Remove unnecessary methods, as the nodeModuleVariable is no longer used
in the full graph transformations.
---
 terraform/node_module_variable.go      | 116 ++++++++-----------------
 terraform/node_module_variable_test.go |  27 +++---
 terraform/node_root_variable.go        |   1 -
 terraform/transform_module_variable.go |   2 +-
 4 files changed, 47 insertions(+), 99 deletions(-)

diff --git a/terraform/node_module_variable.go b/terraform/node_module_variable.go
index 8ee7c98077..53543b47fc 100644
--- a/terraform/node_module_variable.go
+++ b/terraform/node_module_variable.go
@@ -11,9 +11,11 @@ import (
 	"github.com/zclconf/go-cty/cty"
 )
 
-// NodePlannableModuleVariable is the placeholder for an variable that has not yet had
+type ConcreteVariableNodeFunc func(n *nodeExpandModuleVariable) dag.Vertex
+
+// nodeExpandModuleVariable is the placeholder for an variable that has not yet had
 // its module path expanded.
-type NodePlannableModuleVariable struct {
+type nodeExpandModuleVariable struct {
 	Addr   addrs.InputVariable
 	Module addrs.Module
 	Config *configs.Variable
@@ -21,23 +23,23 @@ type NodePlannableModuleVariable struct {
 }
 
 var (
-	_ GraphNodeDynamicExpandable = (*NodePlannableModuleVariable)(nil)
-	_ GraphNodeReferenceOutside  = (*NodePlannableModuleVariable)(nil)
-	_ GraphNodeReferenceable     = (*NodePlannableModuleVariable)(nil)
-	_ GraphNodeReferencer        = (*NodePlannableModuleVariable)(nil)
-	_ graphNodeTemporaryValue    = (*NodePlannableModuleVariable)(nil)
-	_ RemovableIfNotTargeted     = (*NodePlannableModuleVariable)(nil)
+	_ GraphNodeDynamicExpandable = (*nodeExpandModuleVariable)(nil)
+	_ GraphNodeReferenceOutside  = (*nodeExpandModuleVariable)(nil)
+	_ GraphNodeReferenceable     = (*nodeExpandModuleVariable)(nil)
+	_ GraphNodeReferencer        = (*nodeExpandModuleVariable)(nil)
+	_ graphNodeTemporaryValue    = (*nodeExpandModuleVariable)(nil)
+	_ RemovableIfNotTargeted     = (*nodeExpandModuleVariable)(nil)
 )
 
-func (n *NodePlannableModuleVariable) temporaryValue() bool {
+func (n *nodeExpandModuleVariable) temporaryValue() bool {
 	return true
 }
 
-func (n *NodePlannableModuleVariable) DynamicExpand(ctx EvalContext) (*Graph, error) {
+func (n *nodeExpandModuleVariable) DynamicExpand(ctx EvalContext) (*Graph, error) {
 	var g Graph
 	expander := ctx.InstanceExpander()
 	for _, module := range expander.ExpandModule(n.Module) {
-		o := &NodeApplyableModuleVariable{
+		o := &nodeModuleVariable{
 			Addr:           n.Addr.Absolute(module),
 			Config:         n.Config,
 			Expr:           n.Expr,
@@ -48,17 +50,17 @@ func (n *NodePlannableModuleVariable) DynamicExpand(ctx EvalContext) (*Graph, er
 	return &g, nil
 }
 
-func (n *NodePlannableModuleVariable) Name() string {
+func (n *nodeExpandModuleVariable) Name() string {
 	return fmt.Sprintf("%s.%s", n.Module, n.Addr.String())
 }
 
 // GraphNodeModulePath
-func (n *NodePlannableModuleVariable) ModulePath() addrs.Module {
+func (n *nodeExpandModuleVariable) ModulePath() addrs.Module {
 	return n.Module
 }
 
 // GraphNodeReferencer
-func (n *NodePlannableModuleVariable) References() []*addrs.Reference {
+func (n *nodeExpandModuleVariable) References() []*addrs.Reference {
 
 	// If we have no value expression, we cannot depend on anything.
 	if n.Expr == nil {
@@ -86,12 +88,12 @@ func (n *NodePlannableModuleVariable) References() []*addrs.Reference {
 }
 
 // GraphNodeReferenceOutside implementation
-func (n *NodePlannableModuleVariable) ReferenceOutside() (selfPath, referencePath addrs.Module) {
+func (n *nodeExpandModuleVariable) ReferenceOutside() (selfPath, referencePath addrs.Module) {
 	return n.Module, n.Module.Parent()
 }
 
 // GraphNodeReferenceable
-func (n *NodePlannableModuleVariable) ReferenceableAddrs() []addrs.Referenceable {
+func (n *nodeExpandModuleVariable) ReferenceableAddrs() []addrs.Referenceable {
 	// FIXME: References for module variables probably need to be thought out a bit more
 	// Otherwise, we can reference the output via the address itself, or the
 	// module call
@@ -100,18 +102,18 @@ func (n *NodePlannableModuleVariable) ReferenceableAddrs() []addrs.Referenceable
 }
 
 // RemovableIfNotTargeted
-func (n *NodePlannableModuleVariable) RemoveIfNotTargeted() bool {
+func (n *nodeExpandModuleVariable) RemoveIfNotTargeted() bool {
 	return true
 }
 
 // GraphNodeTargetDownstream
-func (n *NodePlannableModuleVariable) TargetDownstream(targetedDeps, untargetedDeps dag.Set) bool {
+func (n *nodeExpandModuleVariable) TargetDownstream(targetedDeps, untargetedDeps dag.Set) bool {
 	return true
 }
 
-// NodeApplyableModuleVariable represents a module variable input during
+// nodeModuleVariable represents a module variable input during
 // the apply step.
-type NodeApplyableModuleVariable struct {
+type nodeModuleVariable struct {
 	Addr   addrs.AbsInputVariableInstance
 	Config *configs.Variable // Config is the var in the config
 	Expr   hcl.Expression    // Expr is the value expression given in the call
@@ -123,92 +125,42 @@ type NodeApplyableModuleVariable struct {
 // Ensure that we are implementing all of the interfaces we think we are
 // implementing.
 var (
-	_ GraphNodeModuleInstance   = (*NodeApplyableModuleVariable)(nil)
-	_ RemovableIfNotTargeted    = (*NodeApplyableModuleVariable)(nil)
-	_ GraphNodeReferenceOutside = (*NodeApplyableModuleVariable)(nil)
-	_ GraphNodeReferenceable    = (*NodeApplyableModuleVariable)(nil)
-	_ GraphNodeReferencer       = (*NodeApplyableModuleVariable)(nil)
-	_ GraphNodeEvalable         = (*NodeApplyableModuleVariable)(nil)
-	_ graphNodeTemporaryValue   = (*NodeApplyableModuleVariable)(nil)
-	_ dag.GraphNodeDotter       = (*NodeApplyableModuleVariable)(nil)
+	_ GraphNodeModuleInstance = (*nodeModuleVariable)(nil)
+	_ RemovableIfNotTargeted  = (*nodeModuleVariable)(nil)
+	_ GraphNodeEvalable       = (*nodeModuleVariable)(nil)
+	_ graphNodeTemporaryValue = (*nodeModuleVariable)(nil)
+	_ dag.GraphNodeDotter     = (*nodeModuleVariable)(nil)
 )
 
-func (n *NodeApplyableModuleVariable) temporaryValue() bool {
+func (n *nodeModuleVariable) temporaryValue() bool {
 	return true
 }
 
-func (n *NodeApplyableModuleVariable) Name() string {
+func (n *nodeModuleVariable) Name() string {
 	return n.Addr.String()
 }
 
 // GraphNodeModuleInstance
-func (n *NodeApplyableModuleVariable) Path() addrs.ModuleInstance {
+func (n *nodeModuleVariable) Path() addrs.ModuleInstance {
 	// We execute in the parent scope (above our own module) because
 	// expressions in our value are resolved in that context.
 	return n.Addr.Module.Parent()
 }
 
 // GraphNodeModulePath
-func (n *NodeApplyableModuleVariable) ModulePath() addrs.Module {
+func (n *nodeModuleVariable) ModulePath() addrs.Module {
 	return n.Addr.Module.Parent().Module()
 }
 
 // RemovableIfNotTargeted
-func (n *NodeApplyableModuleVariable) RemoveIfNotTargeted() bool {
+func (n *nodeModuleVariable) RemoveIfNotTargeted() bool {
 	// We need to add this so that this node will be removed if
 	// it isn't targeted or a dependency of a target.
 	return true
 }
 
-// GraphNodeReferenceOutside implementation
-func (n *NodeApplyableModuleVariable) ReferenceOutside() (selfPath, referencePath addrs.Module) {
-
-	// Module input variables have their value expressions defined in the
-	// context of their calling (parent) module, and so references from
-	// a node of this type should be resolved in the parent module instance.
-	referencePath = n.Addr.Module.Parent().Module()
-
-	// Input variables are _referenced_ from their own module, though.
-	selfPath = n.Addr.Module.Module()
-
-	return // uses named return values
-}
-
-// GraphNodeReferenceable
-func (n *NodeApplyableModuleVariable) ReferenceableAddrs() []addrs.Referenceable {
-	return []addrs.Referenceable{n.Addr.Variable}
-}
-
-// GraphNodeReferencer
-func (n *NodeApplyableModuleVariable) References() []*addrs.Reference {
-
-	// If we have no value expression, we cannot depend on anything.
-	if n.Expr == nil {
-		return nil
-	}
-
-	// Variables in the root don't depend on anything, because their values
-	// are gathered prior to the graph walk and recorded in the context.
-	if len(n.Addr.Module) == 0 {
-		return nil
-	}
-
-	// Otherwise, we depend on anything referenced by our value expression.
-	// We ignore diagnostics here under the assumption that we'll re-eval
-	// all these things later and catch them then; for our purposes here,
-	// we only care about valid references.
-	//
-	// Due to our GraphNodeReferenceOutside implementation, the addresses
-	// returned by this function are interpreted in the _parent_ module from
-	// where our associated variable was declared, which is correct because
-	// our value expression is assigned within a "module" block in the parent
-	// module.
-	refs, _ := lang.ReferencesInExpr(n.Expr)
-	return refs
-}
-
 // GraphNodeEvalable
-func (n *NodeApplyableModuleVariable) EvalTree() EvalNode {
+func (n *nodeModuleVariable) EvalTree() EvalNode {
 	// If we have no value, do nothing
 	if n.Expr == nil {
 		return &EvalNoop{}
@@ -253,7 +205,7 @@ func (n *NodeApplyableModuleVariable) EvalTree() EvalNode {
 }
 
 // dag.GraphNodeDotter impl.
-func (n *NodeApplyableModuleVariable) DotNode(name string, opts *dag.DotOpts) *dag.DotNode {
+func (n *nodeModuleVariable) DotNode(name string, opts *dag.DotOpts) *dag.DotNode {
 	return &dag.DotNode{
 		Name: name,
 		Attrs: map[string]string{
diff --git a/terraform/node_module_variable_test.go b/terraform/node_module_variable_test.go
index 291648e711..209da4f244 100644
--- a/terraform/node_module_variable_test.go
+++ b/terraform/node_module_variable_test.go
@@ -12,9 +12,9 @@ import (
 	"github.com/hashicorp/terraform/configs"
 )
 
-func TestNodeApplyableModuleVariablePath(t *testing.T) {
-	n := &NodeApplyableModuleVariable{
-		Addr: addrs.RootModuleInstance.Child("child", addrs.NoKey).InputVariable("foo"),
+func TestnodeModuleVariablePath(t *testing.T) {
+	n := &nodeModuleVariable{
+		Addr: addrs.RootModuleInstance.InputVariable("foo"),
 		Config: &configs.Variable{
 			Name: "foo",
 		},
@@ -27,9 +27,9 @@ func TestNodeApplyableModuleVariablePath(t *testing.T) {
 	}
 }
 
-func TestNodeApplyableModuleVariableReferenceableName(t *testing.T) {
-	n := &NodeApplyableModuleVariable{
-		Addr: addrs.RootModuleInstance.Child("child", addrs.NoKey).InputVariable("foo"),
+func TestnodeModuleVariableReferenceableName(t *testing.T) {
+	n := &nodeExpandModuleVariable{
+		Addr: addrs.InputVariable{Name: "foo"},
 		Config: &configs.Variable{
 			Name: "foo",
 		},
@@ -59,9 +59,9 @@ func TestNodeApplyableModuleVariableReferenceableName(t *testing.T) {
 
 }
 
-func TestNodeApplyableModuleVariableReference(t *testing.T) {
-	n := &NodeApplyableModuleVariable{
-		Addr: addrs.RootModuleInstance.Child("child", addrs.NoKey).InputVariable("foo"),
+func TestnodeModuleVariableReference(t *testing.T) {
+	n := &nodeExpandModuleVariable{
+		Addr: addrs.InputVariable{Name: "foo"},
 		Config: &configs.Variable{
 			Name: "foo",
 		},
@@ -84,12 +84,9 @@ func TestNodeApplyableModuleVariableReference(t *testing.T) {
 	}
 }
 
-func TestNodeApplyableModuleVariableReference_grandchild(t *testing.T) {
-	n := &NodeApplyableModuleVariable{
-		Addr: addrs.RootModuleInstance.
-			Child("child", addrs.NoKey).
-			Child("grandchild", addrs.NoKey).
-			InputVariable("foo"),
+func TestnodeModuleVariableReference_grandchild(t *testing.T) {
+	n := &nodeExpandModuleVariable{
+		Addr: addrs.InputVariable{Name: "foo"},
 		Config: &configs.Variable{
 			Name: "foo",
 		},
diff --git a/terraform/node_root_variable.go b/terraform/node_root_variable.go
index 1144f3f1fd..9740bf4ebf 100644
--- a/terraform/node_root_variable.go
+++ b/terraform/node_root_variable.go
@@ -15,7 +15,6 @@ type NodeRootVariable struct {
 var (
 	_ GraphNodeModuleInstance = (*NodeRootVariable)(nil)
 	_ GraphNodeReferenceable  = (*NodeRootVariable)(nil)
-	_ dag.GraphNodeDotter     = (*NodeApplyableModuleVariable)(nil)
 )
 
 func (n *NodeRootVariable) Name() string {
diff --git a/terraform/transform_module_variable.go b/terraform/transform_module_variable.go
index c651358882..1d74ba3327 100644
--- a/terraform/transform_module_variable.go
+++ b/terraform/transform_module_variable.go
@@ -105,7 +105,7 @@ func (t *ModuleVariableTransformer) transformSingle(g *Graph, parent, c *configs
 
 		// Add a plannable node, as the variable may expand
 		// during module expansion
-		node := &NodePlannableModuleVariable{
+		node := &nodeExpandModuleVariable{
 			Addr: addrs.InputVariable{
 				Name: v.Name,
 			},

From d060a3d0e83aad0162e888e0043ff5d2cc907b7b Mon Sep 17 00:00:00 2001
From: James Bardin <j.bardin@gmail.com>
Date: Tue, 7 Apr 2020 19:30:18 -0400
Subject: [PATCH 2/5] eval variables with unknown expansion data

While we don't have any expansion info during validation, we can try to
evaluate variable expressions to catch some basic errors. Do this by
creating module instance RepetitionData with unknown values. This
unfortunately will still miss the incorrect usage of count/each values,
but that would require the module call's each mode, which is not
available at this time.
---
 terraform/context_validate_test.go     |  2 +-
 terraform/eval_variable.go             | 42 +++++++++++++++-----------
 terraform/node_module_variable.go      | 18 +++++++----
 terraform/node_root_variable.go        |  2 --
 terraform/transform_module_variable.go |  1 -
 5 files changed, 38 insertions(+), 27 deletions(-)

diff --git a/terraform/context_validate_test.go b/terraform/context_validate_test.go
index 8b380ba03f..178f291746 100644
--- a/terraform/context_validate_test.go
+++ b/terraform/context_validate_test.go
@@ -1435,7 +1435,7 @@ resource "aws_instance" "foo" {
 module "nested" {
   count = 2
   source = "./nested"
-  input = 2
+  input = count.index
 }
 `,
 		"mod/nested/main.tf": `
diff --git a/terraform/eval_variable.go b/terraform/eval_variable.go
index e5ff7cdf74..5fc90245b5 100644
--- a/terraform/eval_variable.go
+++ b/terraform/eval_variable.go
@@ -8,6 +8,7 @@ import (
 	"github.com/hashicorp/hcl/v2"
 	"github.com/hashicorp/terraform/addrs"
 	"github.com/hashicorp/terraform/configs"
+	"github.com/hashicorp/terraform/instances"
 	"github.com/hashicorp/terraform/tfdiags"
 	"github.com/zclconf/go-cty/cty"
 	"github.com/zclconf/go-cty/cty/convert"
@@ -42,12 +43,12 @@ type EvalModuleCallArgument struct {
 	Expr           hcl.Expression
 	ModuleInstance addrs.ModuleInstance
 
-	// If this flag is set, any diagnostics are discarded and this operation
-	// will always succeed, though may produce an unknown value in the
-	// event of an error.
-	IgnoreDiagnostics bool
-
 	Values map[string]cty.Value
+
+	// validateOnly indicates that this evaluation is only for config
+	// validation, and we will not have any expansion module instance
+	// repetition data.
+	validateOnly bool
 }
 
 func (n *EvalModuleCallArgument) Eval(ctx EvalContext) (interface{}, error) {
@@ -69,9 +70,24 @@ func (n *EvalModuleCallArgument) Eval(ctx EvalContext) (interface{}, error) {
 		return nil, nil
 	}
 
-	// Get the repetition data for this module instance,
-	// so we can create the appropriate scope for evaluating our expression
-	moduleInstanceRepetitionData := ctx.InstanceExpander().GetModuleInstanceRepetitionData(n.ModuleInstance)
+	var moduleInstanceRepetitionData instances.RepetitionData
+
+	switch {
+	case n.validateOnly:
+		// the instance expander does not track unknown expansion values, so we
+		// have to assume all RepetitionData is unknown.
+		moduleInstanceRepetitionData = instances.RepetitionData{
+			CountIndex: cty.UnknownVal(cty.Number),
+			EachKey:    cty.UnknownVal(cty.String),
+			EachValue:  cty.DynamicVal,
+		}
+
+	default:
+		// Get the repetition data for this module instance,
+		// so we can create the appropriate scope for evaluating our expression
+		moduleInstanceRepetitionData = ctx.InstanceExpander().GetModuleInstanceRepetitionData(n.ModuleInstance)
+	}
+
 	scope := ctx.EvaluationScope(nil, moduleInstanceRepetitionData)
 	val, diags := scope.EvalExpr(expr, cty.DynamicPseudoType)
 
@@ -96,9 +112,6 @@ func (n *EvalModuleCallArgument) Eval(ctx EvalContext) (interface{}, error) {
 	}
 
 	n.Values[name] = val
-	if n.IgnoreDiagnostics {
-		return nil, nil
-	}
 	return nil, diags.ErrWithWarnings()
 }
 
@@ -116,15 +129,10 @@ type evalVariableValidations struct {
 	// This will be nil for root module variables, because their values come
 	// from outside the configuration.
 	Expr hcl.Expression
-
-	// If this flag is set, this node becomes a no-op.
-	// This is here for consistency with EvalModuleCallArgument so that it
-	// can be populated with the same value, where needed.
-	IgnoreDiagnostics bool
 }
 
 func (n *evalVariableValidations) Eval(ctx EvalContext) (interface{}, error) {
-	if n.Config == nil || n.IgnoreDiagnostics || len(n.Config.Validations) == 0 {
+	if n.Config == nil || len(n.Config.Validations) == 0 {
 		log.Printf("[TRACE] evalVariableValidations: not active for %s, so skipping", n.Addr)
 		return nil, nil
 	}
diff --git a/terraform/node_module_variable.go b/terraform/node_module_variable.go
index 53543b47fc..62ebb19132 100644
--- a/terraform/node_module_variable.go
+++ b/terraform/node_module_variable.go
@@ -11,8 +11,6 @@ import (
 	"github.com/zclconf/go-cty/cty"
 )
 
-type ConcreteVariableNodeFunc func(n *nodeExpandModuleVariable) dag.Vertex
-
 // nodeExpandModuleVariable is the placeholder for an variable that has not yet had
 // its module path expanded.
 type nodeExpandModuleVariable struct {
@@ -176,15 +174,25 @@ func (n *nodeModuleVariable) EvalTree() EvalNode {
 		Nodes: []EvalNode{
 			&EvalOpFilter{
 				Ops: []walkOperation{walkRefresh, walkPlan, walkApply,
-					walkDestroy, walkValidate},
+					walkDestroy},
 				Node: &EvalModuleCallArgument{
 					Addr:           n.Addr.Variable,
 					Config:         n.Config,
 					Expr:           n.Expr,
 					ModuleInstance: n.ModuleInstance,
 					Values:         vals,
+				},
+			},
 
-					IgnoreDiagnostics: false,
+			&EvalOpFilter{
+				Ops: []walkOperation{walkValidate},
+				Node: &EvalModuleCallArgument{
+					Addr:           n.Addr.Variable,
+					Config:         n.Config,
+					Expr:           n.Expr,
+					ModuleInstance: n.ModuleInstance,
+					Values:         vals,
+					validateOnly:   true,
 				},
 			},
 
@@ -197,8 +205,6 @@ func (n *nodeModuleVariable) EvalTree() EvalNode {
 				Addr:   n.Addr,
 				Config: n.Config,
 				Expr:   n.Expr,
-
-				IgnoreDiagnostics: false,
 			},
 		},
 	}
diff --git a/terraform/node_root_variable.go b/terraform/node_root_variable.go
index 9740bf4ebf..f9814f375a 100644
--- a/terraform/node_root_variable.go
+++ b/terraform/node_root_variable.go
@@ -50,8 +50,6 @@ func (n *NodeRootVariable) EvalTree() EvalNode {
 		Addr:   addrs.RootModuleInstance.InputVariable(n.Addr.Name),
 		Config: n.Config,
 		Expr:   nil, // not set for root module variables
-
-		IgnoreDiagnostics: false,
 	}
 }
 
diff --git a/terraform/transform_module_variable.go b/terraform/transform_module_variable.go
index 1d74ba3327..99a86c01d9 100644
--- a/terraform/transform_module_variable.go
+++ b/terraform/transform_module_variable.go
@@ -113,7 +113,6 @@ func (t *ModuleVariableTransformer) transformSingle(g *Graph, parent, c *configs
 			Config: v,
 			Expr:   expr,
 		}
-
 		g.Add(node)
 	}
 

From b1bc7a792b9076ad7397a8ec00be888151de844e Mon Sep 17 00:00:00 2001
From: James Bardin <j.bardin@gmail.com>
Date: Wed, 8 Apr 2020 17:09:17 -0400
Subject: [PATCH 3/5] rename and cleanup use of count/for_each eval func

Stop evaluating count and for each if they aren't set in the config.
Remove "Resource" from the function names, as they are also now used
with modules.
---
 terraform/eval_apply.go            |  4 +-
 terraform/eval_count.go            | 45 ++++++++++--------
 terraform/eval_diff.go             |  2 +-
 terraform/eval_for_each.go         | 74 ++++++++++++++++--------------
 terraform/eval_for_each_test.go    | 24 ++++------
 terraform/eval_read_data.go        |  2 +-
 terraform/eval_state.go            | 46 +++++++++----------
 terraform/eval_validate.go         | 11 +++--
 terraform/node_data_refresh.go     | 65 +++++++++++++-------------
 terraform/node_module_expand.go    |  4 +-
 terraform/node_resource_refresh.go | 44 +++++++++---------
 11 files changed, 165 insertions(+), 156 deletions(-)

diff --git a/terraform/eval_apply.go b/terraform/eval_apply.go
index 788542a448..ed8e7bc106 100644
--- a/terraform/eval_apply.go
+++ b/terraform/eval_apply.go
@@ -61,7 +61,7 @@ func (n *EvalApply) Eval(ctx EvalContext) (interface{}, error) {
 	configVal := cty.NullVal(cty.DynamicPseudoType)
 	if n.Config != nil {
 		var configDiags tfdiags.Diagnostics
-		forEach, _ := evaluateResourceForEachExpression(n.Config.ForEach, ctx)
+		forEach, _ := evaluateForEachExpression(n.Config.ForEach, ctx)
 		keyData := EvalDataForInstanceKey(n.Addr.Key, forEach)
 		configVal, _, configDiags = ctx.EvaluateBlock(n.Config.Config, schema, nil, keyData)
 		diags = diags.Append(configDiags)
@@ -595,7 +595,7 @@ func (n *EvalApplyProvisioners) apply(ctx EvalContext, provs []*configs.Provisio
 		// in its result. That's okay because each.value is prohibited for
 		// destroy-time provisioners.
 		if n.When != configs.ProvisionerWhenDestroy {
-			m, forEachDiags := evaluateResourceForEachExpression(n.ResourceConfig.ForEach, ctx)
+			m, forEachDiags := evaluateForEachExpression(n.ResourceConfig.ForEach, ctx)
 			diags = diags.Append(forEachDiags)
 			forEach = m
 		}
diff --git a/terraform/eval_count.go b/terraform/eval_count.go
index 3da947f1fd..99ddeafee6 100644
--- a/terraform/eval_count.go
+++ b/terraform/eval_count.go
@@ -11,7 +11,7 @@ import (
 	"github.com/zclconf/go-cty/cty/gocty"
 )
 
-// evaluateResourceCountExpression is our standard mechanism for interpreting an
+// evaluateCountExpression is our standard mechanism for interpreting an
 // expression given for a "count" argument on a resource. This should be called
 // from the DynamicExpand of a node representing a resource in order to
 // determine the final count value.
@@ -24,9 +24,9 @@ import (
 //
 // If error diagnostics are returned then the result is always the meaningless
 // placeholder value -1.
-func evaluateResourceCountExpression(expr hcl.Expression, ctx EvalContext) (int, tfdiags.Diagnostics) {
-	count, known, diags := evaluateResourceCountExpressionKnown(expr, ctx)
-	if !known {
+func evaluateCountExpression(expr hcl.Expression, ctx EvalContext) (int, tfdiags.Diagnostics) {
+	countVal, diags := evaluateCountExpressionValue(expr, ctx)
+	if !countVal.IsKnown() {
 		// Currently this is a rather bad outcome from a UX standpoint, since we have
 		// no real mechanism to deal with this situation and all we can do is produce
 		// an error message.
@@ -40,22 +40,29 @@ func evaluateResourceCountExpression(expr hcl.Expression, ctx EvalContext) (int,
 			Subject:  expr.Range().Ptr(),
 		})
 	}
-	return count, diags
+
+	if countVal.IsNull() || !countVal.IsKnown() {
+		return -1, diags
+	}
+
+	count, _ := countVal.AsBigFloat().Int64()
+	return int(count), diags
 }
 
-// evaluateResourceCountExpressionKnown is like evaluateResourceCountExpression
-// except that it handles an unknown result by returning count = 0 and
-// a known = false, rather than by reporting the unknown value as an error
-// diagnostic.
-func evaluateResourceCountExpressionKnown(expr hcl.Expression, ctx EvalContext) (count int, known bool, diags tfdiags.Diagnostics) {
+// evaluateCountExpressionValue is like evaluateCountExpression
+// except that it returns a cty.Value which must be a cty.Number and can be
+// unknown.
+func evaluateCountExpressionValue(expr hcl.Expression, ctx EvalContext) (cty.Value, tfdiags.Diagnostics) {
+	var diags tfdiags.Diagnostics
+	nullCount := cty.NullVal(cty.Number)
 	if expr == nil {
-		return -1, true, nil
+		return nullCount, nil
 	}
 
 	countVal, countDiags := ctx.EvaluateExpr(expr, cty.Number, nil)
 	diags = diags.Append(countDiags)
 	if diags.HasErrors() {
-		return -1, true, diags
+		return nullCount, diags
 	}
 
 	switch {
@@ -66,11 +73,13 @@ func evaluateResourceCountExpressionKnown(expr hcl.Expression, ctx EvalContext)
 			Detail:   `The given "count" argument value is null. An integer is required.`,
 			Subject:  expr.Range().Ptr(),
 		})
-		return -1, true, diags
+		return nullCount, diags
+
 	case !countVal.IsKnown():
-		return 0, false, diags
+		return cty.UnknownVal(cty.Number), diags
 	}
 
+	var count int
 	err := gocty.FromCtyValue(countVal, &count)
 	if err != nil {
 		diags = diags.Append(&hcl.Diagnostic{
@@ -79,7 +88,7 @@ func evaluateResourceCountExpressionKnown(expr hcl.Expression, ctx EvalContext)
 			Detail:   fmt.Sprintf(`The given "count" argument value is unsuitable: %s.`, err),
 			Subject:  expr.Range().Ptr(),
 		})
-		return -1, true, diags
+		return nullCount, diags
 	}
 	if count < 0 {
 		diags = diags.Append(&hcl.Diagnostic{
@@ -88,10 +97,10 @@ func evaluateResourceCountExpressionKnown(expr hcl.Expression, ctx EvalContext)
 			Detail:   `The given "count" argument value is unsuitable: negative numbers are not supported.`,
 			Subject:  expr.Range().Ptr(),
 		})
-		return -1, true, diags
+		return nullCount, diags
 	}
 
-	return count, true, diags
+	return countVal, diags
 }
 
 // fixResourceCountSetTransition is a helper function to fix up the state when a
@@ -101,7 +110,7 @@ func evaluateResourceCountExpressionKnown(expr hcl.Expression, ctx EvalContext)
 //
 // The correct time to call this function is in the DynamicExpand method for
 // a node representing a resource, just after evaluating the count with
-// evaluateResourceCountExpression, and before any other analysis of the
+// evaluateCountExpression, and before any other analysis of the
 // state such as orphan detection.
 //
 // This function calls methods on the given EvalContext to update the current
diff --git a/terraform/eval_diff.go b/terraform/eval_diff.go
index c5c24b49e7..509bd5bbcd 100644
--- a/terraform/eval_diff.go
+++ b/terraform/eval_diff.go
@@ -133,7 +133,7 @@ func (n *EvalDiff) Eval(ctx EvalContext) (interface{}, error) {
 		// Should be caught during validation, so we don't bother with a pretty error here
 		return nil, fmt.Errorf("provider does not support resource type %q", n.Addr.Resource.Type)
 	}
-	forEach, _ := evaluateResourceForEachExpression(n.Config.ForEach, ctx)
+	forEach, _ := evaluateForEachExpression(n.Config.ForEach, ctx)
 	keyData := EvalDataForInstanceKey(n.Addr.Key, forEach)
 	configVal, _, configDiags := ctx.EvaluateBlock(config.Config, schema, nil, keyData)
 	diags = diags.Append(configDiags)
diff --git a/terraform/eval_for_each.go b/terraform/eval_for_each.go
index 599995728c..978e034d91 100644
--- a/terraform/eval_for_each.go
+++ b/terraform/eval_for_each.go
@@ -8,14 +8,14 @@ import (
 	"github.com/zclconf/go-cty/cty"
 )
 
-// evaluateResourceForEachExpression interprets a "for_each" argument on a resource.
+// evaluateForEachExpression interprets a "for_each" argument on a resource.
 //
 // Returns a cty.Value map, and diagnostics if necessary. It will return nil if
 // the expression is nil, and is used to distinguish between an unset for_each and an
 // empty map
-func evaluateResourceForEachExpression(expr hcl.Expression, ctx EvalContext) (forEach map[string]cty.Value, diags tfdiags.Diagnostics) {
-	forEachMap, known, diags := evaluateResourceForEachExpressionKnown(expr, ctx)
-	if !known {
+func evaluateForEachExpression(expr hcl.Expression, ctx EvalContext) (forEach map[string]cty.Value, diags tfdiags.Diagnostics) {
+	forEachVal, diags := evaluateForEachExpressionValue(expr, ctx)
+	if !forEachVal.IsKnown() {
 		// Attach a diag as we do with count, with the same downsides
 		diags = diags.Append(&hcl.Diagnostic{
 			Severity: hcl.DiagError,
@@ -24,23 +24,31 @@ func evaluateResourceForEachExpression(expr hcl.Expression, ctx EvalContext) (fo
 			Subject:  expr.Range().Ptr(),
 		})
 	}
-	return forEachMap, diags
+
+	if forEachVal.IsNull() || !forEachVal.IsKnown() || forEachVal.LengthInt() == 0 {
+		// we check length, because an empty set return a nil map
+		return map[string]cty.Value{}, diags
+	}
+
+	return forEachVal.AsValueMap(), diags
 }
 
-// evaluateResourceForEachExpressionKnown is like evaluateResourceForEachExpression
-// except that it handles an unknown result by returning an empty map and
-// a known = false, rather than by reporting the unknown value as an error
-// diagnostic.
-func evaluateResourceForEachExpressionKnown(expr hcl.Expression, ctx EvalContext) (forEach map[string]cty.Value, known bool, diags tfdiags.Diagnostics) {
+// evaluateForEachExpressionValue is like evaluateForEachExpression
+// except that it returns a cty.Value map or set which can be unknown.
+func evaluateForEachExpressionValue(expr hcl.Expression, ctx EvalContext) (cty.Value, tfdiags.Diagnostics) {
+	var diags tfdiags.Diagnostics
+	nullMap := cty.NullVal(cty.Map(cty.DynamicPseudoType))
+
 	if expr == nil {
-		return nil, true, nil
+		return nullMap, diags
 	}
 
 	forEachVal, forEachDiags := ctx.EvaluateExpr(expr, cty.DynamicPseudoType, nil)
 	diags = diags.Append(forEachDiags)
 	if diags.HasErrors() {
-		return nil, true, diags
+		return nullMap, diags
 	}
+	ty := forEachVal.Type()
 
 	switch {
 	case forEachVal.IsNull():
@@ -50,44 +58,42 @@ func evaluateResourceForEachExpressionKnown(expr hcl.Expression, ctx EvalContext
 			Detail:   `The given "for_each" argument value is unsuitable: the given "for_each" argument value is null. A map, or set of strings is allowed.`,
 			Subject:  expr.Range().Ptr(),
 		})
-		return nil, true, diags
+		return nullMap, diags
 	case !forEachVal.IsKnown():
-		return map[string]cty.Value{}, false, diags
-	}
+		// ensure that we have a map, and not a DynamicValue
+		return cty.UnknownVal(cty.Map(cty.DynamicPseudoType)), diags
 
-	if !forEachVal.CanIterateElements() || forEachVal.Type().IsListType() || forEachVal.Type().IsTupleType() {
+	case !(ty.IsMapType() || ty.IsSetType() || ty.IsObjectType()):
 		diags = diags.Append(&hcl.Diagnostic{
 			Severity: hcl.DiagError,
 			Summary:  "Invalid for_each argument",
-			Detail:   fmt.Sprintf(`The given "for_each" argument value is unsuitable: the "for_each" argument must be a map, or set of strings, and you have provided a value of type %s.`, forEachVal.Type().FriendlyName()),
+			Detail:   fmt.Sprintf(`The given "for_each" argument value is unsuitable: the "for_each" argument must be a map, or set of strings, and you have provided a value of type %s.`, ty.FriendlyName()),
 			Subject:  expr.Range().Ptr(),
 		})
-		return nil, true, diags
+		return nullMap, diags
+
+	case forEachVal.LengthInt() == 0:
+		// If the map is empty ({}), return an empty map, because cty will
+		// return nil when representing {} AsValueMap This also covers an empty
+		// set (toset([]))
+		return forEachVal, diags
 	}
 
-	// If the map is empty ({}), return an empty map, because cty will return nil when representing {} AsValueMap
-	// This also covers an empty set (toset([]))
-	if forEachVal.LengthInt() == 0 {
-		return map[string]cty.Value{}, true, diags
-	}
-
-	if forEachVal.Type().IsSetType() {
-		if forEachVal.Type().ElementType() != cty.String {
+	if ty.IsSetType() {
+		if ty.ElementType() != cty.String {
 			diags = diags.Append(&hcl.Diagnostic{
 				Severity: hcl.DiagError,
 				Summary:  "Invalid for_each set argument",
 				Detail:   fmt.Sprintf(`The given "for_each" argument value is unsuitable: "for_each" supports maps and sets of strings, but you have provided a set containing type %s.`, forEachVal.Type().ElementType().FriendlyName()),
 				Subject:  expr.Range().Ptr(),
 			})
-			return nil, true, diags
+			return cty.NullVal(ty), diags
 		}
 
-		// A set may contain unknown values that must be
-		// discovered by checking with IsWhollyKnown (which iterates through the
-		// structure), while for maps in cty, keys can never be unknown or null,
-		// thus the earlier IsKnown check suffices for maps
+		// since we can't use a set values that are unknown, we treat the
+		// entire set as unknown
 		if !forEachVal.IsWhollyKnown() {
-			return map[string]cty.Value{}, false, diags
+			return cty.UnknownVal(ty), diags
 		}
 
 		// A set of strings may contain null, which makes it impossible to
@@ -102,10 +108,10 @@ func evaluateResourceForEachExpressionKnown(expr hcl.Expression, ctx EvalContext
 					Detail:   fmt.Sprintf(`The given "for_each" argument value is unsuitable: "for_each" sets must not contain null values.`),
 					Subject:  expr.Range().Ptr(),
 				})
-				return nil, true, diags
+				return cty.NullVal(ty), diags
 			}
 		}
 	}
 
-	return forEachVal.AsValueMap(), true, nil
+	return forEachVal, nil
 }
diff --git a/terraform/eval_for_each_test.go b/terraform/eval_for_each_test.go
index 5f0cabcf0b..83aa4595e9 100644
--- a/terraform/eval_for_each_test.go
+++ b/terraform/eval_for_each_test.go
@@ -12,7 +12,7 @@ import (
 	"github.com/zclconf/go-cty/cty"
 )
 
-func TestEvaluateResourceForEachExpression_valid(t *testing.T) {
+func TestEvaluateForEachExpression_valid(t *testing.T) {
 	tests := map[string]struct {
 		Expr       hcl.Expression
 		ForEachMap map[string]cty.Value
@@ -58,7 +58,7 @@ func TestEvaluateResourceForEachExpression_valid(t *testing.T) {
 		t.Run(name, func(t *testing.T) {
 			ctx := &MockEvalContext{}
 			ctx.installSimpleEval()
-			forEachMap, diags := evaluateResourceForEachExpression(test.Expr, ctx)
+			forEachMap, diags := evaluateForEachExpression(test.Expr, ctx)
 
 			if len(diags) != 0 {
 				t.Errorf("unexpected diagnostics %s", spew.Sdump(diags))
@@ -75,7 +75,7 @@ func TestEvaluateResourceForEachExpression_valid(t *testing.T) {
 	}
 }
 
-func TestEvaluateResourceForEachExpression_errors(t *testing.T) {
+func TestEvaluateForEachExpression_errors(t *testing.T) {
 	tests := map[string]struct {
 		Expr                     hcl.Expression
 		Summary, DetailSubstring string
@@ -131,7 +131,7 @@ func TestEvaluateResourceForEachExpression_errors(t *testing.T) {
 		t.Run(name, func(t *testing.T) {
 			ctx := &MockEvalContext{}
 			ctx.installSimpleEval()
-			_, diags := evaluateResourceForEachExpression(test.Expr, ctx)
+			_, diags := evaluateForEachExpression(test.Expr, ctx)
 
 			if len(diags) != 1 {
 				t.Fatalf("got %d diagnostics; want 1", diags)
@@ -149,7 +149,7 @@ func TestEvaluateResourceForEachExpression_errors(t *testing.T) {
 	}
 }
 
-func TestEvaluateResourceForEachExpressionKnown(t *testing.T) {
+func TestEvaluateForEachExpressionKnown(t *testing.T) {
 	tests := map[string]hcl.Expression{
 		"unknown string set": hcltest.MockExprLiteral(cty.UnknownVal(cty.Set(cty.String))),
 		"unknown map":        hcltest.MockExprLiteral(cty.UnknownVal(cty.Map(cty.Bool))),
@@ -159,23 +159,15 @@ func TestEvaluateResourceForEachExpressionKnown(t *testing.T) {
 		t.Run(name, func(t *testing.T) {
 			ctx := &MockEvalContext{}
 			ctx.installSimpleEval()
-			forEachMap, known, diags := evaluateResourceForEachExpressionKnown(expr, ctx)
+			forEachVal, diags := evaluateForEachExpressionValue(expr, ctx)
 
 			if len(diags) != 0 {
 				t.Errorf("unexpected diagnostics %s", spew.Sdump(diags))
 			}
 
-			if known {
-				t.Errorf("got %v known, want false", known)
+			if forEachVal.IsKnown() {
+				t.Error("got known, want unknown")
 			}
-
-			if len(forEachMap) != 0 {
-				t.Errorf(
-					"expected empty map\ngot:  %s",
-					spew.Sdump(forEachMap),
-				)
-			}
-
 		})
 	}
 }
diff --git a/terraform/eval_read_data.go b/terraform/eval_read_data.go
index ef9fad0384..b3f7fdbe5a 100644
--- a/terraform/eval_read_data.go
+++ b/terraform/eval_read_data.go
@@ -96,7 +96,7 @@ func (n *EvalReadData) Eval(ctx EvalContext) (interface{}, error) {
 	objTy := schema.ImpliedType()
 	priorVal := cty.NullVal(objTy) // for data resources, prior is always null because we start fresh every time
 
-	forEach, _ := evaluateResourceForEachExpression(n.Config.ForEach, ctx)
+	forEach, _ := evaluateForEachExpression(n.Config.ForEach, ctx)
 	keyData := EvalDataForInstanceKey(n.Addr.Key, forEach)
 
 	var configDiags tfdiags.Diagnostics
diff --git a/terraform/eval_state.go b/terraform/eval_state.go
index 7f626e96ff..d12adf3743 100644
--- a/terraform/eval_state.go
+++ b/terraform/eval_state.go
@@ -466,36 +466,32 @@ func (n *EvalWriteResourceState) Eval(ctx EvalContext) (interface{}, error) {
 	// can refer to it. Since this node represents the abstract module, we need
 	// to expand the module here to create all resources.
 	expander := ctx.InstanceExpander()
-	count, countDiags := evaluateResourceCountExpression(n.Config.Count, ctx)
-	diags = diags.Append(countDiags)
-	if countDiags.HasErrors() {
-		return nil, diags.Err()
-	}
 
-	eachMode := states.NoEach
-	if count >= 0 { // -1 signals "count not set"
-		eachMode = states.EachList
-	}
+	switch {
+	case n.Config.Count != nil:
+		count, countDiags := evaluateCountExpression(n.Config.Count, ctx)
+		diags = diags.Append(countDiags)
+		if countDiags.HasErrors() {
+			return nil, diags.Err()
+		}
 
-	forEach, forEachDiags := evaluateResourceForEachExpression(n.Config.ForEach, ctx)
-	diags = diags.Append(forEachDiags)
-	if forEachDiags.HasErrors() {
-		return nil, diags.Err()
-	}
-
-	if forEach != nil {
-		eachMode = states.EachMap
-	}
-	// This method takes care of all of the business logic of updating this
-	// while ensuring that any existing instances are preserved, etc.
-	state.SetResourceMeta(n.Addr, eachMode, n.ProviderAddr)
-
-	switch eachMode {
-	case states.EachList:
+		state.SetResourceMeta(n.Addr, states.EachList, n.ProviderAddr)
 		expander.SetResourceCount(n.Addr.Module, n.Addr.Resource, count)
-	case states.EachMap:
+
+	case n.Config.ForEach != nil:
+		forEach, forEachDiags := evaluateForEachExpression(n.Config.ForEach, ctx)
+		diags = diags.Append(forEachDiags)
+		if forEachDiags.HasErrors() {
+			return nil, diags.Err()
+		}
+
+		// This method takes care of all of the business logic of updating this
+		// while ensuring that any existing instances are preserved, etc.
+		state.SetResourceMeta(n.Addr, states.EachMap, n.ProviderAddr)
 		expander.SetResourceForEach(n.Addr.Module, n.Addr.Resource, forEach)
+
 	default:
+		state.SetResourceMeta(n.Addr, states.NoEach, n.ProviderAddr)
 		expander.SetResourceSingle(n.Addr.Module, n.Addr.Resource)
 	}
 
diff --git a/terraform/eval_validate.go b/terraform/eval_validate.go
index 9146d5577e..00b80d37b5 100644
--- a/terraform/eval_validate.go
+++ b/terraform/eval_validate.go
@@ -375,7 +375,9 @@ func (n *EvalValidateResource) Eval(ctx EvalContext) (interface{}, error) {
 	mode := cfg.Mode
 
 	keyData := EvalDataForNoInstanceKey
-	if n.Config.Count != nil {
+
+	switch {
+	case n.Config.Count != nil:
 		// If the config block has count, we'll evaluate with an unknown
 		// number as count.index so we can still type check even though
 		// we won't expand count until the plan phase.
@@ -387,9 +389,8 @@ func (n *EvalValidateResource) Eval(ctx EvalContext) (interface{}, error) {
 		// of this will happen when we DynamicExpand during the plan walk.
 		countDiags := n.validateCount(ctx, n.Config.Count)
 		diags = diags.Append(countDiags)
-	}
 
-	if n.Config.ForEach != nil {
+	case n.Config.ForEach != nil:
 		keyData = InstanceKeyEvalData{
 			EachKey:   cty.UnknownVal(cty.String),
 			EachValue: cty.UnknownVal(cty.DynamicPseudoType),
@@ -608,10 +609,10 @@ func (n *EvalValidateResource) validateCount(ctx EvalContext, expr hcl.Expressio
 }
 
 func (n *EvalValidateResource) validateForEach(ctx EvalContext, expr hcl.Expression) (diags tfdiags.Diagnostics) {
-	_, known, forEachDiags := evaluateResourceForEachExpressionKnown(expr, ctx)
+	val, forEachDiags := evaluateForEachExpressionValue(expr, ctx)
 	// If the value isn't known then that's the best we can do for now, but
 	// we'll check more thoroughly during the plan walk
-	if !known {
+	if !val.IsKnown() {
 		return diags
 	}
 
diff --git a/terraform/node_data_refresh.go b/terraform/node_data_refresh.go
index a01334c86f..8e1b9d5f05 100644
--- a/terraform/node_data_refresh.go
+++ b/terraform/node_data_refresh.go
@@ -65,43 +65,46 @@ func (n *NodeRefreshableDataResource) Path() addrs.ModuleInstance {
 func (n *NodeRefreshableDataResource) DynamicExpand(ctx EvalContext) (*Graph, error) {
 	var diags tfdiags.Diagnostics
 
-	count, countKnown, countDiags := evaluateResourceCountExpressionKnown(n.Config.Count, ctx)
-	diags = diags.Append(countDiags)
-	if countDiags.HasErrors() {
-		return nil, diags.Err()
-	}
-	if !countKnown {
-		// If the count isn't known yet, we'll skip refreshing and try expansion
-		// again during the plan walk.
-		return nil, nil
-	}
+	expander := ctx.InstanceExpander()
 
-	forEachMap, forEachKnown, forEachDiags := evaluateResourceForEachExpressionKnown(n.Config.ForEach, ctx)
-	diags = diags.Append(forEachDiags)
-	if forEachDiags.HasErrors() {
-		return nil, diags.Err()
-	}
-	if !forEachKnown {
-		// If the for_each isn't known yet, we'll skip refreshing and try expansion
-		// again during the plan walk.
-		return nil, nil
+	switch {
+	case n.Config.Count != nil:
+		count, countDiags := evaluateCountExpressionValue(n.Config.Count, ctx)
+		diags = diags.Append(countDiags)
+		if countDiags.HasErrors() {
+			return nil, diags.Err()
+		}
+		if !count.IsKnown() {
+			// If the count isn't known yet, we'll skip refreshing and try expansion
+			// again during the plan walk.
+			return nil, nil
+		}
+
+		c, _ := count.AsBigFloat().Int64()
+		expander.SetResourceCount(n.Addr.Module, n.Addr.Resource, int(c))
+
+	case n.Config.ForEach != nil:
+		forEachVal, forEachDiags := evaluateForEachExpressionValue(n.Config.ForEach, ctx)
+		diags = diags.Append(forEachDiags)
+		if forEachDiags.HasErrors() {
+			return nil, diags.Err()
+		}
+		if !forEachVal.IsKnown() {
+			// If the for_each isn't known yet, we'll skip refreshing and try expansion
+			// again during the plan walk.
+			return nil, nil
+		}
+
+		expander.SetResourceForEach(n.Addr.Module, n.Addr.Resource, forEachVal.AsValueMap())
+
+	default:
+		expander.SetResourceSingle(n.Addr.Module, n.Addr.Resource)
 	}
 
 	// Next we need to potentially rename an instance address in the state
 	// if we're transitioning whether "count" is set at all.
-	fixResourceCountSetTransition(ctx, n.ResourceAddr(), count != -1)
+	fixResourceCountSetTransition(ctx, n.ResourceAddr(), n.Config.Count != nil)
 
-	// Inform our instance expander about our expansion results above,
-	// and then use it to calculate the instance addresses we'll expand for.
-	expander := ctx.InstanceExpander()
-	switch {
-	case count >= 0:
-		expander.SetResourceCount(n.Addr.Module, n.Addr.Resource, count)
-	case forEachMap != nil:
-		expander.SetResourceForEach(n.Addr.Module, n.Addr.Resource, forEachMap)
-	default:
-		expander.SetResourceSingle(n.Addr.Module, n.Addr.Resource)
-	}
 	instanceAddrs := expander.ExpandResource(n.Addr)
 
 	// Our graph transformers require access to the full state, so we'll
diff --git a/terraform/node_module_expand.go b/terraform/node_module_expand.go
index a0669b7d4d..057ac26144 100644
--- a/terraform/node_module_expand.go
+++ b/terraform/node_module_expand.go
@@ -229,14 +229,14 @@ func (n *evalPrepareModuleExpansion) Eval(ctx EvalContext) (interface{}, error)
 
 		switch {
 		case n.ModuleCall.Count != nil:
-			count, diags := evaluateResourceCountExpression(n.ModuleCall.Count, ctx)
+			count, diags := evaluateCountExpression(n.ModuleCall.Count, ctx)
 			if diags.HasErrors() {
 				return nil, diags.Err()
 			}
 			expander.SetModuleCount(module, call, count)
 
 		case n.ModuleCall.ForEach != nil:
-			forEach, diags := evaluateResourceForEachExpression(n.ModuleCall.ForEach, ctx)
+			forEach, diags := evaluateForEachExpression(n.ModuleCall.ForEach, ctx)
 			if diags.HasErrors() {
 				return nil, diags.Err()
 			}
diff --git a/terraform/node_resource_refresh.go b/terraform/node_resource_refresh.go
index fa73b275b8..0956bcd884 100644
--- a/terraform/node_resource_refresh.go
+++ b/terraform/node_resource_refresh.go
@@ -89,32 +89,34 @@ func (n *NodeRefreshableManagedResource) Path() addrs.ModuleInstance {
 func (n *NodeRefreshableManagedResource) DynamicExpand(ctx EvalContext) (*Graph, error) {
 	var diags tfdiags.Diagnostics
 
-	count, countDiags := evaluateResourceCountExpression(n.Config.Count, ctx)
-	diags = diags.Append(countDiags)
-	if countDiags.HasErrors() {
-		return nil, diags.Err()
-	}
+	expander := ctx.InstanceExpander()
+	// Inform our instance expander about our expansion results, and then use
+	// it to calculate the instance addresses we'll expand for.
+	switch {
+	case n.Config.Count != nil:
+		count, countDiags := evaluateCountExpression(n.Config.Count, ctx)
+		diags = diags.Append(countDiags)
+		if countDiags.HasErrors() {
+			return nil, diags.Err()
+		}
 
-	forEachMap, forEachDiags := evaluateResourceForEachExpression(n.Config.ForEach, ctx)
-	if forEachDiags.HasErrors() {
-		return nil, diags.Err()
+		expander.SetResourceCount(n.Addr.Module, n.Addr.Resource, count)
+
+	case n.Config.ForEach != nil:
+		forEachMap, forEachDiags := evaluateForEachExpression(n.Config.ForEach, ctx)
+		if forEachDiags.HasErrors() {
+			return nil, diags.Err()
+		}
+
+		expander.SetResourceForEach(n.Addr.Module, n.Addr.Resource, forEachMap)
+
+	default:
+		expander.SetResourceSingle(n.Addr.Module, n.Addr.Resource)
 	}
 
 	// Next we need to potentially rename an instance address in the state
 	// if we're transitioning whether "count" is set at all.
-	fixResourceCountSetTransition(ctx, n.Addr.Config(), count != -1)
-
-	// Inform our instance expander about our expansion results above,
-	// and then use it to calculate the instance addresses we'll expand for.
-	expander := ctx.InstanceExpander()
-	switch {
-	case count >= 0:
-		expander.SetResourceCount(n.Addr.Module, n.Addr.Resource, count)
-	case forEachMap != nil:
-		expander.SetResourceForEach(n.Addr.Module, n.Addr.Resource, forEachMap)
-	default:
-		expander.SetResourceSingle(n.Addr.Module, n.Addr.Resource)
-	}
+	fixResourceCountSetTransition(ctx, n.Addr.Config(), n.Config.Count != nil)
 	instanceAddrs := expander.ExpandResource(n.Addr)
 
 	// Our graph transformers require access to the full state, so we'll

From 73a20bfb1776a8fb358d2c981d4beface18d60e3 Mon Sep 17 00:00:00 2001
From: James Bardin <j.bardin@gmail.com>
Date: Thu, 9 Apr 2020 10:13:03 -0400
Subject: [PATCH 4/5] fixup mangled comments

---
 terraform/eval_count.go    | 17 ++++++-----------
 terraform/eval_for_each.go | 11 +++++++----
 2 files changed, 13 insertions(+), 15 deletions(-)

diff --git a/terraform/eval_count.go b/terraform/eval_count.go
index 99ddeafee6..b09c72e661 100644
--- a/terraform/eval_count.go
+++ b/terraform/eval_count.go
@@ -12,18 +12,13 @@ import (
 )
 
 // evaluateCountExpression is our standard mechanism for interpreting an
-// expression given for a "count" argument on a resource. This should be called
-// from the DynamicExpand of a node representing a resource in order to
-// determine the final count value.
+// expression given for a "count" argument on a resource or a module. This
+// should be called during expansion in order to determine the final count
+// value.
 //
-// If the result is zero or positive and no error diagnostics are returned, then
-// the result is the literal count value to use.
-//
-// If the result is -1, this indicates that the given expression is nil and so
-// the "count" behavior should not be enabled for this resource at all.
-//
-// If error diagnostics are returned then the result is always the meaningless
-// placeholder value -1.
+// evaluateCountExpression differs from evaluateCountExpressionValue by
+// returning an error if the count value is not known, and converting the
+// cty.Value to an integer.
 func evaluateCountExpression(expr hcl.Expression, ctx EvalContext) (int, tfdiags.Diagnostics) {
 	countVal, diags := evaluateCountExpressionValue(expr, ctx)
 	if !countVal.IsKnown() {
diff --git a/terraform/eval_for_each.go b/terraform/eval_for_each.go
index 978e034d91..02ec4815b7 100644
--- a/terraform/eval_for_each.go
+++ b/terraform/eval_for_each.go
@@ -8,11 +8,14 @@ import (
 	"github.com/zclconf/go-cty/cty"
 )
 
-// evaluateForEachExpression interprets a "for_each" argument on a resource.
+// evaluateForEachExpression is our standard mechanism for interpreting an
+// expression given for a "for_each" argument on a resource or a module. This
+// should be called during expansion in order to determine the final keys and
+// values.
 //
-// Returns a cty.Value map, and diagnostics if necessary. It will return nil if
-// the expression is nil, and is used to distinguish between an unset for_each and an
-// empty map
+// evaluateForEachExpression differs from evaluateForEachExpressionValue by
+// returning an error if the count value is not known, and converting the
+// cty.Value to a map[string]cty.Value for compatibility with other calls.
 func evaluateForEachExpression(expr hcl.Expression, ctx EvalContext) (forEach map[string]cty.Value, diags tfdiags.Diagnostics) {
 	forEachVal, diags := evaluateForEachExpressionValue(expr, ctx)
 	if !forEachVal.IsKnown() {

From 3d8b1dea9736cca4a7ffbcd2347800f9b78f112f Mon Sep 17 00:00:00 2001
From: James Bardin <j.bardin@gmail.com>
Date: Thu, 9 Apr 2020 11:47:16 -0400
Subject: [PATCH 5/5] Update terraform/eval_for_each.go

Co-Authored-By: Pam Selle <pam@hashicorp.com>
---
 terraform/eval_for_each.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/terraform/eval_for_each.go b/terraform/eval_for_each.go
index 02ec4815b7..c23e9a3fbf 100644
--- a/terraform/eval_for_each.go
+++ b/terraform/eval_for_each.go
@@ -77,7 +77,7 @@ func evaluateForEachExpressionValue(expr hcl.Expression, ctx EvalContext) (cty.V
 
 	case forEachVal.LengthInt() == 0:
 		// If the map is empty ({}), return an empty map, because cty will
-		// return nil when representing {} AsValueMap This also covers an empty
+		// return nil when representing {} AsValueMap. This also covers an empty
 		// set (toset([]))
 		return forEachVal, diags
 	}