Allow 'with' expression on value types

docs/contributing/Compiler Test Plan.md

@@ -34,7 +34,7 @@ This document provides guidance for thinking about language interactions and tes
 
 # Type and members
 - Access modifiers (public, protected, internal, protected internal, private protected, private), static, ref
-- type declarations (class, record with or without positional members, struct, interface, type parameter)
+- type declarations (class, record class/struct with or without positional members, struct, interface, type parameter)
 - methods
 - fields
 - properties (including get/set/init accessors)
@@ -91,8 +91,9 @@ This document provides guidance for thinking about language interactions and tes
 - Ref return, ref readonly return, ref ternary, ref readonly local, ref local re-assignment, ref foreach
 - `this = e;` in `struct` .ctor
 - Stackalloc (including initializers)
-- Patterns (constant, declaration, `var`, positional, property, and discard forms)
+- Patterns (constant, declaration, `var`, positional, property, discard, parenthesized, type, relational, `and`/`or`/`not`)
 - Switch expressions
+- With expressions (on record classes and on value types)
 - Nullability annotations (`?`, attributes) and analysis
 - If you add a place an expression can appear in code, make sure `SpillSequenceSpiller` handles it. Test with a `switch` expression or `stackalloc` in that place.
 - If you add a new expression form that requires spilling, test it in the catch filter.

src/Compilers/CSharp/Portable/Binder/Binder.ValueChecks.cs

@@ -3006,37 +3006,39 @@ internal static bool CheckValEscape(SyntaxNode node, BoundExpression expr, uint
                         isRefEscape: false);
 
                 case BoundKind.ObjectCreationExpression:
-                    var objectCreation = (BoundObjectCreationExpression)expr;
-                    var constructorSymbol = objectCreation.Constructor;
-
-                    var escape = CheckInvocationEscape(
-                        objectCreation.Syntax,
-                        constructorSymbol,
-                        null,
-                        constructorSymbol.Parameters,
-                        objectCreation.Arguments,
-                        objectCreation.ArgumentRefKindsOpt,
-                        objectCreation.ArgsToParamsOpt,
-                        checkingReceiver,
-                        escapeFrom,
-                        escapeTo,
-                        diagnostics,
-                        isRefEscape: false);
-
-                    var initializerExpr = objectCreation.InitializerExpressionOpt;
-                    if (initializerExpr != null)
                     {
-                        escape = escape &&
-                            CheckValEscape(
-                                initializerExpr.Syntax,
-                                initializerExpr,
-                                escapeFrom,
-                                escapeTo,
-                                checkingReceiver: false,
-                                diagnostics: diagnostics);
-                    }
+                        var objectCreation = (BoundObjectCreationExpression)expr;
+                        var constructorSymbol = objectCreation.Constructor;
+
+                        var escape = CheckInvocationEscape(
+                            objectCreation.Syntax,
+                            constructorSymbol,
+                            null,
+                            constructorSymbol.Parameters,
+                            objectCreation.Arguments,
+                            objectCreation.ArgumentRefKindsOpt,
+                            objectCreation.ArgsToParamsOpt,
+                            checkingReceiver,
+                            escapeFrom,
+                            escapeTo,
+                            diagnostics,
+                            isRefEscape: false);
+
+                        var initializerExpr = objectCreation.InitializerExpressionOpt;
+                        if (initializerExpr != null)
+                        {
+                            escape = escape &&
+                                CheckValEscape(
+                                    initializerExpr.Syntax,
+                                    initializerExpr,
+                                    escapeFrom,
+                                    escapeTo,
+                                    checkingReceiver: false,
+                                    diagnostics: diagnostics);
+                        }
 
-                    return escape;
+                        return escape;
+                    }
 
                 case BoundKind.UnaryOperator:
                     var unary = (BoundUnaryOperator)expr;
@@ -3123,6 +3125,20 @@ internal static bool CheckValEscape(SyntaxNode node, BoundExpression expr, uint
 
                     return true;
 
+                case BoundKind.WithExpression:
+                    {
+                        var withExpr = (BoundWithExpression)expr;
+                        var escape = CheckValEscape(node, withExpr.Receiver, escapeFrom, escapeTo, checkingReceiver: false, diagnostics);
+
+                        var initializerExpr = withExpr.InitializerExpression;
+                        if (initializerExpr != null)
+                        {
+                            escape = escape && CheckValEscape(initializerExpr.Syntax, initializerExpr, escapeFrom, escapeTo, checkingReceiver: false, diagnostics: diagnostics);
+                        }
+
+                        return escape;
+                    }
+
                 default:
                     // in error situations some unexpected nodes could make here
                     // returning "false" seems safer than throwing.

src/Compilers/CSharp/Portable/Binder/Binder_WithExpression.cs

@@ -32,15 +32,19 @@ private BoundExpression BindWithExpression(WithExpressionSyntax syntax, BindingD
             }
 
             MethodSymbol? cloneMethod = null;
-            if (!receiverType.IsErrorType())
+            if (receiverType.IsValueType)
+            {
+                CheckFeatureAvailability(syntax, MessageID.IDS_FeatureWithOnStructs, diagnostics);
+            }
+            else if (!receiverType.IsErrorType())
             {
                 CompoundUseSiteInfo<AssemblySymbol> useSiteInfo = GetNewCompoundUseSiteInfo(diagnostics);
 
                 cloneMethod = SynthesizedRecordClone.FindValidCloneMethod(receiverType is TypeParameterSymbol typeParameter ? typeParameter.EffectiveBaseClass(ref useSiteInfo) : receiverType, ref useSiteInfo);
                 if (cloneMethod is null)
                 {
                     hasErrors = true;
-                    diagnostics.Add(ErrorCode.ERR_NoSingleCloneMethod, syntax.Expression.Location, receiverType);
+                    diagnostics.Add(ErrorCode.ERR_CannotClone, syntax.Expression.Location, receiverType);
                 }
                 else
                 {

src/Compilers/CSharp/Portable/CSharpResources.resx

@@ -6274,8 +6274,8 @@ To remove the warning, you can use /reference instead (set the Embed Interop Typ
   <data name="ERR_InvalidWithReceiverType" xml:space="preserve">
     <value>The receiver of a `with` expression must have a non-void type.</value>
   </data>
-  <data name="ERR_NoSingleCloneMethod" xml:space="preserve">
-    <value>The receiver type '{0}' is not a valid record type.</value>
+  <data name="ERR_CannotClone" xml:space="preserve">
+    <value>The receiver type '{0}' is not a valid record type and is not a struct type.</value>
   </data>
   <data name="ERR_AssignmentInitOnly" xml:space="preserve">
     <value>Init-only property or indexer '{0}' can only be assigned in an object initializer, or on 'this' or 'base' in an instance constructor or an 'init' accessor.</value>
@@ -6567,6 +6567,9 @@ To remove the warning, you can use /reference instead (set the Embed Interop Typ
     <value>record structs</value>
     <comment>'record structs' is not localizable.</comment>
   </data>
+  <data name="IDS_FeatureWithOnStructs" xml:space="preserve">
+    <value>with on structs</value>
+  </data>
   <data name="IDS_FeatureVarianceSafetyForStaticInterfaceMembers" xml:space="preserve">
     <value>variance safety for static interface members</value>
   </data>

src/Compilers/CSharp/Portable/Errors/ErrorCode.cs

@@ -1860,7 +1860,7 @@ internal enum ErrorCode
         ERR_ExplicitPropertyMismatchInitOnly = 8855,
         ERR_BadInitAccessor = 8856,
         ERR_InvalidWithReceiverType = 8857,
-        ERR_NoSingleCloneMethod = 8858,
+        ERR_CannotClone = 8858,
         ERR_CloneDisallowedInRecord = 8859,
         WRN_RecordNamedDisallowed = 8860,
         ERR_UnexpectedArgumentList = 8861,

src/Compilers/CSharp/Portable/Errors/MessageID.cs

@@ -217,6 +217,7 @@ internal enum MessageID
         IDS_FeatureConstantInterpolatedStrings = MessageBase + 12792,
         IDS_FeatureMixedDeclarationsAndExpressionsInDeconstruction = MessageBase + 12793,
         IDS_FeatureRecordStructs = MessageBase + 12794,
+        IDS_FeatureWithOnStructs = MessageBase + 12795,
     }
 
     // Message IDs may refer to strings that need to be localized.
@@ -325,7 +326,8 @@ internal static LanguageVersion RequiredVersion(this MessageID feature)
             {
                 // C# preview features.
                 case MessageID.IDS_FeatureRecordStructs:
-                case MessageID.IDS_FeatureMixedDeclarationsAndExpressionsInDeconstruction:
+                case MessageID.IDS_FeatureWithOnStructs: // semantic check
+                case MessageID.IDS_FeatureMixedDeclarationsAndExpressionsInDeconstruction: // semantic check
                     return LanguageVersion.Preview;
                 // C# 9.0 features.
                 case MessageID.IDS_FeatureLambdaDiscardParameters: // semantic check

src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_ObjectCreationExpression.cs

@@ -103,31 +103,48 @@ public override BoundNode VisitObjectCreationExpression(BoundObjectCreationExpre
 
         public override BoundNode VisitWithExpression(BoundWithExpression withExpr)
         {
-            RoslynDebug.AssertNotNull(withExpr.CloneMethod);
-            Debug.Assert(withExpr.CloneMethod.ParameterCount == 0);
             Debug.Assert(withExpr.Receiver.Type!.Equals(withExpr.Type, TypeCompareKind.ConsiderEverything));
 
             // for a with expression of the form
             //
             //      receiver with { P1 = e1, P2 = e2 }
             //
-            // we want to lower it to a call to the receiver's `Clone` method, then
+            // if the receiver is a struct, duplicate the value, then set the given struct properties:
+            //
+            //     var tmp = receiver;
+            //     tmp.P1 = e1;
+            //     tmp.P2 = e2;
+            //     tmp
+            //
+            // otherwise the receiver is a record class and we want to lower it to a call to its `Clone` method, then
             // set the given record properties. i.e.
             //
             //      var tmp = (ReceiverType)receiver.Clone();
             //      tmp.P1 = e1;
             //      tmp.P2 = e2;
             //      tmp
 
-            var cloneCall = _factory.Convert(
-                withExpr.Type,
-                _factory.Call(
-                    VisitExpression(withExpr.Receiver),
-                    withExpr.CloneMethod));
+            BoundExpression expression;
+
+            if (withExpr.Type.IsValueType)
+            {
+                expression = VisitExpression(withExpr.Receiver);
+            }
+            else
+            {
+                RoslynDebug.AssertNotNull(withExpr.CloneMethod);
+                Debug.Assert(withExpr.CloneMethod.ParameterCount == 0);
+
+                expression = _factory.Convert(
+                    withExpr.Type,
+                    _factory.Call(
+                        VisitExpression(withExpr.Receiver),
+                        withExpr.CloneMethod));
+            }
 
             return MakeExpressionWithInitializer(
                 withExpr.Syntax,
-                cloneCall,
+                expression,
                 withExpr.InitializerExpression,
                 withExpr.Type);
         }