Improve resource tracking for optional binding

runtime/sema/check_casting_expression.go

@@ -53,18 +53,13 @@ func (checker *Checker) VisitCastingExpression(expression *ast.CastingExpression
 	checker.Elaboration.CastingStaticValueTypes[expression] = leftHandType
 
 	if leftHandType.IsResourceType() {
-		checker.recordResourceInvalidation(
-			leftHandExpression,
-			leftHandType,
-			ResourceInvalidationKindMoveDefinite,
-		)
-
-		// If the failable casted type is a resource, the failable cast expression
-		// must occur in an optional binding, i.e. inside a variable declaration
-		// as the if-statement test element
 
 		if expression.Operation == ast.OperationFailableCast {
 
+			// If the failable casted type is a resource, the failable cast expression
+			// must occur in an optional binding, i.e. inside a variable declaration
+			// as the if-statement test element
+
 			if expression.ParentVariableDeclaration == nil ||
 				expression.ParentVariableDeclaration.ParentIfStatement == nil {
 
@@ -82,6 +77,23 @@ func (checker *Checker) VisitCastingExpression(expression *ast.CastingExpression
 					},
 				)
 			}
+
+			// NOTE: Counter-intuitively, do not *always* invalidate the casted expression:
+			// As the failable cast must occur in an if-statement, the statement itself
+			// takes care of the invalidation:
+			// - In the then-branch, the cast succeeded, so the casted variable becomes invalidated
+			// - Whereas in the else-branch, the cast failed, and the casted variable is still available
+
+		} else {
+
+			// For non-failable casts of a resource,
+			// always record an invalidation
+
+			checker.recordResourceInvalidation(
+				leftHandExpression,
+				leftHandType,
+				ResourceInvalidationKindMoveDefinite,
+			)
 		}
 	}
 
@@ -140,10 +152,6 @@ func (checker *Checker) VisitCastingExpression(expression *ast.CastingExpression
 		return rightHandType
 
 	case ast.OperationCast:
-		// If there are errors in the lhs-expr, then the target type is considered as
-		// the inferred-type of the expression. i.e: exprActualType == rightHandType
-		// Then, it is not possible to determine whether the target type is redundant.
-		// Therefore, don't check for redundant casts, if there are errors.
 		if checker.Config.ExtendedElaborationEnabled && !hasErrors {
 			checker.Elaboration.StaticCastTypes[expression] =
 				CastTypes{
@@ -163,7 +171,6 @@ func (checker *Checker) VisitCastingExpression(expression *ast.CastingExpression
 // FailableCastCanSucceed checks a failable (dynamic) cast, i.e. a cast that might succeed at run-time.
 // It returns true if the cast from subType to superType could potentially succeed at run-time,
 // and returns false if the cast will definitely always fail.
-//
 func FailableCastCanSucceed(subType, superType Type) bool {
 
 	// TODO: report impossible casts, e.g.

runtime/sema/check_conditional.go

@@ -45,12 +45,25 @@ func (checker *Checker) VisitIfStatement(statement *ast.IfStatement) (_ struct{}
 		)
 
 	case *ast.VariableDeclaration:
+		declarationType := checker.visitVariableDeclarationValues(test, true)
+
 		checker.checkConditionalBranches(
 			func() Type {
 				checker.enterValueScope()
 				defer checker.leaveValueScope(thenElement.EndPosition, true)
 
-				checker.visitVariableDeclaration(test, true)
+				if castingExpression, ok := test.Value.(*ast.CastingExpression); ok &&
+					castingExpression.Operation == ast.OperationFailableCast {
+					leftHandType := checker.Elaboration.CastingStaticValueTypes[castingExpression]
+					if leftHandType.IsResourceType() {
+						checker.recordResourceInvalidation(
+							castingExpression.Expression,
+							leftHandType,
+							ResourceInvalidationKindMoveDefinite,
+						)
+					}
+				}
+				checker.declareVariableDeclaration(test, declarationType)
 
 				checker.checkBlock(thenElement)
 				return nil

runtime/sema/check_variable_declaration.go

@@ -24,11 +24,13 @@ import (
 )
 
 func (checker *Checker) VisitVariableDeclaration(declaration *ast.VariableDeclaration) (_ struct{}) {
-	checker.visitVariableDeclaration(declaration, false)
+	declarationType := checker.visitVariableDeclarationValues(declaration, false)
+	checker.declareVariableDeclaration(declaration, declarationType)
+
 	return
 }
 
-func (checker *Checker) visitVariableDeclaration(declaration *ast.VariableDeclaration, isOptionalBinding bool) {
+func (checker *Checker) visitVariableDeclarationValues(declaration *ast.VariableDeclaration, isOptionalBinding bool) Type {
 
 	checker.checkDeclarationAccessModifier(
 		declaration.Access,
@@ -177,6 +179,10 @@ func (checker *Checker) visitVariableDeclaration(declaration *ast.VariableDeclar
 			SecondValueType: secondValueType,
 		}
 
+	return declarationType
+}
+
+func (checker *Checker) declareVariableDeclaration(declaration *ast.VariableDeclaration, declarationType Type) {
 	// Finally, declare the variable in the current value activation
 
 	identifier := declaration.Identifier.Identifier

runtime/tests/checker/resources_test.go

@@ -4056,8 +4056,6 @@ func TestCheckResourceOptionalBinding(t *testing.T) {
           let maybeR: @R? <- create R()
           if let r <- maybeR {
               destroy r
-          } else {
-              destroy maybeR
           }
       }
     `)
@@ -4076,8 +4074,6 @@ func TestCheckInvalidResourceOptionalBindingResourceLossInThen(t *testing.T) {
           let maybeR: @R? <- create R()
           if let r <- maybeR {
               // resource loss of r
-          } else {
-              destroy maybeR
           }
       }
     `)
@@ -4087,7 +4083,7 @@ func TestCheckInvalidResourceOptionalBindingResourceLossInThen(t *testing.T) {
 	assert.IsType(t, &sema.ResourceLossError{}, errs[0])
 }
 
-func TestCheckInvalidResourceOptionalBindingResourceLossInElse(t *testing.T) {
+func TestCheckInvalidResourceOptionalBindingResourceUseAfterInvalidationInThen(t *testing.T) {
 
 	t.Parallel()
 
@@ -4098,18 +4094,17 @@ func TestCheckInvalidResourceOptionalBindingResourceLossInElse(t *testing.T) {
           let maybeR: @R? <- create R()
           if let r <- maybeR {
               destroy r
-          } else {
-              // resource loss of maybeR
+              destroy maybeR
           }
       }
     `)
 
 	errs := ExpectCheckerErrors(t, err, 1)
 
-	assert.IsType(t, &sema.ResourceLossError{}, errs[0])
+	assert.IsType(t, &sema.ResourceUseAfterInvalidationError{}, errs[0])
 }
 
-func TestCheckInvalidResourceOptionalBindingResourceUseAfterInvalidationInThen(t *testing.T) {
+func TestCheckInvalidResourceOptionalBindingResourceUseAfterInvalidationAfterBranches(t *testing.T) {
 
 	t.Parallel()
 
@@ -4120,10 +4115,12 @@ func TestCheckInvalidResourceOptionalBindingResourceUseAfterInvalidationInThen(t
           let maybeR: @R? <- create R()
           if let r <- maybeR {
               destroy r
-              destroy maybeR
-          } else {
-              destroy maybeR
           }
+          f(<-maybeR)
+      }
+
+      fun f(_ r: @R?) {
+          destroy r
       }
     `)
 
@@ -4132,31 +4129,171 @@ func TestCheckInvalidResourceOptionalBindingResourceUseAfterInvalidationInThen(t
 	assert.IsType(t, &sema.ResourceUseAfterInvalidationError{}, errs[0])
 }
 
-func TestCheckInvalidResourceOptionalBindingResourceUseAfterInvalidationAfterBranches(t *testing.T) {
+func TestCheckResourceOptionalBindingWithSecondValue(t *testing.T) {
 
 	t.Parallel()
 
 	_, err := ParseAndCheck(t, `
       resource R {}
 
       fun test() {
-          let maybeR: @R? <- create R()
-          if let r <- maybeR {
-              destroy r
+          let r1 <- create R()
+          var r2: @R? <- create R()
+
+          if let r3 <- r2 <- r1 {
+              // r1 was definitely moved
+              // r2 contains r1
+              destroy r2
+              // only then branch defined r3
+              destroy r3
           } else {
-              destroy maybeR
+              // r1 was definitely moved
+              // r2 contains r1
+              destroy r2
           }
-          f(<-maybeR)
-      }
-
-      fun f(_ r: @R?) {
-          destroy r
       }
     `)
+	require.NoError(t, err)
+}
 
-	errs := ExpectCheckerErrors(t, err, 1)
+func TestCheckResourceOptionalBindingResourceInvalidation(t *testing.T) {
 
-	assert.IsType(t, &sema.ResourceUseAfterInvalidationError{}, errs[0])
+	t.Parallel()
+
+	t.Run("separate, without else", func(t *testing.T) {
+
+		t.Parallel()
+
+		_, err := ParseAndCheck(t, `
+          resource R {}
+
+          fun asOpt(_ r: @R): @R? {
+              return <-r
+          }
+
+          fun test() {
+              let r <- create R()
+              let optR <- asOpt(<-r)
+              if let r2 <- optR {
+                  destroy r2
+              }
+          }
+        `)
+		require.NoError(t, err)
+	})
+
+	t.Run("separate, with else", func(t *testing.T) {
+
+		t.Parallel()
+
+		_, err := ParseAndCheck(t, `
+          resource R {}
+
+          fun asOpt(_ r: @R): @R? {
+              return <-r
+          }
+
+          fun consume(_ r: @R?) {
+              destroy <-r
+          }
+
+          fun test() {
+              let r <- create R()
+              let optR <- asOpt(<-r)
+              if let r2 <- optR {
+                  destroy r2
+              } else {
+                  consume(<-optR)
+              }
+          }
+        `)
+
+		errs := ExpectCheckerErrors(t, err, 1)
+
+		assert.IsType(t, &sema.ResourceUseAfterInvalidationError{}, errs[0])
+	})
+
+	t.Run("inline, without else", func(t *testing.T) {
+
+		t.Parallel()
+
+		_, err := ParseAndCheck(t, `
+          resource R {}
+
+          fun asOpt(_ r: @R): @R? {
+              return <-r
+          }
+
+          fun test() {
+              let r <- create R()
+              if let r2 <- asOpt(<-r) {
+                  destroy r2
+              }
+          }
+        `)
+
+		require.NoError(t, err)
+	})
+
+	t.Run("inline, with else, non-optional", func(t *testing.T) {
+
+		t.Parallel()
+
+		_, err := ParseAndCheck(t, `
+          resource R {}
+
+          fun asOpt(_ r: @R): @R? {
+              return <-r
+          }
+
+          fun consume(_ r: @R?) {
+              destroy <-r
+          }
+
+          fun test() {
+              let r <- create R()
+              if let r2 <- asOpt(<-r) {
+                  destroy r2
+              } else {
+                  consume(<-r)
+              }
+          }
+        `)
+
+		errs := ExpectCheckerErrors(t, err, 1)
+
+		assert.IsType(t, &sema.ResourceUseAfterInvalidationError{}, errs[0])
+	})
+
+	t.Run("inline, with else, optional", func(t *testing.T) {
+
+		t.Parallel()
+
+		_, err := ParseAndCheck(t, `
+          resource R {}
+
+          fun identity(_ r: @R?): @R? {
+              return <-r
+          }
+
+          fun consume(_ r: @R?) {
+              destroy <-r
+          }
+
+          fun test() {
+              let r: @R? <- create R()
+              if let r2 <- identity(<-r) {
+                  destroy r2
+              } else {
+                  consume(<-r)
+              }
+          }
+        `)
+
+		errs := ExpectCheckerErrors(t, err, 1)
+
+		assert.IsType(t, &sema.ResourceUseAfterInvalidationError{}, errs[0])
+	})
 }
 
 func TestCheckResourceOptionalBindingFailableCast(t *testing.T) {
@@ -4340,9 +4477,10 @@ func TestCheckInvalidResourceFailableCastOutsideOptionalBinding(t *testing.T) {
       }
     `)
 
-	errs := ExpectCheckerErrors(t, err, 1)
+	errs := ExpectCheckerErrors(t, err, 2)
 
 	assert.IsType(t, &sema.InvalidFailableResourceDowncastOutsideOptionalBindingError{}, errs[0])
+	assert.IsType(t, &sema.ResourceLossError{}, errs[1])
 }
 
 func TestCheckInvalidResourceFailableCastNonIdentifier(t *testing.T) {