Refactor reference creation

Author: SupunS

interpreter/errors.go

@@ -1067,6 +1067,23 @@ func (e NestedReferenceError) Error() string {
 	)
 }
 
+// NonOptionalReferenceToNilError
+type NonOptionalReferenceToNilError struct {
+	ReferenceType sema.Type
+	LocationRange
+}
+
+var _ errors.UserError = NonOptionalReferenceToNilError{}
+
+func (NonOptionalReferenceToNilError) IsUserError() {}
+
+func (e NonOptionalReferenceToNilError) Error() string {
+	return fmt.Sprintf(
+		"cannot create a reference to nil: expected `%s`, but found `nil`",
+		e.ReferenceType.ID(),
+	)
+}
+
 // InclusiveRangeConstructionError
 
 type InclusiveRangeConstructionError struct {

interpreter/interpreter_expression.go

@@ -309,42 +309,7 @@ func (interpreter *Interpreter) memberExpressionGetterSetter(
 // e.g.1: Given type T, this method returns &T.
 // e.g.2: Given T?, this returns (&T)?
 func (interpreter *Interpreter) getReferenceValue(value Value, resultType sema.Type, locationRange LocationRange) Value {
-
-	// `resultType` is always an [optional] reference.
-	// This is guaranteed by the checker.
-	referenceType, ok := sema.UnwrapOptionalType(resultType).(*sema.ReferenceType)
-	if !ok {
-		panic(errors.NewUnreachableError())
-	}
-
-	switch value := value.(type) {
-	case NilValue, ReferenceValue:
-		// Reference to a nil, should return a nil.
-		// If the value is already a reference then return the same reference.
-		// However, we need to make sure that this reference is actually a subtype of the resultType,
-		// since the checker may not be aware that we are "short-circuiting" in this case
-		// Additionally, it is only safe to "compress" reference types like this when the desired
-		// result reference type is unauthorized
-
-		staticType := value.StaticType(interpreter)
-		if referenceType.Authorization != sema.UnauthorizedAccess || !interpreter.IsSubTypeOfSemaType(staticType, resultType) {
-			panic(InvalidMemberReferenceError{
-				ExpectedType:  resultType,
-				ActualType:    interpreter.MustConvertStaticToSemaType(staticType),
-				LocationRange: locationRange,
-			})
-		}
-
-		return value
-
-	case *SomeValue:
-		innerValue := interpreter.getReferenceValue(value.value, resultType, locationRange)
-		return NewSomeValueNonCopying(interpreter, innerValue)
-	}
-
-	auth := ConvertSemaAccessToStaticAuthorization(interpreter, referenceType.Authorization)
-
-	return NewEphemeralReferenceValue(interpreter, auth, value, referenceType.Type, locationRange)
+	return interpreter.createReference(resultType, value, locationRange, true)
 }
 
 func (interpreter *Interpreter) checkMemberAccess(
@@ -1466,20 +1431,30 @@ func (interpreter *Interpreter) VisitReferenceExpression(referenceExpression *as
 
 	result := interpreter.evalExpression(referenceExpression.Expression)
 
-	return interpreter.createReference(borrowType, result, referenceExpression)
+	locationRange := LocationRange{
+		Location:    interpreter.Location,
+		HasPosition: referenceExpression,
+	}
+
+	return interpreter.createReference(borrowType, result, locationRange, false)
 }
 
 func (interpreter *Interpreter) createReference(
 	borrowType sema.Type,
 	value Value,
-	hasPosition ast.HasPosition,
+	locationRange LocationRange,
+	isImplicit bool,
 ) Value {
 
 	// There are four potential cases:
-	// 1) Target type is optional, actual value is also optional (nil/SomeValue)
-	// 2) Target type is optional, actual value is non-optional
-	// 3) Target type is non-optional, actual value is optional (SomeValue)
-	// 4) Target type is non-optional, actual value is non-optional
+	// (1) Target type is optional, actual value is also optional
+	//     (1.a) value is SomeValue
+	//     (1.b) value is nil
+	// (2) Target type is optional, actual value is non-optional
+	// (3) Target type is non-optional, actual value is optional
+	//     (3.a) value is SomeValue
+	//     (3.b) value is nil
+	// (4) Target type is non-optional, actual value is non-optional
 
 	switch typ := borrowType.(type) {
 	case *sema.OptionalType:
@@ -1489,48 +1464,75 @@ func (interpreter *Interpreter) createReference(
 		switch value := value.(type) {
 		case *SomeValue:
 			// Case (1):
-			// References to optionals are transformed into optional references,
-			// so move the *SomeValue out to the reference itself
+			// References to optionals are transformed into optional references.
 
-			locationRange := LocationRange{
-				Location:    interpreter.Location,
-				HasPosition: hasPosition,
-			}
+			// (1.a) value is SomeValue
+			// Move the *SomeValue out to the reference itself.
 
 			innerValue := value.InnerValue(interpreter, locationRange)
 
-			referenceValue := interpreter.createReference(innerType, innerValue, hasPosition)
+			referenceValue := interpreter.createReference(innerType, innerValue, locationRange, false)
 
 			// Wrap the reference with an optional (since an optional is expected).
 			return NewSomeValueNonCopying(interpreter, referenceValue)
 
 		case NilValue:
+			// Case (1.b) value is nil.
+			// Since the resulting type can accommodate a nil (optional-reference), return nil,
 			return Nil
 
 		default:
 			// Case (2):
 			// If the referenced value is non-optional,
 			// but the target type is optional.
-			referenceValue := interpreter.createReference(innerType, value, hasPosition)
+			referenceValue := interpreter.createReference(innerType, value, locationRange, false)
 
 			// Wrap the reference with an optional (since an optional is expected).
 			return NewSomeValueNonCopying(interpreter, referenceValue)
 		}
 
 	case *sema.ReferenceType:
-		// Case (3): target type is non-optional, actual value is optional.
-		if someValue, ok := value.(*SomeValue); ok {
-			locationRange := LocationRange{
-				Location:    interpreter.Location,
-				HasPosition: hasPosition,
+
+		switch value := value.(type) {
+		case *SomeValue:
+			// Case (3.a): target type is non-optional, actual value is optional.
+			innerValue := value.InnerValue(interpreter, locationRange)
+
+			return interpreter.createReference(typ, innerValue, locationRange, false)
+
+		case NilValue:
+			// Case (3.b) value is nil.
+			// Since the resulting type can NOT accommodate a nil (non-optional reference), error-out.
+			panic(NonOptionalReferenceToNilError{
+				ReferenceType: typ,
+				LocationRange: locationRange,
+			})
+
+		case ReferenceValue:
+			if isImplicit {
+				// During implicit reference creation (e.g: member/index access on a reference),
+				// if the value is already a reference then return the same reference.
+				// However, we need to make sure that this reference is actually a subtype of the resultType,
+				// since the checker may not be aware that we are "short-circuiting" in this case.
+				// Additionally, it is only safe to "compress" reference types like this when the desired
+				// result reference type is unauthorized
+				staticType := value.StaticType(interpreter)
+				if typ.Authorization != sema.UnauthorizedAccess || !interpreter.IsSubTypeOfSemaType(staticType, typ) {
+					panic(InvalidMemberReferenceError{
+						ExpectedType:  typ,
+						ActualType:    interpreter.MustConvertStaticToSemaType(staticType),
+						LocationRange: locationRange,
+					})
+				}
+
+				return value
 			}
-			innerValue := someValue.InnerValue(interpreter, locationRange)
 
-			return interpreter.createReference(typ, innerValue, hasPosition)
+			// break
 		}
 
 		// Case (4): target type is non-optional, actual value is also non-optional.
-		return interpreter.newEphemeralReference(value, typ, hasPosition)
+		return interpreter.newEphemeralReference(value, typ, locationRange)
 
 	default:
 		panic(errors.NewUnreachableError())
@@ -1540,17 +1542,12 @@ func (interpreter *Interpreter) createReference(
 func (interpreter *Interpreter) newEphemeralReference(
 	value Value,
 	typ *sema.ReferenceType,
-	hasPosition ast.HasPosition,
+	locationRange LocationRange,
 ) *EphemeralReferenceValue {
 	// If we are currently interpreting a function that was declared with mapped entitlement access, any appearances
 	// of that mapped access in the body of the function should be replaced with the computed output of the map
 	auth := ConvertSemaAccessToStaticAuthorization(interpreter, typ.Authorization)
 
-	locationRange := LocationRange{
-		Location:    interpreter.Location,
-		HasPosition: hasPosition,
-	}
-
 	return NewEphemeralReferenceValue(
 		interpreter,
 		auth,

interpreter/member_test.go

@@ -696,16 +696,9 @@ func TestInterpretMemberAccess(t *testing.T) {
             }
         `)
 
-		// Currently a runtime error
-		value, err := inter.Invoke("test")
-		require.NoError(t, err)
-
-		AssertValuesEqual(
-			t,
-			inter,
-			interpreter.Nil,
-			value,
-		)
+		_, err := inter.Invoke("test")
+		RequireError(t, err)
+		require.ErrorAs(t, err, &interpreter.NonOptionalReferenceToNilError{})
 	})
 
 	t.Run("composite reference, primitive field", func(t *testing.T) {

interpreter/reference_test.go

@@ -1675,14 +1675,14 @@ func TestInterpretInvalidReferenceToOptionalConfusion(t *testing.T) {
 	_, err := inter.Invoke("main")
 	RequireError(t, err)
 
-	require.ErrorAs(t, err, &interpreter.ForceCastTypeMismatchError{})
+	require.ErrorAs(t, err, &interpreter.NonOptionalReferenceToNilError{})
 }
 
 func TestInterpretReferenceToOptional(t *testing.T) {
 
 	t.Parallel()
 
-	t.Run("nil in anystruct", func(t *testing.T) {
+	t.Run("nil in AnyStruct", func(t *testing.T) {
 		t.Parallel()
 
 		inter := parseCheckAndInterpret(t, `
@@ -1692,19 +1692,9 @@ func TestInterpretReferenceToOptional(t *testing.T) {
             }
         `)
 
-		value, err := inter.Invoke("main")
-		require.NoError(t, err)
-
-		AssertValuesEqual(
-			t,
-			inter,
-			&interpreter.EphemeralReferenceValue{
-				Value:         interpreter.Nil,
-				BorrowedType:  sema.AnyStructType,
-				Authorization: interpreter.UnauthorizedAccess,
-			},
-			value,
-		)
+		_, err := inter.Invoke("main")
+		RequireError(t, err)
+		require.ErrorAs(t, err, &interpreter.NonOptionalReferenceToNilError{})
 	})
 
 	t.Run("nil in optional", func(t *testing.T) {

sema/check_member_expression.go

@@ -25,7 +25,7 @@ import (
 
 // NOTE: only called if the member expression is *not* an assignment
 func (checker *Checker) VisitMemberExpression(expression *ast.MemberExpression) Type {
-	accessedType, memberType, member, isOptional := checker.visitMember(expression, false)
+	accessedType, memberType, member, _ := checker.visitMember(expression, false)
 
 	if !accessedType.IsInvalidType() {
 		memberAccessType := accessedType
@@ -83,14 +83,6 @@ func (checker *Checker) VisitMemberExpression(expression *ast.MemberExpression)
 
 	checker.checkResourceMemberCapturingInFunction(expression, member, memberType)
 
-	// If the member access is optional chaining, only wrap the result value
-	// in an optional, if it is not already an optional value
-	if isOptional {
-		if _, ok := memberType.(*OptionalType); !ok {
-			memberType = NewOptionalType(checker.memoryGauge, memberType)
-		}
-	}
-
 	return memberType
 }
 
@@ -203,7 +195,7 @@ func (checker *Checker) visitMember(expression *ast.MemberExpression, isAssignme
 	// i.e. a Go type switch would be sufficient.
 	// However, for some types (e.g. reference types) this depends on what type is referenced
 
-	getMemberForType := func(expressionType Type) {
+	findAndSetResultingType := func(expressionType Type, optional bool) {
 		resolver, ok := expressionType.GetMembers()[identifier]
 		if !ok {
 			return
@@ -216,6 +208,16 @@ func (checker *Checker) visitMember(expression *ast.MemberExpression, isAssignme
 			checker.report,
 		)
 		resultingType = member.TypeAnnotation.Type
+
+		// If the member is accessed using optional-chaining, then the resulting type also should be optional.
+		// However, if the member is already optional, then no need to double-wrap from optionals.
+		if optional {
+			if _, memberIsOptional := resultingType.(*OptionalType); !memberIsOptional {
+				resultingType = NewOptionalType(checker.memoryGauge, resultingType)
+			}
+		}
+
+		isOptional = optional
 	}
 
 	// Get the member from the accessed value based
@@ -229,8 +231,7 @@ func (checker *Checker) visitMember(expression *ast.MemberExpression, isAssignme
 		if optionalExpressionType, ok := accessedType.(*OptionalType); ok {
 			// The accessed type is optional, get the member from the wrapped type
 
-			getMemberForType(optionalExpressionType.Type)
-			isOptional = true
+			findAndSetResultingType(optionalExpressionType.Type, true)
 		} else {
 			// Optional chaining was used on a non-optional type, report an error
 
@@ -247,14 +248,12 @@ func (checker *Checker) visitMember(expression *ast.MemberExpression, isAssignme
 			// to avoid spurious error that member does not exist,
 			// even if the non-optional accessed type has the member
 
-			getMemberForType(accessedType)
+			findAndSetResultingType(accessedType, false)
 		}
 	} else {
 		// The member is accessed directly without optional chaining.
 		// Get the member directly from the accessed type
-
-		getMemberForType(accessedType)
-		isOptional = false
+		findAndSetResultingType(accessedType, false)
 	}
 
 	if member == nil {

sema/check_reference_expression.go

@@ -116,8 +116,8 @@ func (checker *Checker) expectedTypeForReferencedExpr(
 			checker.expectedTypeForReferencedExpr(expectedType.Type, hasPosition)
 
 		// Re-wrap with an optional
-		expectedLeftType = &OptionalType{Type: expectedLeftType}
-		returnType = &OptionalType{Type: returnType}
+		expectedLeftType = NewOptionalType(checker.memoryGauge, expectedLeftType)
+		returnType = NewOptionalType(checker.memoryGauge, returnType)
 
 	case *ReferenceType:
 		referencedType := expectedType.Type