From 13e15e1af62e354a23be6df3f9483ccdf001dcde Mon Sep 17 00:00:00 2001 From: Andy Gocke Date: Mon, 27 Apr 2020 22:23:54 -0700 Subject: [PATCH 01/17] Use init-only for records and adjust binding Changes records to generate init-only properties instead of get-only, and modifies the `with` expression to treat arguments as assignments to the left-hand side, with the safety rules of object initializers (meaning init-only assignments are allowed). Also changes records to generate a Clone method and a copy constructor. The clone calls the copy constructor and the copy constructor does a memberwise copy of the instance fields of the other type. This change also tries to simplify and reuse more pieces of binding and lowering from other initializer-based forms. The with-expression is now bound as though it were an assignment to a field or member access on a call to the Clone method. --- .../Portable/Binder/Binder_WithExpression.cs | 97 ++--- .../CSharp/Portable/BoundTree/BoundNodes.xml | 2 +- .../Generated/BoundNodes.xml.Generated.cs | 19 +- .../LocalRewriter_WithExpression.cs | 38 +- .../CSharp/Portable/Symbols/LexicalSortKey.cs | 5 +- .../Source/SourceMemberContainerSymbol.cs | 27 +- .../Records/SynthesizedRecordClone.cs | 139 +++++++ .../Records/SynthesizedRecordCopyCtor.cs | 59 +++ .../SynthesizedRecordPropertySymbol.cs | 139 ++++++- .../Semantic/Semantics/InitOnlyMemberTests.cs | 9 +- .../Semantic/Semantics/LookupPositionTests.cs | 5 +- .../Test/Semantic/Semantics/RecordTests.cs | 343 +++++++++++++----- .../Test/Symbol/Symbols/Source/RecordTests.cs | 157 ++++++-- 13 files changed, 823 insertions(+), 216 deletions(-) create mode 100644 src/Compilers/CSharp/Portable/Symbols/Synthesized/Records/SynthesizedRecordClone.cs create mode 100644 src/Compilers/CSharp/Portable/Symbols/Synthesized/Records/SynthesizedRecordCopyCtor.cs diff --git a/src/Compilers/CSharp/Portable/Binder/Binder_WithExpression.cs b/src/Compilers/CSharp/Portable/Binder/Binder_WithExpression.cs index 430ce5356bb97..8c44cb8362a93 100644 --- a/src/Compilers/CSharp/Portable/Binder/Binder_WithExpression.cs +++ b/src/Compilers/CSharp/Portable/Binder/Binder_WithExpression.cs @@ -21,7 +21,6 @@ internal partial class Binder { private BoundExpression BindWithExpression(WithExpressionSyntax syntax, DiagnosticBag diagnostics) { - // PROTOTYPE: this entire method is likely to change var receiver = BindRValueWithoutTargetType(syntax.Receiver, diagnostics); var receiverType = receiver.Type; @@ -95,85 +94,51 @@ private BoundExpression BindWithExpression(WithExpressionSyntax syntax, Diagnost } var cloneReturnType = cloneMethod?.ReturnType; - - var args = ArrayBuilder<(Symbol?, BoundExpression)>.GetInstance(); + // PROTOTYPE: Handle dynamic + var implicitReceiver = new BoundObjectOrCollectionValuePlaceholder( + syntax.Receiver, + // formatting + #pragma warning disable IDE0055 + cloneReturnType ?? receiverType) { WasCompilerGenerated = true }; + #pragma warning restore IDE0055 + + var args = ArrayBuilder.GetInstance(); // Bind with expression arguments foreach (var expr in syntax.Initializer.Expressions) { - Symbol? member = null; - BoundExpression boundRight; + BoundExpression boundExpr; // We're expecting a simple assignment only, with an ID on the left if (!(expr is AssignmentExpressionSyntax assignment) || !(assignment.Left is IdentifierNameSyntax left)) { - boundRight = BindExpression(expr, diagnostics); + boundExpr = BindExpression(expr, diagnostics); hasErrors = true; diagnostics.Add(ErrorCode.ERR_BadWithExpressionArgument, expr.Location); } else { var propName = left.Identifier.Text; - if (!(cloneReturnType is null)) - { - var location = left.Location; - this.LookupMembersInType( - lookupResult, - cloneReturnType, - propName, - arity: 0, - basesBeingResolved: null, - options: LookupOptions.Default, - originalBinder: this, - diagnose: false, - useSiteDiagnostics: ref useSiteDiagnostics); - // PROTOTYPE: Should handle hiding/overriding and bind like regular accesses - if (lookupResult.IsSingleViable && - lookupResult.SingleSymbolOrDefault is var sym) - { - switch (sym.Kind) - { - case SymbolKind.Property: - member = sym; - // PROTOTYPE: this should check for init-only, but that isn't a separate feature yet - // It also will not work in metadata. - if (!(sym is SynthesizedRecordPropertySymbol)) - { - goto default; - } - break; - - default: - hasErrors = true; - diagnostics.Add( - ErrorCode.ERR_WithMemberIsNotRecordProperty, - location); - break; - } - } - - if (!hasErrors && member is null) - { - hasErrors = true; - Error( - diagnostics, - ErrorCode.ERR_NoSuchMemberOrExtension, - location, - cloneReturnType, - propName); - } - } - - boundRight = BindValue(assignment.Right, diagnostics, BindValueKind.RValue); - if (!(member is null)) - { - boundRight = GenerateConversionForAssignment( - member.GetTypeOrReturnType().Type, - boundRight, - diagnostics); - } - lookupResult.Clear(); + BoundExpression? boundMember = null; + boundMember = BindInstanceMemberAccess( + node: left, + right: left, + boundLeft: implicitReceiver, + rightName: propName, + rightArity: 0, + typeArgumentsSyntax: default(SeparatedSyntaxList), + typeArgumentsWithAnnotations: default(ImmutableArray), + invoked: false, + indexed: false, + diagnostics: diagnostics); + + hasErrors |= boundMember.HasAnyErrors || implicitReceiver.HasAnyErrors; + + boundMember = CheckValue(boundMember, BindValueKind.Assignable, diagnostics); + + var boundRight = BindValue(assignment.Right, diagnostics, BindValueKind.RValue); + boundExpr = BindAssignment(expr, boundMember, boundRight, isRef: false, diagnostics); } - args.Add((member, boundRight)); + args.Add(boundExpr); } lookupResult.Free(); diff --git a/src/Compilers/CSharp/Portable/BoundTree/BoundNodes.xml b/src/Compilers/CSharp/Portable/BoundTree/BoundNodes.xml index 074906ffe45b6..af0ba6f97d5a9 100644 --- a/src/Compilers/CSharp/Portable/BoundTree/BoundNodes.xml +++ b/src/Compilers/CSharp/Portable/BoundTree/BoundNodes.xml @@ -2131,7 +2131,7 @@ - + diff --git a/src/Compilers/CSharp/Portable/Generated/BoundNodes.xml.Generated.cs b/src/Compilers/CSharp/Portable/Generated/BoundNodes.xml.Generated.cs index 8ac783c614d46..144b4548f74d2 100644 --- a/src/Compilers/CSharp/Portable/Generated/BoundNodes.xml.Generated.cs +++ b/src/Compilers/CSharp/Portable/Generated/BoundNodes.xml.Generated.cs @@ -7734,8 +7734,8 @@ public BoundExpressionWithNullability Update(BoundExpression expression, Nullabl internal sealed partial class BoundWithExpression : BoundExpression { - public BoundWithExpression(SyntaxNode syntax, BoundExpression receiver, MethodSymbol? cloneMethod, ImmutableArray<(Symbol? Member, BoundExpression Expression)> arguments, TypeSymbol type, bool hasErrors = false) - : base(BoundKind.WithExpression, syntax, type, hasErrors || receiver.HasErrors()) + public BoundWithExpression(SyntaxNode syntax, BoundExpression receiver, MethodSymbol? cloneMethod, ImmutableArray arguments, TypeSymbol type, bool hasErrors = false) + : base(BoundKind.WithExpression, syntax, type, hasErrors || receiver.HasErrors() || arguments.HasErrors()) { RoslynDebug.Assert(receiver is object, "Field 'receiver' cannot be null (make the type nullable in BoundNodes.xml to remove this check)"); @@ -7754,11 +7754,11 @@ public BoundWithExpression(SyntaxNode syntax, BoundExpression receiver, MethodSy public MethodSymbol? CloneMethod { get; } - public ImmutableArray<(Symbol? Member, BoundExpression Expression)> Arguments { get; } + public ImmutableArray Arguments { get; } [DebuggerStepThrough] public override BoundNode? Accept(BoundTreeVisitor visitor) => visitor.VisitWithExpression(this); - public BoundWithExpression Update(BoundExpression receiver, MethodSymbol? cloneMethod, ImmutableArray<(Symbol? Member, BoundExpression Expression)> arguments, TypeSymbol type) + public BoundWithExpression Update(BoundExpression receiver, MethodSymbol? cloneMethod, ImmutableArray arguments, TypeSymbol type) { if (receiver != this.Receiver || !Symbols.SymbolEqualityComparer.ConsiderEverything.Equals(cloneMethod, this.CloneMethod) || arguments != this.Arguments || !TypeSymbol.Equals(type, this.Type, TypeCompareKind.ConsiderEverything)) { @@ -9473,6 +9473,7 @@ internal abstract partial class BoundTreeWalker: BoundTreeVisitor public override BoundNode? VisitWithExpression(BoundWithExpression node) { this.Visit(node.Receiver); + this.VisitList(node.Arguments); return null; } } @@ -10638,8 +10639,9 @@ internal abstract partial class BoundTreeRewriter : BoundTreeVisitor public override BoundNode? VisitWithExpression(BoundWithExpression node) { BoundExpression receiver = (BoundExpression)this.Visit(node.Receiver); + ImmutableArray arguments = this.VisitList(node.Arguments); TypeSymbol? type = this.VisitType(node.Type); - return node.Update(receiver, node.CloneMethod, node.Arguments, type); + return node.Update(receiver, node.CloneMethod, arguments, type); } } @@ -12930,16 +12932,17 @@ public NullabilityRewriter(ImmutableDictionary arguments = this.VisitList(node.Arguments); BoundWithExpression updatedNode; if (_updatedNullabilities.TryGetValue(node, out (NullabilityInfo Info, TypeSymbol Type) infoAndType)) { - updatedNode = node.Update(receiver, cloneMethod, node.Arguments, infoAndType.Type); + updatedNode = node.Update(receiver, cloneMethod, arguments, infoAndType.Type); updatedNode.TopLevelNullability = infoAndType.Info; } else { - updatedNode = node.Update(receiver, cloneMethod, node.Arguments, node.Type); + updatedNode = node.Update(receiver, cloneMethod, arguments, node.Type); } return updatedNode; } @@ -14746,7 +14749,7 @@ private BoundTreeDumperNodeProducer() { new TreeDumperNode("receiver", null, new TreeDumperNode[] { Visit(node.Receiver, null) }), new TreeDumperNode("cloneMethod", node.CloneMethod, null), - new TreeDumperNode("arguments", node.Arguments, null), + new TreeDumperNode("arguments", null, from x in node.Arguments select Visit(x, null)), new TreeDumperNode("type", node.Type, null), new TreeDumperNode("isSuppressed", node.IsSuppressed, null), new TreeDumperNode("hasErrors", node.HasErrors, null) diff --git a/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_WithExpression.cs b/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_WithExpression.cs index 3f6b91f7b9540..238370f34d331 100644 --- a/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_WithExpression.cs +++ b/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_WithExpression.cs @@ -42,13 +42,37 @@ public override BoundNode VisitWithExpression(BoundWithExpression withExpr) // tmp.Pn = En; foreach (var arg in withExpr.Arguments) { - RoslynDebug.AssertNotNull(arg.Member); - // PROTOTYPE: only works for source symbols - var prop = (SynthesizedRecordPropertySymbol)arg.Member; - stores.Add(F.AssignmentExpression( - (BoundExpression)F.Field((BoundExpression)receiverLocal, (FieldSymbol)prop.BackingField), - (BoundExpression)VisitExpression((BoundExpression)arg.Expression) - )); + // If we got here without errors we should have a simple assignment with either a + // field or property on the left and a value expression on the right + var assignment = (BoundAssignmentOperator)arg; + // We need to construct the assignment LHS manually because swapping out the + // placeholder receiver could change the ref assignability of the expression, which + // the rest of lowering doesn't like + var left = assignment.Left.ExpressionSymbol switch + { + PropertySymbol p => MakePropertyAccess( + withExpr.Syntax, + receiverLocal, + p, + receiverLocal.ResultKind, + p.Type, + isLeftOfAssignment: true), + FieldSymbol f => MakeFieldAccess( + withExpr.Syntax, + receiverLocal, + f, + constantValueOpt: null, + receiverLocal.ResultKind, + f.Type), + _ => throw ExceptionUtilities.UnexpectedValue(assignment.Left.Kind) + }; + stores.Add(MakeStaticAssignmentOperator( + assignment.Syntax, + left, + VisitExpression(assignment.Right), + isRef: false, + left.Type!, + used: false)); } return new BoundSequence( diff --git a/src/Compilers/CSharp/Portable/Symbols/LexicalSortKey.cs b/src/Compilers/CSharp/Portable/Symbols/LexicalSortKey.cs index 3838e4b0e8252..87193c5cf81b7 100644 --- a/src/Compilers/CSharp/Portable/Symbols/LexicalSortKey.cs +++ b/src/Compilers/CSharp/Portable/Symbols/LexicalSortKey.cs @@ -44,8 +44,8 @@ public int Position public static readonly LexicalSortKey NotInitialized = new LexicalSortKey() { _treeOrdinal = -1, _position = -1 }; // Put Record Equals right before synthesized constructors. - public static LexicalSortKey SynthesizedRecordEquals => new LexicalSortKey() { _treeOrdinal = int.MaxValue, _position = int.MaxValue - 4 }; - public static LexicalSortKey SynthesizedRecordObjEquals => new LexicalSortKey() { _treeOrdinal = int.MaxValue, _position = int.MaxValue - 3 }; + public static LexicalSortKey SynthesizedRecordEquals => new LexicalSortKey() { _treeOrdinal = int.MaxValue, _position = int.MaxValue - 5 }; + public static LexicalSortKey SynthesizedRecordObjEquals => new LexicalSortKey() { _treeOrdinal = int.MaxValue, _position = int.MaxValue - 4 }; // Dev12 compiler adds synthetic constructors to the child list after adding all other members. @@ -53,6 +53,7 @@ public int Position // until later when it is known if they can be optimized or not. // As a result the last emitted method tokens are synthetic ctor and then synthetic cctor (if not optimized) // Since it is not too hard, we will try keeping the same order just to be easy on metadata diffing tools and such. + public static readonly LexicalSortKey SynthesizedRecordCopyCtor = new LexicalSortKey() { _treeOrdinal = int.MaxValue, _position = int.MaxValue - 3 }; public static readonly LexicalSortKey SynthesizedRecordCtor = new LexicalSortKey() { _treeOrdinal = int.MaxValue, _position = int.MaxValue - 2 }; public static readonly LexicalSortKey SynthesizedCtor = new LexicalSortKey() { _treeOrdinal = int.MaxValue, _position = int.MaxValue - 1 }; public static readonly LexicalSortKey SynthesizedCCtor = new LexicalSortKey() { _treeOrdinal = int.MaxValue, _position = int.MaxValue }; diff --git a/src/Compilers/CSharp/Portable/Symbols/Source/SourceMemberContainerSymbol.cs b/src/Compilers/CSharp/Portable/Symbols/Source/SourceMemberContainerSymbol.cs index b74be3f6d0441..e604285f7b697 100644 --- a/src/Compilers/CSharp/Portable/Symbols/Source/SourceMemberContainerSymbol.cs +++ b/src/Compilers/CSharp/Portable/Symbols/Source/SourceMemberContainerSymbol.cs @@ -2946,6 +2946,7 @@ private void AddSynthesizedRecordMembersIfNecessary(MembersAndInitializersBuilde BinderFactory binderFactory = this.DeclaringCompilation.GetBinderFactory(paramList.SyntaxTree); var binder = binderFactory.GetBinder(paramList); + // PROTOTYPE: need to check base members as well var memberSignatures = s_duplicateMemberSignatureDictionary.Allocate(); foreach (var member in members) { @@ -2953,6 +2954,11 @@ private void AddSynthesizedRecordMembersIfNecessary(MembersAndInitializersBuilde } var ctor = addCtor(paramList); + if (!this.IsStructType()) + { + addCopyCtor(); + } + addCloneMethod(); addProperties(ctor.Parameters); var thisEquals = addThisEquals(); addObjectEquals(thisEquals); @@ -2977,15 +2983,34 @@ SynthesizedRecordConstructor addCtor(ParameterListSyntax paramList) return ctor; } + void addCopyCtor() + { + var ctor = new SynthesizedRecordCopyCtor(this, diagnostics); + if (!memberSignatures.ContainsKey(ctor)) + { + members.Add(ctor); + } + } + + void addCloneMethod() + { + var clone = new SynthesizedRecordClone(this); + if (!memberSignatures.ContainsKey(clone)) + { + members.Add(clone); + } + } + void addProperties(ImmutableArray recordParameters) { foreach (ParameterSymbol param in ctor.Parameters) { - var property = new SynthesizedRecordPropertySymbol(this, param); + var property = new SynthesizedRecordPropertySymbol(this, param, diagnostics); if (!memberSignatures.ContainsKey(property)) { members.Add(property); members.Add(property.GetMethod); + members.Add(property.SetMethod); members.Add(property.BackingField); } } diff --git a/src/Compilers/CSharp/Portable/Symbols/Synthesized/Records/SynthesizedRecordClone.cs b/src/Compilers/CSharp/Portable/Symbols/Synthesized/Records/SynthesizedRecordClone.cs new file mode 100644 index 0000000000000..3880c0e26096b --- /dev/null +++ b/src/Compilers/CSharp/Portable/Symbols/Synthesized/Records/SynthesizedRecordClone.cs @@ -0,0 +1,139 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +#nullable enable + +using System; +using System.Collections.Generic; +using System.Collections.Immutable; +using System.Reflection; +using Microsoft.Cci; +using Microsoft.CodeAnalysis.CSharp.Emit; +using Microsoft.CodeAnalysis.PooledObjects; +using Roslyn.Utilities; + +namespace Microsoft.CodeAnalysis.CSharp.Symbols +{ + internal sealed class SynthesizedRecordClone : SynthesizedInstanceMethodSymbol + { + public override NamedTypeSymbol ContainingType { get; } + + public SynthesizedRecordClone(NamedTypeSymbol containingType) + { + ContainingType = containingType; + } + public override string Name => WellKnownMemberNames.CloneMethodName; + + public override MethodKind MethodKind => MethodKind.Ordinary; + + public override int Arity => 0; + + public override bool IsExtensionMethod => false; + + public override bool HidesBaseMethodsByName => false; + + public override bool IsVararg => false; + + public override bool ReturnsVoid => false; + + public override bool IsAsync => false; + + public override RefKind RefKind => RefKind.None; + + public override ImmutableArray Parameters => ImmutableArray.Empty; + + public override TypeWithAnnotations ReturnTypeWithAnnotations => TypeWithAnnotations.Create( + isNullableEnabled: true, + ContainingType); + + public override FlowAnalysisAnnotations ReturnTypeFlowAnalysisAnnotations => FlowAnalysisAnnotations.None; + + public override ImmutableHashSet ReturnNotNullIfParameterNotNull => ImmutableHashSet.Empty; + + public override ImmutableArray TypeArgumentsWithAnnotations + => ImmutableArray.Empty; + + public override ImmutableArray TypeParameters => ImmutableArray.Empty; + + public override ImmutableArray ExplicitInterfaceImplementations => ImmutableArray.Empty; + + public override ImmutableArray RefCustomModifiers => ImmutableArray.Empty; + + public override Symbol? AssociatedSymbol => null; + + public override Symbol ContainingSymbol => ContainingType; + + public override ImmutableArray Locations => ContainingType.Locations; + + public override Accessibility DeclaredAccessibility => Accessibility.Public; + + public override bool IsStatic => false; + + public override bool IsVirtual => true; + + public override bool IsOverride => false; + + public override bool IsAbstract => false; + + public override bool IsSealed => false; + + public override bool IsExtern => false; + + internal override bool HasSpecialName => true; + + internal override LexicalSortKey GetLexicalSortKey() => LexicalSortKey.SynthesizedRecordEquals; + + internal override MethodImplAttributes ImplementationAttributes => MethodImplAttributes.Managed; + + internal override bool HasDeclarativeSecurity => false; + + internal override MarshalPseudoCustomAttributeData? ReturnValueMarshallingInformation => null; + + internal override bool RequiresSecurityObject => false; + + internal override CallingConvention CallingConvention => CallingConvention.HasThis; + + internal override bool GenerateDebugInfo => false; + + public override DllImportData? GetDllImportData() => null; + + internal override ImmutableArray GetAppliedConditionalSymbols() + => ImmutableArray.Empty; + + internal override IEnumerable GetSecurityInformation() + => Array.Empty(); + + internal override bool IsMetadataNewSlot(bool ignoreInterfaceImplementationChanges = false) => false; + + internal override bool IsMetadataVirtual(bool ignoreInterfaceImplementationChanges = false) => false; + + internal override bool SynthesizesLoweredBoundBody => true; + + internal override void GenerateMethodBody(TypeCompilationState compilationState, DiagnosticBag diagnostics) + { + var F = new SyntheticBoundNodeFactory(this, ContainingType.GetNonNullSyntaxNode(), compilationState, diagnostics); + + // If this is a class, call the copy ctor. Otherwise, just return `this + if (ContainingType.IsStructType()) + { + F.CloseMethod(F.Block(F.Return(F.This()))); + return; + } + + var members = ContainingType.GetMembers(".ctor"); + foreach (var member in members) + { + var ctor = (MethodSymbol)member; + if (ctor.ParameterCount == 1 && + ctor.Parameters[0].Type.Equals(ContainingType, TypeCompareKind.ConsiderEverything)) + { + F.CloseMethod(F.Block(F.Return(F.New(ctor, F.This())))); + return; + } + } + + throw ExceptionUtilities.Unreachable; + } + } +} \ No newline at end of file diff --git a/src/Compilers/CSharp/Portable/Symbols/Synthesized/Records/SynthesizedRecordCopyCtor.cs b/src/Compilers/CSharp/Portable/Symbols/Synthesized/Records/SynthesizedRecordCopyCtor.cs new file mode 100644 index 0000000000000..b6a5ce7c7d72a --- /dev/null +++ b/src/Compilers/CSharp/Portable/Symbols/Synthesized/Records/SynthesizedRecordCopyCtor.cs @@ -0,0 +1,59 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +#nullable enable + +using System.Collections.Immutable; +using System.Diagnostics; +using Microsoft.CodeAnalysis.CSharp.Syntax; +using Microsoft.CodeAnalysis.PooledObjects; + +namespace Microsoft.CodeAnalysis.CSharp.Symbols +{ + internal sealed class SynthesizedRecordCopyCtor : SynthesizedInstanceConstructor + { + public SynthesizedRecordCopyCtor( + SourceMemberContainerTypeSymbol containingType, + DiagnosticBag diagnostics) + : base(containingType) + { + Debug.Assert(!containingType.IsStructType(), "only reference types should define copy constructors"); + Parameters = ImmutableArray.Create(SynthesizedParameterSymbol.Create( + this, + TypeWithAnnotations.Create( + isNullableEnabled: true, + ContainingType), + ordinal: 0, + RefKind.None)); + } + + public override ImmutableArray Parameters { get; } + + internal override LexicalSortKey GetLexicalSortKey() + { + // We need a separate sort key because struct records will have two synthesized + // constructors: the record constructor, and the parameterless constructor + return LexicalSortKey.SynthesizedRecordCopyCtor; + } + + internal override void GenerateMethodBodyStatements(SyntheticBoundNodeFactory F, ArrayBuilder statements, DiagnosticBag diagnostics) + { + // Write assignments to backing fields + // + // { + // this.backingField1 = arg1 + // ... + // this.backingFieldN = argN + // } + var param = F.Parameter(Parameters[0]); + foreach (var member in ContainingType.GetMembers()) + { + if (member is FieldSymbol { IsStatic: false } field) + { + statements.Add(F.Assignment(F.Field(F.This(), field), F.Field(param, field))); + } + } + } + } +} diff --git a/src/Compilers/CSharp/Portable/Symbols/Synthesized/Records/SynthesizedRecordPropertySymbol.cs b/src/Compilers/CSharp/Portable/Symbols/Synthesized/Records/SynthesizedRecordPropertySymbol.cs index 832ec997babf4..2c0badf5b1107 100644 --- a/src/Compilers/CSharp/Portable/Symbols/Synthesized/Records/SynthesizedRecordPropertySymbol.cs +++ b/src/Compilers/CSharp/Portable/Symbols/Synthesized/Records/SynthesizedRecordPropertySymbol.cs @@ -21,11 +21,13 @@ internal sealed class SynthesizedRecordPropertySymbol : SourceOrRecordPropertySy private readonly ParameterSymbol _backingParameter; internal override SynthesizedBackingFieldSymbol BackingField { get; } public override MethodSymbol GetMethod { get; } + public override MethodSymbol SetMethod { get; } public override NamedTypeSymbol ContainingType { get; } public SynthesizedRecordPropertySymbol( NamedTypeSymbol containingType, - ParameterSymbol backingParameter) + ParameterSymbol backingParameter, + DiagnosticBag diagnostics) : base(backingParameter.Locations[0]) { ContainingType = containingType; @@ -38,6 +40,7 @@ public SynthesizedRecordPropertySymbol( isStatic: false, hasInitializer: backingParameter.HasExplicitDefaultValue); GetMethod = new GetAccessorSymbol(this, name); + SetMethod = new InitAccessorSymbol(this, name, diagnostics); } internal override bool IsAutoProperty => true; @@ -52,8 +55,6 @@ public SynthesizedRecordPropertySymbol( public override bool IsIndexer => false; - public override MethodSymbol? SetMethod => null; - public override ImmutableArray ExplicitInterfaceImplementations => ImmutableArray.Empty; public override Symbol ContainingSymbol => ContainingType; @@ -207,5 +208,137 @@ internal override void GenerateMethodBody(TypeCompilationState compilationState, F.CloseMethod(F.Block(F.Return(F.Field(F.This(), _property.BackingField)))); } } + + private sealed class InitAccessorSymbol : SynthesizedInstanceMethodSymbol + { + private readonly SynthesizedRecordPropertySymbol _property; + + public override TypeWithAnnotations ReturnTypeWithAnnotations { get; } + public override string Name { get; } + + public InitAccessorSymbol( + SynthesizedRecordPropertySymbol property, + string paramName, + DiagnosticBag diagnostics) + { + _property = property; + Name = SourcePropertyAccessorSymbol.GetAccessorName( + paramName, + getNotSet: false, + isWinMdOutput: false /* PROTOTYPE */); + + var comp = property.DeclaringCompilation; + var type = TypeWithAnnotations.Create(comp.GetSpecialType(SpecialType.System_Void)); + var initOnlyType = Binder.GetWellKnownType( + comp, + WellKnownType.System_Runtime_CompilerServices_IsExternalInit, + diagnostics, + property.Location); + var modifiers = ImmutableArray.Create(CSharpCustomModifier.CreateRequired(initOnlyType)); + + ReturnTypeWithAnnotations = type.WithModifiers(modifiers); + } + + internal override bool IsInitOnly => true; + + public override MethodKind MethodKind => MethodKind.PropertySet; + + public override int Arity => 0; + + public override bool IsExtensionMethod => false; + + public override bool HidesBaseMethodsByName => false; + + public override bool IsVararg => false; + + public override bool ReturnsVoid => true; + + public override bool IsAsync => false; + + public override RefKind RefKind => RefKind.None; + + public override FlowAnalysisAnnotations ReturnTypeFlowAnalysisAnnotations => FlowAnalysisAnnotations.None; + + public override ImmutableArray TypeArgumentsWithAnnotations => ImmutableArray.Empty; + + public override ImmutableArray TypeParameters => ImmutableArray.Empty; + + public override ImmutableArray Parameters => ImmutableArray.Create(SynthesizedParameterSymbol.Create( + this, + _property.TypeWithAnnotations, + ordinal: 0, + RefKind.None, + name: ParameterSymbol.ValueParameterName)); + + public override ImmutableArray ExplicitInterfaceImplementations => ImmutableArray.Empty; + + public override ImmutableArray RefCustomModifiers => _property.RefCustomModifiers; + + public override Symbol AssociatedSymbol => _property; + + public override Symbol ContainingSymbol => _property.ContainingSymbol; + + public override ImmutableArray Locations => _property.Locations; + + public override Accessibility DeclaredAccessibility => _property.DeclaredAccessibility; + + public override bool IsStatic => _property.IsStatic; + + public override bool IsVirtual => _property.IsVirtual; + + public override bool IsOverride => _property.IsOverride; + + public override bool IsAbstract => _property.IsAbstract; + + public override bool IsSealed => _property.IsSealed; + + public override bool IsExtern => _property.IsExtern; + + public override ImmutableHashSet ReturnNotNullIfParameterNotNull => ImmutableHashSet.Empty; + + internal override bool HasSpecialName => _property.HasSpecialName; + + internal override MethodImplAttributes ImplementationAttributes => MethodImplAttributes.Managed; + + internal override bool HasDeclarativeSecurity => false; + + internal override MarshalPseudoCustomAttributeData? ReturnValueMarshallingInformation => null; + + internal override bool RequiresSecurityObject => false; + + internal override CallingConvention CallingConvention => CallingConvention.HasThis; + + internal override bool GenerateDebugInfo => false; + + public override DllImportData? GetDllImportData() => null; + + internal override ImmutableArray GetAppliedConditionalSymbols() + => ImmutableArray.Empty; + + internal override IEnumerable GetSecurityInformation() + => Array.Empty(); + + internal override bool IsMetadataNewSlot(bool ignoreInterfaceImplementationChanges = false) => false; + + internal override bool IsMetadataVirtual(bool ignoreInterfaceImplementationChanges = false) => false; + + internal override bool SynthesizesLoweredBoundBody => true; + + internal override void GenerateMethodBody(TypeCompilationState compilationState, DiagnosticBag diagnostics) + { + // Method body: + // + // { + // this.<>backingField = value; + // } + + var F = new SyntheticBoundNodeFactory(this, this.GetNonNullSyntaxNode(), compilationState, diagnostics); + + F.CurrentFunction = this; + F.CloseMethod(F.Block( + F.Assignment(F.Field(F.This(), _property.BackingField), F.Parameter(Parameters[0])), + F.Return())); + } + } } } diff --git a/src/Compilers/CSharp/Test/Semantic/Semantics/InitOnlyMemberTests.cs b/src/Compilers/CSharp/Test/Semantic/Semantics/InitOnlyMemberTests.cs index f4943988233d7..7784fc171731f 100644 --- a/src/Compilers/CSharp/Test/Semantic/Semantics/InitOnlyMemberTests.cs +++ b/src/Compilers/CSharp/Test/Semantic/Semantics/InitOnlyMemberTests.cs @@ -2172,15 +2172,18 @@ void M() { } var cMembers = comp.GlobalNamespace.GetMember("C").GetMembers(); // PROTOTYPE(init-only): expecting an 'init' setter - AssertEx.SetEqual(new[] { + AssertEx.SetEqual(cMembers.ToTestDisplayStrings(), new[] { "System.Int32 C.k__BackingField", "System.Int32 C.i.get", - "System.Int32 C.i { get; }", + "void modreq(System.Runtime.CompilerServices.IsExternalInit) C.i.init", + "System.Int32 C.i { get; init; }", "void C.M()", + "C C.Clone()", "System.Boolean C.Equals(C? )", "System.Boolean C.Equals(System.Object? )", "System.Int32 C.GetHashCode()", - "C..ctor(System.Int32 i)" }, cMembers.ToTestDisplayStrings()); + "C..ctor(C )", + "C..ctor(System.Int32 i)" }); foreach (var member in cMembers) { diff --git a/src/Compilers/CSharp/Test/Semantic/Semantics/LookupPositionTests.cs b/src/Compilers/CSharp/Test/Semantic/Semantics/LookupPositionTests.cs index 6144cf466a7f5..f0a39ce51fc29 100644 --- a/src/Compilers/CSharp/Test/Semantic/Semantics/LookupPositionTests.cs +++ b/src/Compilers/CSharp/Test/Semantic/Semantics/LookupPositionTests.cs @@ -51,8 +51,9 @@ public void PositionalRecord2() Add( // C Type parameters "T"), Add( // Members - "System.Int32 C.x { get; }", - "T C.t { get; }", + "C C.Clone()", + "System.Int32 C.x { get; init; }", + "T C.t { get; init; }", "System.Boolean C.Equals(C? )", "System.Boolean C.Equals(System.Object? )", "System.Boolean System.Object.Equals(System.Object obj)", diff --git a/src/Compilers/CSharp/Test/Semantic/Semantics/RecordTests.cs b/src/Compilers/CSharp/Test/Semantic/Semantics/RecordTests.cs index 16517d53049e4..61ae0167d34c1 100644 --- a/src/Compilers/CSharp/Test/Semantic/Semantics/RecordTests.cs +++ b/src/Compilers/CSharp/Test/Semantic/Semantics/RecordTests.cs @@ -16,13 +16,18 @@ namespace Microsoft.CodeAnalysis.CSharp.UnitTests.Semantics public class RecordTests : CompilingTestBase { private static CSharpCompilation CreateCompilation(CSharpTestSource source) - => CSharpTestBase.CreateCompilation(source, parseOptions: TestOptions.RegularPreview); + => CSharpTestBase.CreateCompilation(new[] { source, IsExternalInitTypeDefinition }, + parseOptions: TestOptions.RegularPreview); - private CompilationVerifier CompileAndVerify(CSharpTestSource src, string? expectedOutput = null) + private CompilationVerifier CompileAndVerify( + CSharpTestSource src, + string? expectedOutput = null, + IEnumerable? references = null) => base.CompileAndVerify( - src, + new[] { src, IsExternalInitTypeDefinition }, expectedOutput: expectedOutput, parseOptions: TestOptions.RegularPreview, + references: references, // init-only is unverifiable verify: Verification.Skipped); @@ -43,9 +48,15 @@ data class Point { } // (2,12): error CS8652: The feature 'records' is currently in Preview and *unsupported*. To use Preview features, use the 'preview' language version. // class Point(int x, int y); Diagnostic(ErrorCode.ERR_FeatureInPreview, "(int x, int y)").WithArguments("records").WithLocation(2, 12), - // (2,12): error CS8761: Records must have both a 'data' modifier and parameter list + // (2,12): error CS8800: Records must have both a 'data' modifier and non-empty parameter list // class Point(int x, int y); Diagnostic(ErrorCode.ERR_BadRecordDeclaration, "(int x, int y)").WithLocation(2, 12), + // (2,17): error CS0518: Predefined type 'System.Runtime.CompilerServices.IsExternalInit' is not defined or imported + // class Point(int x, int y); + Diagnostic(ErrorCode.ERR_PredefinedTypeNotFound, "x").WithArguments("System.Runtime.CompilerServices.IsExternalInit").WithLocation(2, 17), + // (2,24): error CS0518: Predefined type 'System.Runtime.CompilerServices.IsExternalInit' is not defined or imported + // class Point(int x, int y); + Diagnostic(ErrorCode.ERR_PredefinedTypeNotFound, "y").WithArguments("System.Runtime.CompilerServices.IsExternalInit").WithLocation(2, 24), // (2,26): error CS8652: The feature 'records' is currently in Preview and *unsupported*. To use Preview features, use the 'preview' language version. // class Point(int x, int y); Diagnostic(ErrorCode.ERR_FeatureInPreview, ";").WithArguments("records").WithLocation(2, 26) @@ -67,6 +78,12 @@ data class Point { } // (2,17): error CS8652: The feature 'records' is currently in Preview and *unsupported*. To use Preview features, use the 'preview' language version. // data class Point(int x, int y); Diagnostic(ErrorCode.ERR_FeatureInPreview, "(int x, int y)").WithArguments("records").WithLocation(2, 17), + // (2,22): error CS0518: Predefined type 'System.Runtime.CompilerServices.IsExternalInit' is not defined or imported + // data class Point(int x, int y); + Diagnostic(ErrorCode.ERR_PredefinedTypeNotFound, "x").WithArguments("System.Runtime.CompilerServices.IsExternalInit").WithLocation(2, 22), + // (2,29): error CS0518: Predefined type 'System.Runtime.CompilerServices.IsExternalInit' is not defined or imported + // data class Point(int x, int y); + Diagnostic(ErrorCode.ERR_PredefinedTypeNotFound, "y").WithArguments("System.Runtime.CompilerServices.IsExternalInit").WithLocation(2, 29), // (2,31): error CS8652: The feature 'records' is currently in Preview and *unsupported*. To use Preview features, use the 'preview' language version. // data class Point(int x, int y); Diagnostic(ErrorCode.ERR_FeatureInPreview, ";").WithArguments("records").WithLocation(2, 31) @@ -468,21 +485,20 @@ public static void Main() public void WithExpr2() { var src = @" +using System; data class C(int X) { public static void Main() { - var c = new C(0); - c = c with { }; + var c1 = new C(1); + c1 = c1 with { }; + var c2 = c1 with { X = 11 }; + Console.WriteLine(c1.X); + Console.WriteLine(c2.X); } }"; - var comp = CreateCompilation(src); - // PROTOTYPE: records don't auto-generate Clone at the moment - comp.VerifyDiagnostics( - // (7,13): error CS8803: The receiver type 'C' does not have an accessible parameterless instance method named "Clone". - // c = c with { }; - Diagnostic(ErrorCode.ERR_NoSingleCloneMethod, "c").WithArguments("C").WithLocation(7, 13) - ); + var verifier = CompileAndVerify(src, expectedOutput: @"1 +11"); } [Fact] @@ -531,14 +547,14 @@ public void WithExpr6() var src = @" class B { - public int X { get; } + public int X { get; init; } public B Clone() => null; } -data class C(int X) : B +class C : B { public static void Main() { - var c = new C(0); + var c = new C(); c = c with { }; } }"; @@ -559,11 +575,12 @@ class B public int X { get; } public virtual B Clone() => null; } -data class C(int X) : B +class C : B { + public new int X { get; init; } public static void Main() { - var c = new C(0); + var c = new C(); B b = c; b = b with { X = 0 }; var b2 = c with { X = 0 }; @@ -572,12 +589,12 @@ public static void Main() }"; var comp = CreateCompilation(src); comp.VerifyDiagnostics( - // (13,22): error CS8808: All arguments to a `with` expression must be compiler-generated record properties. + // (14,22): error CS0200: Property or indexer 'B.X' cannot be assigned to -- it is read only // b = b with { X = 0 }; - Diagnostic(ErrorCode.ERR_WithMemberIsNotRecordProperty, "X").WithLocation(13, 22), - // (14,27): error CS8808: All arguments to a `with` expression must be compiler-generated record properties. + Diagnostic(ErrorCode.ERR_AssgReadonlyProp, "X").WithArguments("B.X").WithLocation(14, 22), + // (15,27): error CS0200: Property or indexer 'B.X' cannot be assigned to -- it is read only // var b2 = c with { X = 0 }; - Diagnostic(ErrorCode.ERR_WithMemberIsNotRecordProperty, "X").WithLocation(14, 27) + Diagnostic(ErrorCode.ERR_AssgReadonlyProp, "X").WithArguments("B.X").WithLocation(15, 27) ); } @@ -678,7 +695,7 @@ .maxstack 3 IL_0011: callvirt ""C C.Clone()"" IL_0016: dup IL_0017: ldc.i4.5 - IL_0018: stfld ""int C.k__BackingField"" + IL_0018: callvirt ""void C.X.init"" IL_001d: callvirt ""int C.X.get"" IL_0022: call ""void System.Console.WriteLine(int)"" IL_0027: ret @@ -716,7 +733,7 @@ .maxstack 3 IL_000d: callvirt ""C C.Clone()"" IL_0012: dup IL_0013: ldc.i4.5 - IL_0014: stfld ""int C.k__BackingField"" + IL_0014: callvirt ""void C.X.init"" IL_0019: call ""void System.Console.WriteLine(object)"" IL_001e: ret }"); @@ -756,13 +773,13 @@ .maxstack 3 IL_000d: callvirt ""C C.Clone()"" IL_0012: dup IL_0013: ldc.i4.5 - IL_0014: stfld ""int C.k__BackingField"" + IL_0014: callvirt ""void C.X.init"" IL_0019: dup IL_001a: call ""void System.Console.WriteLine(object)"" IL_001f: callvirt ""C C.Clone()"" IL_0024: dup IL_0025: ldc.i4.2 - IL_0026: stfld ""int C.k__BackingField"" + IL_0026: callvirt ""void C.Y.init"" IL_002b: call ""void System.Console.WriteLine(object)"" IL_0030: ret }"); @@ -785,10 +802,7 @@ public static void Main() comp.VerifyDiagnostics( // (8,22): error CS1525: Invalid expression term '=' // c = c with { = 5 }; - Diagnostic(ErrorCode.ERR_InvalidExprTerm, "=").WithArguments("=").WithLocation(8, 22), - // (8,22): error CS1061: 'C' does not contain a definition for '' and no accessible extension method '' accepting a first argument of type 'C' could be found (are you missing a using directive or an assembly reference?) - // c = c with { = 5 }; - Diagnostic(ErrorCode.ERR_NoSuchMemberOrExtension, "").WithArguments("C", "").WithLocation(8, 22) + Diagnostic(ErrorCode.ERR_InvalidExprTerm, "=").WithArguments("=").WithLocation(8, 22) ); } @@ -908,10 +922,7 @@ public static void Main() comp.VerifyDiagnostics( // (5,25): warning CS0067: The event 'C.X' is never used // public event Action X; - Diagnostic(ErrorCode.WRN_UnreferencedEvent, "X").WithArguments("C.X").WithLocation(5, 25), - // (10,22): error CS8808: All arguments to a `with` expression must be compiler-generated record properties. - // c = c with { X = null }; - Diagnostic(ErrorCode.ERR_WithMemberIsNotRecordProperty, "X").WithLocation(10, 22) + Diagnostic(ErrorCode.WRN_UnreferencedEvent, "X").WithArguments("C.X").WithLocation(5, 25) ); } @@ -934,9 +945,12 @@ public static void Main() }"; var comp = CreateCompilation(src); comp.VerifyDiagnostics( - // (12,22): error CS8808: All arguments to a `with` expression must be compiler-generated record properties. + // (12,22): error CS0572: 'X': cannot reference a type through an expression; try 'B.X' instead // b = b with { X = 0 }; - Diagnostic(ErrorCode.ERR_WithMemberIsNotRecordProperty, "X").WithLocation(12, 22) + Diagnostic(ErrorCode.ERR_BadTypeReference, "X").WithArguments("X", "B.X").WithLocation(12, 22), + // (12,22): error CS0118: 'B.X' is a type but is used like a variable + // b = b with { X = 0 }; + Diagnostic(ErrorCode.ERR_BadSKknown, "X").WithArguments("B.X", "type", "variable").WithLocation(12, 22) ); } @@ -990,23 +1004,24 @@ public static void Main() public void WithExprNestedErrors() { var src = @" -data class C(int X) +class C { + public int X { get; init; } public static void Main() { - var c = new C(0); + var c = new C(); c = c with { X = """"-3 }; } } "; var comp = CreateCompilation(src); comp.VerifyDiagnostics( - // (7,13): error CS8803: The 'with' expression requires the receiver type 'C' to have a single accessible non-inherited instance method named "Clone". + // (8,13): error CS8808: The receiver type 'C' does not have an accessible parameterless instance method named "Clone". // c = c with { X = ""-3 }; - Diagnostic(ErrorCode.ERR_NoSingleCloneMethod, "c").WithArguments("C").WithLocation(7, 13), - // (7,26): error CS0019: Operator '-' cannot be applied to operands of type 'string' and 'int' + Diagnostic(ErrorCode.ERR_NoSingleCloneMethod, "c").WithArguments("C").WithLocation(8, 13), + // (8,26): error CS0019: Operator '-' cannot be applied to operands of type 'string' and 'int' // c = c with { X = ""-3 }; - Diagnostic(ErrorCode.ERR_BadBinaryOps, @"""""-3").WithArguments("-", "string", "int").WithLocation(7, 26) + Diagnostic(ErrorCode.ERR_BadBinaryOps, @"""""-3").WithArguments("-", "string", "int").WithLocation(8, 26) ); } @@ -1033,27 +1048,27 @@ public static void Main() } [Fact] - public void WithExprPropertyInaccessibleGet() + public void WithExprPropertyInaccessibleSet() { var src = @" -data class C(int X) +class C { - public int X { private get; set; } + public int X { get; private set; } public C Clone() => null; } class D { public static void Main() { - var c = new C(0); + var c = new C(); c = c with { X = 0 }; } }"; var comp = CreateCompilation(src); comp.VerifyDiagnostics( - // (12,22): error CS8808: All arguments to a `with` expression must be compiler-generated record properties. + // (12,22): error CS0272: The property or indexer 'C.X' cannot be used in this context because the set accessor is inaccessible // c = c with { X = 0 }; - Diagnostic(ErrorCode.ERR_WithMemberIsNotRecordProperty, "X").WithLocation(12, 22) + Diagnostic(ErrorCode.ERR_InaccessibleSetter, "X").WithArguments("C.X").WithLocation(12, 22) ); } @@ -1093,11 +1108,11 @@ .maxstack 3 IL_000d: dup IL_000e: ldstr ""Y"" IL_0013: call ""int C.W(string)"" - IL_0018: stfld ""int C.k__BackingField"" + IL_0018: callvirt ""void C.Y.init"" IL_001d: dup IL_001e: ldstr ""X"" IL_0023: call ""int C.W(string)"" - IL_0028: stfld ""int C.k__BackingField"" + IL_0028: callvirt ""void C.X.init"" IL_002d: pop IL_002e: ret }"); @@ -1129,7 +1144,7 @@ .maxstack 3 IL_000c: dup IL_000d: ldc.i4.s 11 IL_000f: conv.i8 - IL_0010: stfld ""long C.k__BackingField"" + IL_0010: callvirt ""void C.X.init"" IL_0015: callvirt ""long C.X.get"" IL_001a: call ""void System.Console.WriteLine(long)"" IL_001f: ret @@ -1182,7 +1197,7 @@ .locals init (S V_0) //s IL_0015: dup IL_0016: ldloc.0 IL_0017: call ""long S.op_Implicit(S)"" - IL_001c: stfld ""long C.k__BackingField"" + IL_001c: callvirt ""void C.X.init"" IL_0021: callvirt ""long C.X.get"" IL_0026: call ""void System.Console.WriteLine(long)"" IL_002b: ret @@ -1271,26 +1286,81 @@ public static void Main() var verifier = CompileAndVerify(src, expectedOutput: "abc"); } + [Fact] + public void WithExprConversions6() + { + var src = @" +using System; +struct S +{ + private int _i; + public S(int i) + { + _i = i; + } + public static implicit operator int(S s) + { + Console.WriteLine(""conversion""); + return s._i; + } +} +class C +{ + private readonly long _x; + public long X { get => _x; init { Console.WriteLine(""set""); _x = value; } } + public C Clone() => new C(); + public static void Main() + { + var c = new C(); + var s = new S(11); + Console.WriteLine((c with { X = s }).X); + } +}"; + var verifier = CompileAndVerify(src, expectedOutput: @" +conversion +set +11"); + verifier.VerifyIL("C.Main", @" +{ + // Code size 43 (0x2b) + .maxstack 3 + .locals init (S V_0) //s + IL_0000: newobj ""C..ctor()"" + IL_0005: ldloca.s V_0 + IL_0007: ldc.i4.s 11 + IL_0009: call ""S..ctor(int)"" + IL_000e: callvirt ""C C.Clone()"" + IL_0013: dup + IL_0014: ldloc.0 + IL_0015: call ""int S.op_Implicit(S)"" + IL_001a: conv.i8 + IL_001b: callvirt ""void C.X.init"" + IL_0020: callvirt ""long C.X.get"" + IL_0025: call ""void System.Console.WriteLine(long)"" + IL_002a: ret +}"); + } + [Fact] public void WithExprStaticProperty() { var src = @" -data class C(int X) +class C { public static int X { get; } public C Clone() => null; public static void Main() { - var c = new C(0); + var c = new C(); c = c with { }; c = c with { X = 11 }; } }"; var comp = CreateCompilation(src); comp.VerifyDiagnostics( - // (10,22): error CS8808: All arguments to a `with` expression must be instance properties or fields. + // (10,22): error CS0176: Member 'C.X' cannot be accessed with an instance reference; qualify it with a type name instead // c = c with { X = 11 }; - Diagnostic(ErrorCode.ERR_WithMemberIsNotRecordProperty, "X").WithLocation(10, 22) + Diagnostic(ErrorCode.ERR_ObjectProhibited, "X").WithArguments("C.X").WithLocation(10, 22) ); } @@ -1311,33 +1381,35 @@ public static void Main() }"; var comp = CreateCompilation(src); comp.VerifyDiagnostics( - // (10,22): error CS8808: All arguments to a `with` expression must be compiler-generated record properties. + // (10,22): error CS1656: Cannot assign to 'X' because it is a 'method group' // c = c with { X = 11 }; - Diagnostic(ErrorCode.ERR_WithMemberIsNotRecordProperty, "X").WithLocation(10, 22)); + Diagnostic(ErrorCode.ERR_AssgReadonlyLocalCause, "X").WithArguments("X", "method group").WithLocation(10, 22) + ); } [Fact] public void WithExprStaticWithMethod() { var src = @" -data class C(int X) +class C { + public int X = 0; public static C Clone() => null; public static void Main() { - var c = new C(0); + var c = new C(); c = c with { }; c = c with { X = 11 }; } }"; var comp = CreateCompilation(src); comp.VerifyDiagnostics( - // (8,13): error CS8803: The receiver type 'C' does not have an accessible parameterless instance method named "Clone". + // (9,13): error CS8808: The receiver type 'C' does not have an accessible parameterless instance method named "Clone". // c = c with { }; - Diagnostic(ErrorCode.ERR_NoSingleCloneMethod, "c").WithArguments("C").WithLocation(8, 13), - // (9,13): error CS8803: The receiver type 'C' does not have an accessible parameterless instance method named "Clone". + Diagnostic(ErrorCode.ERR_NoSingleCloneMethod, "c").WithArguments("C").WithLocation(9, 13), + // (10,13): error CS8808: The receiver type 'C' does not have an accessible parameterless instance method named "Clone". // c = c with { X = 11 }; - Diagnostic(ErrorCode.ERR_NoSingleCloneMethod, "c").WithArguments("C").WithLocation(9, 13) + Diagnostic(ErrorCode.ERR_NoSingleCloneMethod, "c").WithArguments("C").WithLocation(10, 13) ); } @@ -1349,24 +1421,25 @@ class B { public B Clone() => null; } -data class C(int X) : B +class C : B { - public static new C Clone() => null; + public int X = 0; + public static new C Clone() => null; // static public static void Main() { - var c = new C(0); + var c = new C(); c = c with { }; c = c with { X = 11 }; } }"; var comp = CreateCompilation(src); comp.VerifyDiagnostics( - // (12,13): error CS0266: Cannot implicitly convert type 'B' to 'C'. An explicit conversion exists (are you missing a cast?) + // (13,13): error CS0266: Cannot implicitly convert type 'B' to 'C'. An explicit conversion exists (are you missing a cast?) // c = c with { }; - Diagnostic(ErrorCode.ERR_NoImplicitConvCast, "c with { }").WithArguments("B", "C").WithLocation(12, 13), - // (13,22): error CS1061: 'B' does not contain a definition for 'X' and no accessible extension method 'X' accepting a first argument of type 'B' could be found (are you missing a using directive or an assembly reference?) + Diagnostic(ErrorCode.ERR_NoImplicitConvCast, "c with { }").WithArguments("B", "C").WithLocation(13, 13), + // (14,22): error CS1061: 'B' does not contain a definition for 'X' and no accessible extension method 'X' accepting a first argument of type 'B' could be found (are you missing a using directive or an assembly reference?) // c = c with { X = 11 }; - Diagnostic(ErrorCode.ERR_NoSuchMemberOrExtension, "X").WithArguments("B", "X").WithLocation(13, 22) + Diagnostic(ErrorCode.ERR_NoSuchMemberOrExtension, "X").WithArguments("B", "X").WithLocation(14, 22) ); } @@ -1377,7 +1450,7 @@ public void WithExprBadMemberBadType() class C { public C Clone() => null; - public int X { get; } + public int X { get; init; } public static void Main() { var c = new C(); @@ -1386,9 +1459,6 @@ public static void Main() }"; var comp = CreateCompilation(src); comp.VerifyDiagnostics( - // (9,22): error CS8808: All arguments to a `with` expression must be compiler-generated record properties. - // c = c with { X = "a" }; - Diagnostic(ErrorCode.ERR_WithMemberIsNotRecordProperty, "X").WithLocation(9, 22), // (9,26): error CS0029: Cannot implicitly convert type 'string' to 'int' // c = c with { X = "a" }; Diagnostic(ErrorCode.ERR_NoImplicitConv, @"""a""").WithArguments("string", "int").WithLocation(9, 26) @@ -1399,30 +1469,13 @@ public static void Main() public void WithExprCloneReturnDifferent() { var src = @" -data class B(int X) { } -class C : B -{ - public C(int x) : base(x) { } - public B Clone() => new B(0); - public static void Main() - { - var c = new C(0); - var b = c with { X = 0 }; - } -}"; - var comp = CreateCompilation(src); - comp.VerifyDiagnostics(); - } - - [Fact] - public void WithExprCloneReturnDifferent2() - { - var src = @" -data class B(int X) { } +class B +{ + public int X { get; init; } +} class C : B { - public C() : base(0) {} - public B Clone() => new B(0); + public B Clone() => new B(); public static void Main() { var c = new C(); @@ -1500,5 +1553,101 @@ public static void Main() Diagnostic(ErrorCode.ERR_EOFExpected, "}").WithLocation(11, 1) ); } + + [Fact] + public void WithExprNotRecord() + { + var src = @" +using System; +class C +{ + public int X { get; set; } + public string Y { get; init; } + public long Z; + public event Action E; + + public C Clone() => new C { + X = this.X, + Y = this.Y, + Z = this.Z, + E = this.E, + }; + + public static void Main() + { + var c = new C() { X = 1, Y = ""2"", Z = 3, E = () => { } }; + var c2 = c with {}; + Console.WriteLine(c.Equals(c2)); + Console.WriteLine(c2.X); + Console.WriteLine(c2.Y); + Console.WriteLine(c2.Z); + Console.WriteLine(ReferenceEquals(c.E, c2.E)); + var c3 = c with { Y = ""3"", X = 2 }; + Console.WriteLine(c.Y); + Console.WriteLine(c3.Y); + Console.WriteLine(c.X); + Console.WriteLine(c3.X); + } +}"; + var verifier = CompileAndVerify(src, expectedOutput: @" +False +1 +2 +3 +True +2 +3 +1 +2"); + } + + [Fact] + public void WithExprNotRecord2() + { + var comp1 = CreateCompilation(@" +public class C +{ + public int X { get; set; } + public string Y { get; init; } + public long Z; + + public C Clone() => new C { + X = this.X, + Y = this.Y, + Z = this.Z, + }; +}"); + comp1.VerifyDiagnostics(); + + var verifier = CompileAndVerify(@" +class D +{ + public C M(C c) => c with + { + X = 5, + Y = ""a"", + Z = 2, + }; +}", references: new[] { comp1.EmitToImageReference() }); + + verifier.VerifyIL("D.M", @" +{ + // Code size 33 (0x21) + .maxstack 3 + IL_0000: ldarg.1 + IL_0001: callvirt ""C C.Clone()"" + IL_0006: dup + IL_0007: ldc.i4.5 + IL_0008: callvirt ""void C.X.set"" + IL_000d: dup + IL_000e: ldstr ""a"" + IL_0013: callvirt ""void C.Y.init"" + IL_0018: dup + IL_0019: ldc.i4.2 + IL_001a: conv.i8 + IL_001b: stfld ""long C.Z"" + IL_0020: ret +}"); + } } } \ No newline at end of file diff --git a/src/Compilers/CSharp/Test/Symbol/Symbols/Source/RecordTests.cs b/src/Compilers/CSharp/Test/Symbol/Symbols/Source/RecordTests.cs index 6a458908d4ad6..1c1d66fcb1f9e 100644 --- a/src/Compilers/CSharp/Test/Symbol/Symbols/Source/RecordTests.cs +++ b/src/Compilers/CSharp/Test/Symbol/Symbols/Source/RecordTests.cs @@ -4,7 +4,6 @@ #nullable enable -using System.Collections.Immutable; using Microsoft.CodeAnalysis.CSharp.Symbols; using Microsoft.CodeAnalysis.CSharp.Test.Utilities; using Microsoft.CodeAnalysis.Test.Utilities; @@ -15,10 +14,14 @@ namespace Microsoft.CodeAnalysis.CSharp.UnitTests public class RecordTests : CompilingTestBase { private static CSharpCompilation CreateCompilation(CSharpTestSource source) - => CSharpTestBase.CreateCompilation(source, parseOptions: TestOptions.RegularPreview); + => CSharpTestBase.CreateCompilation(new[] { source, IsExternalInitTypeDefinition }, parseOptions: TestOptions.RegularPreview); private CompilationVerifier CompileAndVerify(CSharpTestSource src, string? expectedOutput = null) - => base.CompileAndVerify(src, expectedOutput: expectedOutput, parseOptions: TestOptions.RegularPreview); + => base.CompileAndVerify(new[] { src, IsExternalInitTypeDefinition }, + expectedOutput: expectedOutput, + parseOptions: TestOptions.RegularPreview, + // init-only fails verification + verify: Verification.Skipped); [Fact] public void GeneratedConstructor() @@ -26,7 +29,7 @@ public void GeneratedConstructor() var comp = CreateCompilation(@"data class C(int x, string y);"); comp.VerifyDiagnostics(); var c = comp.GlobalNamespace.GetTypeMember("C"); - var ctor = c.GetMethod(".ctor"); + var ctor = (MethodSymbol)c.GetMembers(".ctor")[0]; Assert.Equal(2, ctor.ParameterCount); var x = ctor.Parameters[0]; @@ -45,7 +48,7 @@ public void GeneratedConstructorDefaultValues() comp.VerifyDiagnostics(); var c = comp.GlobalNamespace.GetTypeMember("C"); Assert.Equal(1, c.Arity); - var ctor = c.GetMethod(".ctor"); + var ctor = (MethodSymbol)c.GetMembers(".ctor")[0]; Assert.Equal(0, ctor.Arity); Assert.Equal(2, ctor.ParameterCount); @@ -74,7 +77,7 @@ public C(int a, string b) Diagnostic(ErrorCode.ERR_DuplicateRecordConstructor, "(int x, string y)").WithLocation(2, 13) ); var c = comp.GlobalNamespace.GetTypeMember("C"); - var ctor = c.GetMethod(".ctor"); + var ctor = (MethodSymbol)c.GetMembers(".ctor")[0]; Assert.Equal(2, ctor.ParameterCount); var a = ctor.Parameters[0]; @@ -99,26 +102,32 @@ public C(int a, int b) // overload comp.VerifyDiagnostics(); var c = comp.GlobalNamespace.GetTypeMember("C"); var ctors = c.GetMembers(".ctor"); - Assert.Equal(2, ctors.Length); + Assert.Equal(3, ctors.Length); foreach (MethodSymbol ctor in ctors) { - Assert.Equal(2, ctor.ParameterCount); - - var p1 = ctor.Parameters[0]; - Assert.Equal(SpecialType.System_Int32, p1.Type.SpecialType); - var p2 = ctor.Parameters[1]; - if (ctor is SynthesizedRecordConstructor) + if (ctor.ParameterCount == 2) { - Assert.Equal("x", p1.Name); - Assert.Equal("y", p2.Name); - Assert.Equal(SpecialType.System_String, p2.Type.SpecialType); + var p1 = ctor.Parameters[0]; + Assert.Equal(SpecialType.System_Int32, p1.Type.SpecialType); + var p2 = ctor.Parameters[1]; + if (ctor is SynthesizedRecordConstructor) + { + Assert.Equal("x", p1.Name); + Assert.Equal("y", p2.Name); + Assert.Equal(SpecialType.System_String, p2.Type.SpecialType); + } + else + { + Assert.Equal("a", p1.Name); + Assert.Equal("b", p2.Name); + Assert.Equal(SpecialType.System_Int32, p2.Type.SpecialType); + } } else { - Assert.Equal("a", p1.Name); - Assert.Equal("b", p2.Name); - Assert.Equal(SpecialType.System_Int32, p2.Type.SpecialType); + Assert.Equal(1, ctor.ParameterCount); + Assert.True(c.Equals(ctor.Parameters[0].Type, TypeCompareKind.ConsiderEverything)); } } } @@ -134,7 +143,8 @@ public void GeneratedProperties() Assert.NotNull(x.GetMethod); Assert.Equal(MethodKind.PropertyGet, x.GetMethod.MethodKind); Assert.Equal(SpecialType.System_Int32, x.Type.SpecialType); - Assert.True(x.IsReadOnly); + Assert.False(x.IsReadOnly); + Assert.False(x.IsWriteOnly); Assert.Equal(Accessibility.Public, x.DeclaredAccessibility); Assert.False(x.IsVirtual); Assert.False(x.IsStatic); @@ -150,12 +160,21 @@ public void GeneratedProperties() Assert.Equal(x, getAccessor.AssociatedSymbol); Assert.Equal(c, getAccessor.ContainingSymbol); Assert.Equal(c, getAccessor.ContainingType); + Assert.Equal(Accessibility.Public, getAccessor.DeclaredAccessibility); + + var setAccessor = x.SetMethod; + Assert.Equal(x, setAccessor.AssociatedSymbol); + Assert.Equal(c, setAccessor.ContainingSymbol); + Assert.Equal(c, setAccessor.ContainingType); + Assert.Equal(Accessibility.Public, setAccessor.DeclaredAccessibility); + Assert.True(setAccessor.IsInitOnly); var y = (SourceOrRecordPropertySymbol)c.GetProperty("y"); Assert.NotNull(y.GetMethod); Assert.Equal(MethodKind.PropertyGet, y.GetMethod.MethodKind); Assert.Equal(SpecialType.System_Int32, y.Type.SpecialType); - Assert.True(y.IsReadOnly); + Assert.False(y.IsReadOnly); + Assert.False(y.IsWriteOnly); Assert.Equal(Accessibility.Public, y.DeclaredAccessibility); Assert.False(x.IsVirtual); Assert.False(x.IsStatic); @@ -171,6 +190,13 @@ public void GeneratedProperties() Assert.Equal(y, getAccessor.AssociatedSymbol); Assert.Equal(c, getAccessor.ContainingSymbol); Assert.Equal(c, getAccessor.ContainingType); + + setAccessor = y.SetMethod; + Assert.Equal(y, setAccessor.AssociatedSymbol); + Assert.Equal(c, setAccessor.ContainingSymbol); + Assert.Equal(c, setAccessor.ContainingType); + Assert.Equal(Accessibility.Public, setAccessor.DeclaredAccessibility); + Assert.True(setAccessor.IsInitOnly); } [Fact] @@ -355,7 +381,6 @@ public void RecordEquals_08() data class C(int X, int Y) { public int Z; - public static void Main() { var c = new C(1, 2); @@ -412,7 +437,6 @@ public void RecordEquals_09() data class C(int X, int Y) { public int Z { get; set; } - public static void Main() { var c = new C(1, 2); @@ -439,7 +463,6 @@ public void RecordEquals_10() data class C(int X, int Y) { public static int Z; - public static void Main() { var c = new C(1, 2); @@ -491,7 +514,6 @@ data class C(int X, int Y) { static Dictionary s_dict = new Dictionary(); public int Z { get => s_dict[this]; set => s_dict[this] = value; } - public static void Main() { var c = new C(1, 2); @@ -542,7 +564,6 @@ public void RecordEquals_12() data class C(int X, int Y) { private event Action E; - public static void Main() { var c = new C(1, 2); @@ -588,6 +609,90 @@ .maxstack 3 IL_0049: ret IL_004a: ldc.i4.0 IL_004b: ret +}"); + } + + [Fact] + public void RecordClone1() + { + var comp = CreateCompilation("data class C(int x, int y);"); + comp.VerifyDiagnostics(); + + var c = comp.GlobalNamespace.GetTypeMember("C"); + var clone = c.GetMethod(WellKnownMemberNames.CloneMethodName); + Assert.Equal(0, clone.Arity); + Assert.Equal(0, clone.ParameterCount); + Assert.Equal(c, clone.ReturnType); + + var ctor = (MethodSymbol)c.GetMembers(".ctor")[1]; + Assert.Equal(1, ctor.ParameterCount); + Assert.True(ctor.Parameters[0].Type.Equals(c, TypeCompareKind.ConsiderEverything)); + + var verifier = CompileAndVerify(comp, verify: Verification.Fails); + verifier.VerifyIL("C." + WellKnownMemberNames.CloneMethodName, @" +{ + // Code size 7 (0x7) + .maxstack 1 + IL_0000: ldarg.0 + IL_0001: newobj ""C..ctor(C)"" + IL_0006: ret +} +"); + verifier.VerifyIL("C..ctor(C)", @" +{ + // Code size 31 (0x1f) + .maxstack 2 + IL_0000: ldarg.0 + IL_0001: call ""object..ctor()"" + IL_0006: ldarg.0 + IL_0007: ldarg.1 + IL_0008: ldfld ""int C.k__BackingField"" + IL_000d: stfld ""int C.k__BackingField"" + IL_0012: ldarg.0 + IL_0013: ldarg.1 + IL_0014: ldfld ""int C.k__BackingField"" + IL_0019: stfld ""int C.k__BackingField"" + IL_001e: ret +}"); + } + + [Fact] + public void RecordClone2() + { + var comp = CreateCompilation(@" +data class C(int x, int y) +{ + public C(C other) { } +}"); + comp.VerifyDiagnostics(); + + var c = comp.GlobalNamespace.GetTypeMember("C"); + var clone = c.GetMethod(WellKnownMemberNames.CloneMethodName); + Assert.Equal(0, clone.Arity); + Assert.Equal(0, clone.ParameterCount); + Assert.Equal(c, clone.ReturnType); + + var ctor = (MethodSymbol)c.GetMembers(".ctor")[0]; + Assert.Equal(1, ctor.ParameterCount); + Assert.True(ctor.Parameters[0].Type.Equals(c, TypeCompareKind.ConsiderEverything)); + + var verifier = CompileAndVerify(comp, verify: Verification.Fails); + verifier.VerifyIL("C." + WellKnownMemberNames.CloneMethodName, @" +{ + // Code size 7 (0x7) + .maxstack 1 + IL_0000: ldarg.0 + IL_0001: newobj ""C..ctor(C)"" + IL_0006: ret +} +"); + verifier.VerifyIL("C..ctor(C)", @" +{ + // Code size 7 (0x7) + .maxstack 1 + IL_0000: ldarg.0 + IL_0001: call ""object..ctor()"" + IL_0006: ret }"); } } From 74e8c4ac72bacf9cf6e76f304ee43a61f8021092 Mon Sep 17 00:00:00 2001 From: Andy Gocke Date: Wed, 27 May 2020 15:27:03 -0700 Subject: [PATCH 02/17] Add tests --- .../Test/Semantic/Semantics/RecordTests.cs | 91 +++++++++++++++++++ 1 file changed, 91 insertions(+) diff --git a/src/Compilers/CSharp/Test/Semantic/Semantics/RecordTests.cs b/src/Compilers/CSharp/Test/Semantic/Semantics/RecordTests.cs index 61ae0167d34c1..1d3fbe50058e2 100644 --- a/src/Compilers/CSharp/Test/Semantic/Semantics/RecordTests.cs +++ b/src/Compilers/CSharp/Test/Semantic/Semantics/RecordTests.cs @@ -1649,5 +1649,96 @@ .maxstack 3 IL_0020: ret }"); } + + [Fact] + public void WithExprAssignToRef1() + { + var src = @" +using System; +data class C(int Y) +{ + private readonly int[] _a = new[] { 0 }; + public ref int X => ref _a[0]; + + public C Clone() => new C(0); + + public static void Main() + { + var c = new C(0) { X = 5 }; + Console.WriteLine(c.X); + c = c with { X = 1 }; + Console.WriteLine(c.X); + } +}"; + var verifier = CompileAndVerify(src, expectedOutput: @" +5 +1"); + verifier.VerifyIL("C.Main", @" +{ + // Code size 51 (0x33) + .maxstack 3 + IL_0000: ldc.i4.0 + IL_0001: newobj ""C..ctor(int)"" + IL_0006: dup + IL_0007: callvirt ""ref int C.X.get"" + IL_000c: ldc.i4.5 + IL_000d: stind.i4 + IL_000e: dup + IL_000f: callvirt ""ref int C.X.get"" + IL_0014: ldind.i4 + IL_0015: call ""void System.Console.WriteLine(int)"" + IL_001a: callvirt ""C C.Clone()"" + IL_001f: dup + IL_0020: callvirt ""ref int C.X.get"" + IL_0025: ldc.i4.1 + IL_0026: stind.i4 + IL_0027: callvirt ""ref int C.X.get"" + IL_002c: ldind.i4 + IL_002d: call ""void System.Console.WriteLine(int)"" + IL_0032: ret +}"); + } + + [Fact] + public void WithExprAssignToRef2() + { + var src = @" +using System; +data class C(int Y) +{ + private readonly int[] _a = new[] { 0 }; + public ref int X + { + get => ref _a[0]; + set { } + } + + public C Clone() => new C(0); + + public static void Main() + { + var a = new[] { 0 }; + var c = new C(0) { X = ref a[0] }; + Console.WriteLine(c.X); + c = c with { X = ref a[0] }; + Console.WriteLine(c.X); + } +}"; + var comp = CreateCompilation(src); + comp.VerifyDiagnostics( + // (9,9): error CS8147: Properties which return by reference cannot have set accessors + // set { } + Diagnostic(ErrorCode.ERR_RefPropertyCannotHaveSetAccessor, "set").WithArguments("C.X.set").WithLocation(9, 9), + // (17,32): error CS1525: Invalid expression term 'ref' + // var c = new C(0) { X = ref a[0] }; + Diagnostic(ErrorCode.ERR_InvalidExprTerm, "ref a[0]").WithArguments("ref").WithLocation(17, 32), + // (17,32): error CS1073: Unexpected token 'ref' + // var c = new C(0) { X = ref a[0] }; + Diagnostic(ErrorCode.ERR_UnexpectedToken, "ref").WithArguments("ref").WithLocation(17, 32), + // (19,26): error CS1073: Unexpected token 'ref' + // c = c with { X = ref a[0] }; + Diagnostic(ErrorCode.ERR_UnexpectedToken, "ref").WithArguments("ref").WithLocation(19, 26) + ); + } } } \ No newline at end of file From 84d9dbce93298ffd43ba1c684be61131d8e31054 Mon Sep 17 00:00:00 2001 From: Rikki Gibson Date: Wed, 27 May 2020 16:21:24 -0700 Subject: [PATCH 03/17] Implement flow analysis for with expressions --- .../Portable/FlowAnalysis/AbstractFlowPass.cs | 6 +- .../NullableWalker.DebugVerifier.cs | 14 ++ .../Portable/FlowAnalysis/NullableWalker.cs | 2 +- .../Test/Semantic/Semantics/RecordTests.cs | 221 ++++++++++++++++++ 4 files changed, 241 insertions(+), 2 deletions(-) diff --git a/src/Compilers/CSharp/Portable/FlowAnalysis/AbstractFlowPass.cs b/src/Compilers/CSharp/Portable/FlowAnalysis/AbstractFlowPass.cs index 24bc8642e6e69..ec9bd56d0afff 100644 --- a/src/Compilers/CSharp/Portable/FlowAnalysis/AbstractFlowPass.cs +++ b/src/Compilers/CSharp/Portable/FlowAnalysis/AbstractFlowPass.cs @@ -2026,7 +2026,11 @@ public override BoundNode VisitWhileStatement(BoundWhileStatement node) public override BoundNode VisitWithExpression(BoundWithExpression node) { - // PROTOTYPE: This is wrong + VisitRvalue(node.Receiver); + foreach (var (_, expr) in node.Arguments) + { + VisitRvalue(expr); + } return null; } diff --git a/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.DebugVerifier.cs b/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.DebugVerifier.cs index c6fa8b084c6d6..9093c4bb71816 100644 --- a/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.DebugVerifier.cs +++ b/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.DebugVerifier.cs @@ -187,6 +187,20 @@ private void VisitBinaryOperatorChildren(BoundBinaryOperatorBase node) Visit(node.BoundContainingTypeOpt); return null; } + + public override BoundNode? VisitWithExpression(BoundWithExpression node) + { + Visit(node.Receiver); + // PROTOTYPE: We shouldn't need to implement this method manually here. + // Either BoundNodeClassWriter should be made aware of tuples, + // or the `BoundWithExpression.Arguments` should be changed to + // `ImmutableArray` as in `BoundObjectInitializerExpressionBase.Initializers`. + foreach (var (_, expr) in node.Arguments) + { + Visit(expr); + } + return null; + } } #endif } diff --git a/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs b/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs index bb070388086ec..e76d5984bbc5b 100644 --- a/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs +++ b/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs @@ -2224,7 +2224,7 @@ public override BoundNode VisitWhileStatement(BoundWhileStatement node) public override BoundNode VisitWithExpression(BoundWithExpression expr) { - // PROTOTYPE: This is wrong + base.VisitWithExpression(expr); SetResultType(expr, expr.CloneMethod?.ReturnTypeWithAnnotations.ToTypeWithState() ?? TypeWithState.Create(expr.Type, NullableFlowState.NotNull)); diff --git a/src/Compilers/CSharp/Test/Semantic/Semantics/RecordTests.cs b/src/Compilers/CSharp/Test/Semantic/Semantics/RecordTests.cs index 16517d53049e4..f39c65c300d3e 100644 --- a/src/Compilers/CSharp/Test/Semantic/Semantics/RecordTests.cs +++ b/src/Compilers/CSharp/Test/Semantic/Semantics/RecordTests.cs @@ -1500,5 +1500,226 @@ public static void Main() Diagnostic(ErrorCode.ERR_EOFExpected, "}").WithLocation(11, 1) ); } + + public void WithExpr_DefiniteAssignment_01() + { + var src = @" +data class B(int X) +{ + static void M(B b) + { + int y; + _ = b with { X = y = 42 }; + y.ToString(); + } +}"; + var comp = CreateCompilation(src); + // PROTOTYPE: records don't auto-generate Clone at the moment + comp.VerifyDiagnostics( + // (7,13): error CS8858: The receiver type 'B' does not have an accessible parameterless instance method named "Clone". + // _ = b with { X = y = 42 }; + Diagnostic(ErrorCode.ERR_NoSingleCloneMethod, "b").WithArguments("B").WithLocation(7, 13)); + } + + [Fact] + public void WithExpr_DefiniteAssignment_02() + { + var src = @" +data class B(int X, string Y) +{ + static void M(B b) + { + int z; + _ = b with { X = z = 42, Y = z.ToString() }; + } +}"; + var comp = CreateCompilation(src); + // PROTOTYPE: records don't auto-generate Clone at the moment + comp.VerifyDiagnostics( + // (7,13): error CS8858: The receiver type 'B' does not have an accessible parameterless instance method named "Clone". + // _ = b with { X = y = 42 }; + Diagnostic(ErrorCode.ERR_NoSingleCloneMethod, "b").WithArguments("B").WithLocation(7, 13)); + } + + [Fact] + public void WithExpr_DefiniteAssignment_03() + { + var src = @" +data class B(int X, string Y) +{ + static void M(B b) + { + int z; + _ = b with { Y = z.ToString(), X = z = 42 }; + } +}"; + var comp = CreateCompilation(src); + // PROTOTYPE: records don't auto-generate Clone at the moment + comp.VerifyDiagnostics( + // (7,13): error CS8858: The receiver type 'B' does not have an accessible parameterless instance method named "Clone". + // _ = b with { Y = z.ToString(), X = z = 42 }; + Diagnostic(ErrorCode.ERR_NoSingleCloneMethod, "b").WithArguments("B").WithLocation(7, 13), + // (7,26): error CS0165: Use of unassigned local variable 'z' + // _ = b with { Y = z.ToString(), X = z = 42 }; + Diagnostic(ErrorCode.ERR_UseDefViolation, "z").WithArguments("z").WithLocation(7, 26)); + } + + [Fact] + public void WithExpr_DefiniteAssignment_04() + { + var src = @" +data class B(int X) +{ + static void M() + { + B b; + _ = (b = new B(42)) with { X = b.X }; + } +}"; + var comp = CreateCompilation(src); + // PROTOTYPE: records don't auto-generate Clone at the moment + comp.VerifyDiagnostics( + // (7,13): error CS8858: The receiver type 'B' does not have an accessible parameterless instance method named "Clone". + // _ = (b = new B(42)) with { X = b.X }; + Diagnostic(ErrorCode.ERR_NoSingleCloneMethod, "(b = new B(42))").WithArguments("B").WithLocation(7, 13)); + } + + [Fact] + public void WithExpr_DefiniteAssignment_05() + { + var src = @" +data class B(int X) +{ + static void M() + { + B b; + _ = new B(b.X) with { X = (b = new B(42)).X }; + } +}"; + var comp = CreateCompilation(src); + // PROTOTYPE: records don't auto-generate Clone at the moment + comp.VerifyDiagnostics( + // (7,13): error CS8858: The receiver type 'B' does not have an accessible parameterless instance method named "Clone". + // _ = new B(b.X) with { X = new B(42).X }; + Diagnostic(ErrorCode.ERR_NoSingleCloneMethod, "new B(b.X)").WithArguments("B").WithLocation(7, 13), + // (7,19): error CS0165: Use of unassigned local variable 'b' + // _ = new B(b.X) with { X = new B(42).X }; + Diagnostic(ErrorCode.ERR_UseDefViolation, "b").WithArguments("b").WithLocation(7, 19)); + } + + [Fact] + public void WithExpr_DefiniteAssignment_06() + { + var src = @" +data class B(int X) +{ + static void M(B b) + { + int y; + _ = b with { X = M(out y) }; + y.ToString(); + } + + static int M(out int y) { y = 42; return 43; } +}"; + var comp = CreateCompilation(src); + // PROTOTYPE: records don't auto-generate Clone at the moment + comp.VerifyDiagnostics( + // (7,13): error CS8858: The receiver type 'B' does not have an accessible parameterless instance method named "Clone". + // _ = b with { X = M(out y) }; + Diagnostic(ErrorCode.ERR_NoSingleCloneMethod, "b").WithArguments("B").WithLocation(7, 13)); + } + + [Fact] + public void WithExpr_DefiniteAssignment_07() + { + var src = @" +data class B(int X) +{ + static void M(B b) + { + _ = b with { X = M(out int y) }; + y.ToString(); + } + + static int M(out int y) { y = 42; return 43; } +}"; + var comp = CreateCompilation(src); + // PROTOTYPE: records don't auto-generate Clone at the moment + comp.VerifyDiagnostics( + // (6,13): error CS8858: The receiver type 'B' does not have an accessible parameterless instance method named "Clone". + // _ = b with { X = M(out int y) }; + Diagnostic(ErrorCode.ERR_NoSingleCloneMethod, "b").WithArguments("B").WithLocation(6, 13)); + } + + [Fact] + public void WithExpr_NullableAnalysis_01() + { + var src = @" +#nullable enable +data class B(int X) +{ + static void M(B b) + { + string? s = null; + _ = b with { X = M(out s) }; + s.ToString(); + } + + static int M(out string s) { s = ""a""; return 42; } +}"; + var comp = CreateCompilation(src); + // PROTOTYPE: records don't auto-generate Clone at the moment + comp.VerifyDiagnostics( + // (8,13): error CS8858: The receiver type 'B' does not have an accessible parameterless instance method named "Clone". + // _ = b with { X = M(out s) }; + Diagnostic(ErrorCode.ERR_NoSingleCloneMethod, "b").WithArguments("B").WithLocation(8, 13)); + } + + [Fact] + public void WithExpr_NullableAnalysis_02() + { + var src = @" +#nullable enable +data class B(string X) +{ + static void M(B b, string? s) + { + b.X.ToString(); + _ = b with { X = s }; // 1 + b.X.ToString(); // 2 + } +}"; + var comp = CreateCompilation(src); + // PROTOTYPE: we fail a Debug.Assert due to a conversion here + // PROTOTYPE: records don't auto-generate Clone at the moment + comp.VerifyDiagnostics( + // (8,13): error CS8858: The receiver type 'B' does not have an accessible parameterless instance method named "Clone". + // _ = b with { X = M(out s) }; + Diagnostic(ErrorCode.ERR_NoSingleCloneMethod, "b").WithArguments("B").WithLocation(8, 13)); + } + + [Fact] + public void WithExpr_NullableAnalysis_03() + { + var src = @" +#nullable enable +data class B(string? X) +{ + static void M(B b, string s) + { + b.X.ToString(); // 1 + _ = b with { X = s }; + b.X.ToString(); + } +}"; + var comp = CreateCompilation(src); + // PROTOTYPE: we fail a Debug.Assert due to a conversion here + // PROTOTYPE: records don't auto-generate Clone at the moment + comp.VerifyDiagnostics( + // (8,13): error CS8858: The receiver type 'B' does not have an accessible parameterless instance method named "Clone". + // _ = b with { X = M(out s) }; + Diagnostic(ErrorCode.ERR_NoSingleCloneMethod, "b").WithArguments("B").WithLocation(8, 13)); + } } } \ No newline at end of file From b3569ee25da2a0a087516fc78165e792d0a12652 Mon Sep 17 00:00:00 2001 From: Rikki Gibson Date: Wed, 27 May 2020 20:52:52 -0700 Subject: [PATCH 04/17] WIP --- .../Portable/Binder/Binder_WithExpression.cs | 5 ++- .../Portable/FlowAnalysis/NullableWalker.cs | 1 + .../Test/Semantic/Semantics/RecordTests.cs | 44 ++++++++++++++++--- 3 files changed, 44 insertions(+), 6 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Binder/Binder_WithExpression.cs b/src/Compilers/CSharp/Portable/Binder/Binder_WithExpression.cs index 430ce5356bb97..83ef1efd047e4 100644 --- a/src/Compilers/CSharp/Portable/Binder/Binder_WithExpression.cs +++ b/src/Compilers/CSharp/Portable/Binder/Binder_WithExpression.cs @@ -94,7 +94,10 @@ private BoundExpression BindWithExpression(WithExpressionSyntax syntax, Diagnost } } - var cloneReturnType = cloneMethod?.ReturnType; + // Even if a clone method is not present, we should come up with + // a type to lookup members of the with-expression in for error recovery + var cloneReturnType = cloneMethod?.ReturnType ?? receiverType; + RoslynDebug.AssertNotNull(cloneReturnType); var args = ArrayBuilder<(Symbol?, BoundExpression)>.GetInstance(); // Bind with expression arguments diff --git a/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs b/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs index e76d5984bbc5b..148ab31bba42f 100644 --- a/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs +++ b/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs @@ -2224,6 +2224,7 @@ public override BoundNode VisitWhileStatement(BoundWhileStatement node) public override BoundNode VisitWithExpression(BoundWithExpression expr) { + // TODO: assignments need to be checked for nullable compatibility base.VisitWithExpression(expr); SetResultType(expr, expr.CloneMethod?.ReturnTypeWithAnnotations.ToTypeWithState() diff --git a/src/Compilers/CSharp/Test/Semantic/Semantics/RecordTests.cs b/src/Compilers/CSharp/Test/Semantic/Semantics/RecordTests.cs index f39c65c300d3e..8215e867687d7 100644 --- a/src/Compilers/CSharp/Test/Semantic/Semantics/RecordTests.cs +++ b/src/Compilers/CSharp/Test/Semantic/Semantics/RecordTests.cs @@ -1706,20 +1706,54 @@ public void WithExpr_NullableAnalysis_03() #nullable enable data class B(string? X) { - static void M(B b, string s) + static void M1(B b, string s, bool flag) { - b.X.ToString(); // 1 + if (flag) { b.X.ToString(); } // 1 _ = b with { X = s }; - b.X.ToString(); + if (flag) { b.X.ToString(); } // 2 + } + + static void M2(B b, string s, bool flag) + { + if (flag) { b.X.ToString(); } // 1 + b = b with { X = s }; + if (flag) { b.X.ToString(); } } }"; var comp = CreateCompilation(src); - // PROTOTYPE: we fail a Debug.Assert due to a conversion here // PROTOTYPE: records don't auto-generate Clone at the moment comp.VerifyDiagnostics( + // (7,9): warning CS8602: Dereference of a possibly null reference. + // b.X.ToString(); // 1 + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "b.X").WithLocation(7, 9), // (8,13): error CS8858: The receiver type 'B' does not have an accessible parameterless instance method named "Clone". - // _ = b with { X = M(out s) }; + // _ = b with { X = s }; Diagnostic(ErrorCode.ERR_NoSingleCloneMethod, "b").WithArguments("B").WithLocation(8, 13)); } + + [Fact] + public void WithExpr_NullableAnalysis_04() + { + var src = @" +#nullable enable +data class B(int X) +{ + static void M(B? b) + { + _ = b with { X = 42 }; // 1 + _ = b.ToString(); + } +}"; + var comp = CreateCompilation(src); + // PROTOTYPE: with-expression should check receiver nullability + // PROTOTYPE: records don't auto-generate Clone at the moment + comp.VerifyDiagnostics( + // (7,13): error CS8858: The receiver type 'B' does not have an accessible parameterless instance method named "Clone". + // _ = b with { X = 42 }; // 1 + Diagnostic(ErrorCode.ERR_NoSingleCloneMethod, "b").WithArguments("B").WithLocation(7, 13), + // (8,13): warning CS8602: Dereference of a possibly null reference. + // _ = b.ToString(); + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "b").WithLocation(8, 13)); + } } } \ No newline at end of file From 64ebad90360158b74d00c9a3e047f51d5697fd9e Mon Sep 17 00:00:00 2001 From: Andy Gocke Date: Wed, 27 May 2020 22:03:52 -0700 Subject: [PATCH 05/17] Reuse object initializer binding + lowering --- .../Portable/Binder/Binder_Expressions.cs | 42 +++++---- .../Portable/Binder/Binder_WithExpression.cs | 54 ++---------- .../CSharp/Portable/BoundTree/BoundNodes.xml | 2 +- .../CSharp/Portable/CSharpResources.resx | 3 - .../CSharp/Portable/Errors/ErrorCode.cs | 1 - .../Generated/BoundNodes.xml.Generated.cs | 30 +++---- .../LocalRewriter_ObjectCreationExpression.cs | 40 +++++++-- .../LocalRewriter_WithExpression.cs | 86 ------------------- .../Portable/xlf/CSharpResources.cs.xlf | 5 -- .../Portable/xlf/CSharpResources.de.xlf | 5 -- .../Portable/xlf/CSharpResources.es.xlf | 5 -- .../Portable/xlf/CSharpResources.fr.xlf | 5 -- .../Portable/xlf/CSharpResources.it.xlf | 5 -- .../Portable/xlf/CSharpResources.ja.xlf | 5 -- .../Portable/xlf/CSharpResources.ko.xlf | 5 -- .../Portable/xlf/CSharpResources.pl.xlf | 5 -- .../Portable/xlf/CSharpResources.pt-BR.xlf | 5 -- .../Portable/xlf/CSharpResources.ru.xlf | 5 -- .../Portable/xlf/CSharpResources.tr.xlf | 5 -- .../Portable/xlf/CSharpResources.zh-Hans.xlf | 5 -- .../Portable/xlf/CSharpResources.zh-Hant.xlf | 5 -- .../Test/Semantic/Semantics/RecordTests.cs | 39 ++++++--- 22 files changed, 110 insertions(+), 252 deletions(-) delete mode 100644 src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_WithExpression.cs diff --git a/src/Compilers/CSharp/Portable/Binder/Binder_Expressions.cs b/src/Compilers/CSharp/Portable/Binder/Binder_Expressions.cs index a545b67913f04..230b4cbdbd64e 100644 --- a/src/Compilers/CSharp/Portable/Binder/Binder_Expressions.cs +++ b/src/Compilers/CSharp/Portable/Binder/Binder_Expressions.cs @@ -4298,7 +4298,13 @@ private BoundObjectInitializerExpressionBase BindInitializerExpression( switch (syntax.Kind()) { case SyntaxKind.ObjectInitializerExpression: - return BindObjectInitializerExpression(syntax, type, diagnostics, implicitReceiver); + // Uses a special binder to produce customized diagnostics for the object initializer + return BindObjectInitializerExpression( + syntax, type, diagnostics, implicitReceiver, useObjectInitDiagnostics: true); + + case SyntaxKind.WithInitializerExpression: + return BindObjectInitializerExpression( + syntax, type, diagnostics, implicitReceiver, useObjectInitDiagnostics: false); case SyntaxKind.CollectionInitializerExpression: return BindCollectionInitializerExpression(syntax, type, diagnostics, implicitReceiver); @@ -4331,7 +4337,8 @@ private BoundObjectInitializerExpression BindObjectInitializerExpression( InitializerExpressionSyntax initializerSyntax, TypeSymbol initializerType, DiagnosticBag diagnostics, - BoundObjectOrCollectionValuePlaceholder implicitReceiver) + BoundObjectOrCollectionValuePlaceholder implicitReceiver, + bool useObjectInitDiagnostics) { // SPEC: 7.6.10.2 Object initializers // @@ -4339,36 +4346,42 @@ private BoundObjectInitializerExpression BindObjectInitializerExpression( // SPEC: Each member initializer must name an accessible field or property of the object being initialized, followed by an equals sign and // SPEC: an expression or an object initializer or collection initializer. - Debug.Assert(initializerSyntax.Kind() == SyntaxKind.ObjectInitializerExpression); + Debug.Assert(initializerSyntax.Kind() == SyntaxKind.ObjectInitializerExpression || + initializerSyntax.Kind() == SyntaxKind.WithInitializerExpression); Debug.Assert((object)initializerType != null); - var initializerBuilder = ArrayBuilder.GetInstance(); - - // Member name map to report duplicate assignments to a field/property. - var memberNameMap = new HashSet(); - // We use a location specific binder for binding object initializer field/property access to generate object initializer specific diagnostics: // 1) CS1914 (ERR_StaticMemberInObjectInitializer) // 2) CS1917 (ERR_ReadonlyValueTypeInObjectInitializer) // 3) CS1918 (ERR_ValueTypePropertyInObjectInitializer) // Note that this is only used for the LHS of the assignment - these diagnostics do not apply on the RHS. // For this reason, we will actually need two binders: this and this.WithAdditionalFlags. - var objectInitializerMemberBinder = this.WithAdditionalFlags(BinderFlags.ObjectInitializerMember); + var objectInitializerMemberBinder = useObjectInitDiagnostics + ? this.WithAdditionalFlags(BinderFlags.ObjectInitializerMember) + : this; + + var initializers = ArrayBuilder.GetInstance(initializerSyntax.Expressions.Count); + // Member name map to report duplicate assignments to a field/property. + var memberNameMap = PooledHashSet.GetInstance(); foreach (var memberInitializer in initializerSyntax.Expressions) { - BoundExpression boundMemberInitializer = BindObjectInitializerMemberAssignment( + BoundExpression boundMemberInitializer = BindInitializerMemberAssignment( memberInitializer, initializerType, objectInitializerMemberBinder, diagnostics, implicitReceiver); - initializerBuilder.Add(boundMemberInitializer); + initializers.Add(boundMemberInitializer); ReportDuplicateObjectMemberInitializers(boundMemberInitializer, memberNameMap, diagnostics); } - return new BoundObjectInitializerExpression(initializerSyntax, implicitReceiver, initializerBuilder.ToImmutableAndFree(), initializerType); + return new BoundObjectInitializerExpression( + initializerSyntax, + implicitReceiver, + initializers.ToImmutableAndFree(), + initializerType); } - private BoundExpression BindObjectInitializerMemberAssignment( + private BoundExpression BindInitializerMemberAssignment( ExpressionSyntax memberInitializer, TypeSymbol initializerType, Binder objectInitializerMemberBinder, @@ -4402,7 +4415,6 @@ private BoundExpression BindObjectInitializerMemberAssignment( // See comments in BindObjectInitializerExpression for more details. Debug.Assert(objectInitializerMemberBinder != null); - Debug.Assert(objectInitializerMemberBinder.Flags.Includes(BinderFlags.ObjectInitializerMember)); boundLeft = objectInitializerMemberBinder.BindObjectInitializerMember(initializer, implicitReceiver, diagnostics); } @@ -4572,8 +4584,6 @@ private BoundExpression BindObjectInitializerMember( // CheckValueKind to generate possible diagnostics for invalid initializers non-viable member lookup result: // 1) CS0154 (ERR_PropertyLacksGet) // 2) CS0200 (ERR_AssgReadonlyProp) - - Debug.Assert(Flags.Includes(CSharp.BinderFlags.ObjectInitializerMember)); if (!CheckValueKind(boundMember.Syntax, boundMember, valueKind, checkingReceiver: false, diagnostics: diagnostics)) { hasErrors = true; diff --git a/src/Compilers/CSharp/Portable/Binder/Binder_WithExpression.cs b/src/Compilers/CSharp/Portable/Binder/Binder_WithExpression.cs index 8c44cb8362a93..b74a571140e26 100644 --- a/src/Compilers/CSharp/Portable/Binder/Binder_WithExpression.cs +++ b/src/Compilers/CSharp/Portable/Binder/Binder_WithExpression.cs @@ -94,60 +94,20 @@ private BoundExpression BindWithExpression(WithExpressionSyntax syntax, Diagnost } var cloneReturnType = cloneMethod?.ReturnType; - // PROTOTYPE: Handle dynamic - var implicitReceiver = new BoundObjectOrCollectionValuePlaceholder( + var initializer = BindInitializerExpression( + syntax.Initializer, + cloneReturnType ?? receiverType, syntax.Receiver, - // formatting - #pragma warning disable IDE0055 - cloneReturnType ?? receiverType) { WasCompilerGenerated = true }; - #pragma warning restore IDE0055 - - var args = ArrayBuilder.GetInstance(); - // Bind with expression arguments - foreach (var expr in syntax.Initializer.Expressions) - { - BoundExpression boundExpr; - // We're expecting a simple assignment only, with an ID on the left - if (!(expr is AssignmentExpressionSyntax assignment) || - !(assignment.Left is IdentifierNameSyntax left)) - { - boundExpr = BindExpression(expr, diagnostics); - hasErrors = true; - diagnostics.Add(ErrorCode.ERR_BadWithExpressionArgument, expr.Location); - } - else - { - var propName = left.Identifier.Text; - BoundExpression? boundMember = null; - boundMember = BindInstanceMemberAccess( - node: left, - right: left, - boundLeft: implicitReceiver, - rightName: propName, - rightArity: 0, - typeArgumentsSyntax: default(SeparatedSyntaxList), - typeArgumentsWithAnnotations: default(ImmutableArray), - invoked: false, - indexed: false, - diagnostics: diagnostics); - - hasErrors |= boundMember.HasAnyErrors || implicitReceiver.HasAnyErrors; - - boundMember = CheckValue(boundMember, BindValueKind.Assignable, diagnostics); - - var boundRight = BindValue(assignment.Right, diagnostics, BindValueKind.RValue); - boundExpr = BindAssignment(expr, boundMember, boundRight, isRef: false, diagnostics); - } - args.Add(boundExpr); - } + diagnostics); - lookupResult.Free(); + // N.B. Since we only don't parse nested initializers in syntax there should be no extra + // errors we need to check for here. return new BoundWithExpression( syntax, receiver, cloneMethod, - args.ToImmutableAndFree(), + initializer, cloneReturnType ?? receiverType, hasErrors: hasErrors); } diff --git a/src/Compilers/CSharp/Portable/BoundTree/BoundNodes.xml b/src/Compilers/CSharp/Portable/BoundTree/BoundNodes.xml index af0ba6f97d5a9..2f0a067b9b404 100644 --- a/src/Compilers/CSharp/Portable/BoundTree/BoundNodes.xml +++ b/src/Compilers/CSharp/Portable/BoundTree/BoundNodes.xml @@ -2131,7 +2131,7 @@ - + diff --git a/src/Compilers/CSharp/Portable/CSharpResources.resx b/src/Compilers/CSharp/Portable/CSharpResources.resx index 0a73fd5c21437..b8ff88ef1dcad 100644 --- a/src/Compilers/CSharp/Portable/CSharpResources.resx +++ b/src/Compilers/CSharp/Portable/CSharpResources.resx @@ -6130,9 +6130,6 @@ To remove the warning, you can use /reference instead (set the Embed Interop Typ 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. - - Arguments to a `with` expression must be simple assignments - A variable may not be declared within a 'not' or 'or' pattern. diff --git a/src/Compilers/CSharp/Portable/Errors/ErrorCode.cs b/src/Compilers/CSharp/Portable/Errors/ErrorCode.cs index 922f1746ef68a..1a2c020f2ff6c 100644 --- a/src/Compilers/CSharp/Portable/Errors/ErrorCode.cs +++ b/src/Compilers/CSharp/Portable/Errors/ErrorCode.cs @@ -1803,7 +1803,6 @@ internal enum ErrorCode ERR_NoSingleCloneMethod = 8858, ERR_ContainingTypeMustDeriveFromWithReturnType = 8859, ERR_WithMemberIsNotRecordProperty = 8860, - ERR_BadWithExpressionArgument = 8861, #endregion diff --git a/src/Compilers/CSharp/Portable/Generated/BoundNodes.xml.Generated.cs b/src/Compilers/CSharp/Portable/Generated/BoundNodes.xml.Generated.cs index 144b4548f74d2..b42aac9435709 100644 --- a/src/Compilers/CSharp/Portable/Generated/BoundNodes.xml.Generated.cs +++ b/src/Compilers/CSharp/Portable/Generated/BoundNodes.xml.Generated.cs @@ -7734,17 +7734,17 @@ public BoundExpressionWithNullability Update(BoundExpression expression, Nullabl internal sealed partial class BoundWithExpression : BoundExpression { - public BoundWithExpression(SyntaxNode syntax, BoundExpression receiver, MethodSymbol? cloneMethod, ImmutableArray arguments, TypeSymbol type, bool hasErrors = false) - : base(BoundKind.WithExpression, syntax, type, hasErrors || receiver.HasErrors() || arguments.HasErrors()) + public BoundWithExpression(SyntaxNode syntax, BoundExpression receiver, MethodSymbol? cloneMethod, BoundObjectInitializerExpressionBase initializerExpression, TypeSymbol type, bool hasErrors = false) + : base(BoundKind.WithExpression, syntax, type, hasErrors || receiver.HasErrors() || initializerExpression.HasErrors()) { RoslynDebug.Assert(receiver is object, "Field 'receiver' cannot be null (make the type nullable in BoundNodes.xml to remove this check)"); - RoslynDebug.Assert(!arguments.IsDefault, "Field 'arguments' cannot be null (use Null=\"allow\" in BoundNodes.xml to remove this check)"); + RoslynDebug.Assert(initializerExpression is object, "Field 'initializerExpression' cannot be null (make the type nullable in BoundNodes.xml to remove this check)"); RoslynDebug.Assert(type is object, "Field 'type' cannot be null (make the type nullable in BoundNodes.xml to remove this check)"); this.Receiver = receiver; this.CloneMethod = cloneMethod; - this.Arguments = arguments; + this.InitializerExpression = initializerExpression; } @@ -7754,15 +7754,15 @@ public BoundWithExpression(SyntaxNode syntax, BoundExpression receiver, MethodSy public MethodSymbol? CloneMethod { get; } - public ImmutableArray Arguments { get; } + public BoundObjectInitializerExpressionBase InitializerExpression { get; } [DebuggerStepThrough] public override BoundNode? Accept(BoundTreeVisitor visitor) => visitor.VisitWithExpression(this); - public BoundWithExpression Update(BoundExpression receiver, MethodSymbol? cloneMethod, ImmutableArray arguments, TypeSymbol type) + public BoundWithExpression Update(BoundExpression receiver, MethodSymbol? cloneMethod, BoundObjectInitializerExpressionBase initializerExpression, TypeSymbol type) { - if (receiver != this.Receiver || !Symbols.SymbolEqualityComparer.ConsiderEverything.Equals(cloneMethod, this.CloneMethod) || arguments != this.Arguments || !TypeSymbol.Equals(type, this.Type, TypeCompareKind.ConsiderEverything)) + if (receiver != this.Receiver || !Symbols.SymbolEqualityComparer.ConsiderEverything.Equals(cloneMethod, this.CloneMethod) || initializerExpression != this.InitializerExpression || !TypeSymbol.Equals(type, this.Type, TypeCompareKind.ConsiderEverything)) { - var result = new BoundWithExpression(this.Syntax, receiver, cloneMethod, arguments, type, this.HasErrors); + var result = new BoundWithExpression(this.Syntax, receiver, cloneMethod, initializerExpression, type, this.HasErrors); result.CopyAttributes(this); return result; } @@ -9473,7 +9473,7 @@ internal abstract partial class BoundTreeWalker: BoundTreeVisitor public override BoundNode? VisitWithExpression(BoundWithExpression node) { this.Visit(node.Receiver); - this.VisitList(node.Arguments); + this.Visit(node.InitializerExpression); return null; } } @@ -10639,9 +10639,9 @@ internal abstract partial class BoundTreeRewriter : BoundTreeVisitor public override BoundNode? VisitWithExpression(BoundWithExpression node) { BoundExpression receiver = (BoundExpression)this.Visit(node.Receiver); - ImmutableArray arguments = this.VisitList(node.Arguments); + BoundObjectInitializerExpressionBase initializerExpression = (BoundObjectInitializerExpressionBase)this.Visit(node.InitializerExpression); TypeSymbol? type = this.VisitType(node.Type); - return node.Update(receiver, node.CloneMethod, arguments, type); + return node.Update(receiver, node.CloneMethod, initializerExpression, type); } } @@ -12932,17 +12932,17 @@ public NullabilityRewriter(ImmutableDictionary arguments = this.VisitList(node.Arguments); + BoundObjectInitializerExpressionBase initializerExpression = (BoundObjectInitializerExpressionBase)this.Visit(node.InitializerExpression); BoundWithExpression updatedNode; if (_updatedNullabilities.TryGetValue(node, out (NullabilityInfo Info, TypeSymbol Type) infoAndType)) { - updatedNode = node.Update(receiver, cloneMethod, arguments, infoAndType.Type); + updatedNode = node.Update(receiver, cloneMethod, initializerExpression, infoAndType.Type); updatedNode.TopLevelNullability = infoAndType.Info; } else { - updatedNode = node.Update(receiver, cloneMethod, arguments, node.Type); + updatedNode = node.Update(receiver, cloneMethod, initializerExpression, node.Type); } return updatedNode; } @@ -14749,7 +14749,7 @@ private BoundTreeDumperNodeProducer() { new TreeDumperNode("receiver", null, new TreeDumperNode[] { Visit(node.Receiver, null) }), new TreeDumperNode("cloneMethod", node.CloneMethod, null), - new TreeDumperNode("arguments", null, from x in node.Arguments select Visit(x, null)), + new TreeDumperNode("initializerExpression", null, new TreeDumperNode[] { Visit(node.InitializerExpression, null) }), new TreeDumperNode("type", node.Type, null), new TreeDumperNode("isSuppressed", node.IsSuppressed, null), new TreeDumperNode("hasErrors", node.HasErrors, null) diff --git a/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_ObjectCreationExpression.cs b/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_ObjectCreationExpression.cs index c90a9b4eada4b..75398d85c2495 100644 --- a/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_ObjectCreationExpression.cs +++ b/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_ObjectCreationExpression.cs @@ -25,7 +25,7 @@ public override BoundNode VisitDynamicObjectCreationExpression(BoundDynamicObjec return constructorInvocation; } - return MakeObjectCreationWithInitializer(node.Syntax, constructorInvocation, node.InitializerExpressionOpt, node.Type); + return MakeExpressionWithInitializer(node.Syntax, constructorInvocation, node.InitializerExpressionOpt, node.Type); } public override BoundNode VisitObjectCreationExpression(BoundObjectCreationExpression node) @@ -101,7 +101,31 @@ public override BoundNode VisitObjectCreationExpression(BoundObjectCreationExpre return rewrittenObjectCreation; } - return MakeObjectCreationWithInitializer(node.Syntax, rewrittenObjectCreation, node.InitializerExpressionOpt, node.Type); + return MakeExpressionWithInitializer(node.Syntax, rewrittenObjectCreation, node.InitializerExpressionOpt, node.Type); + } + + public override BoundNode VisitWithExpression(BoundWithExpression withExpr) + { + RoslynDebug.AssertNotNull(withExpr.CloneMethod); + Debug.Assert(withExpr.CloneMethod.ParameterCount == 0); + + // 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 + // set the given record properties. i.e. + // + // var tmp = receiver.Clone(); + // tmp.P1 = e1; + // tmp.P2 = e2; + // tmp + + return MakeExpressionWithInitializer( + withExpr.Syntax, + _factory.InstanceCall(VisitExpression(withExpr.Receiver), "Clone"), + withExpr.InitializerExpression, + withExpr.Type); } [return: NotNullIfNotNull("initializerExpressionOpt")] @@ -117,10 +141,10 @@ public override BoundNode VisitObjectCreationExpression(BoundObjectCreationExpre return null; } - // Shared helper for MakeObjectCreationWithInitializer and MakeNewT - private BoundExpression MakeObjectCreationWithInitializer( + // Shared helper for VisitWithExpression, MakeObjectCreationWithInitializer and MakeNewT + private BoundExpression MakeExpressionWithInitializer( SyntaxNode syntax, - BoundExpression rewrittenObjectCreation, + BoundExpression rewrittenExpression, BoundExpression initializerExpression, TypeSymbol type) { @@ -129,7 +153,7 @@ private BoundExpression MakeObjectCreationWithInitializer( // Create a temp and assign it with the object creation expression. BoundAssignmentOperator boundAssignmentToTemp; - BoundLocal value = _factory.StoreToTemp(rewrittenObjectCreation, out boundAssignmentToTemp); + BoundLocal value = _factory.StoreToTemp(rewrittenExpression, out boundAssignmentToTemp); // Rewrite object/collection initializer expressions ArrayBuilder? dynamicSiteInitializers = null; @@ -183,7 +207,7 @@ public override BoundNode VisitNewT(BoundNewT node) return rewrittenNewT; } - return MakeObjectCreationWithInitializer(node.Syntax, rewrittenNewT, node.InitializerExpressionOpt, rewrittenNewT.Type!); + return MakeExpressionWithInitializer(node.Syntax, rewrittenNewT, node.InitializerExpressionOpt, rewrittenNewT.Type!); } private BoundExpression MakeNewT(SyntaxNode syntax, TypeParameterSymbol typeParameter) @@ -289,7 +313,7 @@ public override BoundNode VisitNoPiaObjectCreationExpression(BoundNoPiaObjectCre return rewrittenObjectCreation; } - return MakeObjectCreationWithInitializer(node.Syntax, rewrittenObjectCreation, node.InitializerExpressionOpt, node.Type); + return MakeExpressionWithInitializer(node.Syntax, rewrittenObjectCreation, node.InitializerExpressionOpt, node.Type); } } } diff --git a/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_WithExpression.cs b/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_WithExpression.cs deleted file mode 100644 index 238370f34d331..0000000000000 --- a/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_WithExpression.cs +++ /dev/null @@ -1,86 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. -// See the LICENSE file in the project root for more information. - -#nullable enable - -using System.Collections.Immutable; -using System.Diagnostics; -using Microsoft.CodeAnalysis.CSharp.Symbols; -using Microsoft.CodeAnalysis.PooledObjects; -using Roslyn.Utilities; - -namespace Microsoft.CodeAnalysis.CSharp -{ - internal sealed partial class LocalRewriter - { - public override BoundNode VisitWithExpression(BoundWithExpression withExpr) - { - RoslynDebug.AssertNotNull(withExpr.CloneMethod); - Debug.Assert(withExpr.CloneMethod.ParameterCount == 0); - - // 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 - // set the given record properties. i.e. - // - // var tmp = receiver.Clone(); - // tmp.P1 = e1; - // tmp.P2 = e2; - // tmp - var F = _factory; - var stores = ArrayBuilder.GetInstance(withExpr.Arguments.Length + 1); - - // var tmp = receiver.Clone(); - var receiverLocal = F.StoreToTemp( - F.InstanceCall(VisitExpression(withExpr.Receiver), "Clone"), - out var receiverStore); - stores.Add(receiverStore); - - // tmp.Pn = En; - foreach (var arg in withExpr.Arguments) - { - // If we got here without errors we should have a simple assignment with either a - // field or property on the left and a value expression on the right - var assignment = (BoundAssignmentOperator)arg; - // We need to construct the assignment LHS manually because swapping out the - // placeholder receiver could change the ref assignability of the expression, which - // the rest of lowering doesn't like - var left = assignment.Left.ExpressionSymbol switch - { - PropertySymbol p => MakePropertyAccess( - withExpr.Syntax, - receiverLocal, - p, - receiverLocal.ResultKind, - p.Type, - isLeftOfAssignment: true), - FieldSymbol f => MakeFieldAccess( - withExpr.Syntax, - receiverLocal, - f, - constantValueOpt: null, - receiverLocal.ResultKind, - f.Type), - _ => throw ExceptionUtilities.UnexpectedValue(assignment.Left.Kind) - }; - stores.Add(MakeStaticAssignmentOperator( - assignment.Syntax, - left, - VisitExpression(assignment.Right), - isRef: false, - left.Type!, - used: false)); - } - - return new BoundSequence( - withExpr.Syntax, - ImmutableArray.Create(receiverLocal.LocalSymbol), - stores.ToImmutableAndFree(), - receiverLocal, - withExpr.Type); - } - } -} \ No newline at end of file diff --git a/src/Compilers/CSharp/Portable/xlf/CSharpResources.cs.xlf b/src/Compilers/CSharp/Portable/xlf/CSharpResources.cs.xlf index 68a77cb4977ec..79eb95ca4f3aa 100644 --- a/src/Compilers/CSharp/Portable/xlf/CSharpResources.cs.xlf +++ b/src/Compilers/CSharp/Portable/xlf/CSharpResources.cs.xlf @@ -117,11 +117,6 @@ Chyba syntaxe příkazového řádku: {0} není platná hodnota možnosti {1}. Hodnota musí mít tvar {2}. - - Arguments to a `with` expression must be simple assignments - Arguments to a `with` expression must be simple assignments - - '{0}' must match by init-only of overridden member '{1}' '{0}' must match by init-only of overridden member '{1}' diff --git a/src/Compilers/CSharp/Portable/xlf/CSharpResources.de.xlf b/src/Compilers/CSharp/Portable/xlf/CSharpResources.de.xlf index 6a85fc1c26792..163ab4e5ad7db 100644 --- a/src/Compilers/CSharp/Portable/xlf/CSharpResources.de.xlf +++ b/src/Compilers/CSharp/Portable/xlf/CSharpResources.de.xlf @@ -117,11 +117,6 @@ Fehler in der Befehlszeilensyntax: "{0}" ist kein gültiger Wert für die Option "{1}". Der Wert muss im Format "{2}" vorliegen. - - Arguments to a `with` expression must be simple assignments - Arguments to a `with` expression must be simple assignments - - '{0}' must match by init-only of overridden member '{1}' '{0}' must match by init-only of overridden member '{1}' diff --git a/src/Compilers/CSharp/Portable/xlf/CSharpResources.es.xlf b/src/Compilers/CSharp/Portable/xlf/CSharpResources.es.xlf index c4f9226750a65..b23b86329e30f 100644 --- a/src/Compilers/CSharp/Portable/xlf/CSharpResources.es.xlf +++ b/src/Compilers/CSharp/Portable/xlf/CSharpResources.es.xlf @@ -117,11 +117,6 @@ Error de sintaxis de la línea de comandos: "{0}" no es un valor válido para la opción "{1}". El valor debe tener el formato "{2}". - - Arguments to a `with` expression must be simple assignments - Arguments to a `with` expression must be simple assignments - - '{0}' must match by init-only of overridden member '{1}' '{0}' must match by init-only of overridden member '{1}' diff --git a/src/Compilers/CSharp/Portable/xlf/CSharpResources.fr.xlf b/src/Compilers/CSharp/Portable/xlf/CSharpResources.fr.xlf index 43cd031fcbac5..5923b85bc54e7 100644 --- a/src/Compilers/CSharp/Portable/xlf/CSharpResources.fr.xlf +++ b/src/Compilers/CSharp/Portable/xlf/CSharpResources.fr.xlf @@ -117,11 +117,6 @@ Erreur de syntaxe de ligne de commande : '{0}' est une valeur non valide pour l'option '{1}'. La valeur doit se présenter sous la forme '{2}'. - - Arguments to a `with` expression must be simple assignments - Arguments to a `with` expression must be simple assignments - - '{0}' must match by init-only of overridden member '{1}' '{0}' must match by init-only of overridden member '{1}' diff --git a/src/Compilers/CSharp/Portable/xlf/CSharpResources.it.xlf b/src/Compilers/CSharp/Portable/xlf/CSharpResources.it.xlf index f484dcb66f513..f9e5a2ac6b335 100644 --- a/src/Compilers/CSharp/Portable/xlf/CSharpResources.it.xlf +++ b/src/Compilers/CSharp/Portable/xlf/CSharpResources.it.xlf @@ -117,11 +117,6 @@ Errore di sintassi della riga di comando: '{0}' non è un valore valido per l'opzione '{1}'. Il valore deve essere espresso nel formato '{2}'. - - Arguments to a `with` expression must be simple assignments - Arguments to a `with` expression must be simple assignments - - '{0}' must match by init-only of overridden member '{1}' '{0}' must match by init-only of overridden member '{1}' diff --git a/src/Compilers/CSharp/Portable/xlf/CSharpResources.ja.xlf b/src/Compilers/CSharp/Portable/xlf/CSharpResources.ja.xlf index 6dd05687c857f..7bad32af6aa7a 100644 --- a/src/Compilers/CSharp/Portable/xlf/CSharpResources.ja.xlf +++ b/src/Compilers/CSharp/Portable/xlf/CSharpResources.ja.xlf @@ -117,11 +117,6 @@ コマンドライン構文エラー: '{0}' は、'{1}' オプションの有効な値ではありません。値は '{2}' の形式にする必要があります。 - - Arguments to a `with` expression must be simple assignments - Arguments to a `with` expression must be simple assignments - - '{0}' must match by init-only of overridden member '{1}' '{0}' must match by init-only of overridden member '{1}' diff --git a/src/Compilers/CSharp/Portable/xlf/CSharpResources.ko.xlf b/src/Compilers/CSharp/Portable/xlf/CSharpResources.ko.xlf index 59dd4611400fe..d3978b4759eef 100644 --- a/src/Compilers/CSharp/Portable/xlf/CSharpResources.ko.xlf +++ b/src/Compilers/CSharp/Portable/xlf/CSharpResources.ko.xlf @@ -117,11 +117,6 @@ 명령줄 구문 오류: '{0}'은(는) '{1}' 옵션에 유효한 값이 아닙니다. 값은 '{2}' 형식이어야 합니다. - - Arguments to a `with` expression must be simple assignments - Arguments to a `with` expression must be simple assignments - - '{0}' must match by init-only of overridden member '{1}' '{0}' must match by init-only of overridden member '{1}' diff --git a/src/Compilers/CSharp/Portable/xlf/CSharpResources.pl.xlf b/src/Compilers/CSharp/Portable/xlf/CSharpResources.pl.xlf index e9535bb5da2a9..45a76d7d0dd32 100644 --- a/src/Compilers/CSharp/Portable/xlf/CSharpResources.pl.xlf +++ b/src/Compilers/CSharp/Portable/xlf/CSharpResources.pl.xlf @@ -117,11 +117,6 @@ Błąd składni wiersza polecenia: „{0}” nie jest prawidłową wartością dla opcji „{1}”. Wartość musi mieć postać „{2}”. - - Arguments to a `with` expression must be simple assignments - Arguments to a `with` expression must be simple assignments - - '{0}' must match by init-only of overridden member '{1}' '{0}' must match by init-only of overridden member '{1}' diff --git a/src/Compilers/CSharp/Portable/xlf/CSharpResources.pt-BR.xlf b/src/Compilers/CSharp/Portable/xlf/CSharpResources.pt-BR.xlf index 78b7ed3e5de66..759dbdff0882e 100644 --- a/src/Compilers/CSharp/Portable/xlf/CSharpResources.pt-BR.xlf +++ b/src/Compilers/CSharp/Portable/xlf/CSharpResources.pt-BR.xlf @@ -117,11 +117,6 @@ Erro de sintaxe de linha de comando: '{0}' não é um valor válido para a opção '{1}'. O valor precisa estar no formato '{2}'. - - Arguments to a `with` expression must be simple assignments - Arguments to a `with` expression must be simple assignments - - '{0}' must match by init-only of overridden member '{1}' '{0}' must match by init-only of overridden member '{1}' diff --git a/src/Compilers/CSharp/Portable/xlf/CSharpResources.ru.xlf b/src/Compilers/CSharp/Portable/xlf/CSharpResources.ru.xlf index 9f423d11ca893..c193947399c47 100644 --- a/src/Compilers/CSharp/Portable/xlf/CSharpResources.ru.xlf +++ b/src/Compilers/CSharp/Portable/xlf/CSharpResources.ru.xlf @@ -117,11 +117,6 @@ Ошибка в синтаксисе командной строки: "{0}" не является допустимым значением для параметра "{1}". Значение должно иметь форму "{2}". - - Arguments to a `with` expression must be simple assignments - Arguments to a `with` expression must be simple assignments - - '{0}' must match by init-only of overridden member '{1}' '{0}' must match by init-only of overridden member '{1}' diff --git a/src/Compilers/CSharp/Portable/xlf/CSharpResources.tr.xlf b/src/Compilers/CSharp/Portable/xlf/CSharpResources.tr.xlf index c5ecfe140264f..a16b0f13453e6 100644 --- a/src/Compilers/CSharp/Portable/xlf/CSharpResources.tr.xlf +++ b/src/Compilers/CSharp/Portable/xlf/CSharpResources.tr.xlf @@ -117,11 +117,6 @@ Komut satırı söz dizimi hatası: '{0}', '{1}' seçeneği için geçerli bir değer değil. Değer '{2}' biçiminde olmalıdır. - - Arguments to a `with` expression must be simple assignments - Arguments to a `with` expression must be simple assignments - - '{0}' must match by init-only of overridden member '{1}' '{0}' must match by init-only of overridden member '{1}' diff --git a/src/Compilers/CSharp/Portable/xlf/CSharpResources.zh-Hans.xlf b/src/Compilers/CSharp/Portable/xlf/CSharpResources.zh-Hans.xlf index 5d35ee58d6efb..82659ca002b9d 100644 --- a/src/Compilers/CSharp/Portable/xlf/CSharpResources.zh-Hans.xlf +++ b/src/Compilers/CSharp/Portable/xlf/CSharpResources.zh-Hans.xlf @@ -117,11 +117,6 @@ 命令行语法错误:“{0}”不是“{1}”选项的有效值。值的格式必须为 "{2}"。 - - Arguments to a `with` expression must be simple assignments - Arguments to a `with` expression must be simple assignments - - '{0}' must match by init-only of overridden member '{1}' '{0}' must match by init-only of overridden member '{1}' diff --git a/src/Compilers/CSharp/Portable/xlf/CSharpResources.zh-Hant.xlf b/src/Compilers/CSharp/Portable/xlf/CSharpResources.zh-Hant.xlf index 5a545f942c6dd..0223ca0ae8fbd 100644 --- a/src/Compilers/CSharp/Portable/xlf/CSharpResources.zh-Hant.xlf +++ b/src/Compilers/CSharp/Portable/xlf/CSharpResources.zh-Hant.xlf @@ -117,11 +117,6 @@ 命令列語法錯誤: '{0}' 對 '{1}' 選項而言不是有效的值。此值的格式必須是 '{2}'。 - - Arguments to a `with` expression must be simple assignments - Arguments to a `with` expression must be simple assignments - - '{0}' must match by init-only of overridden member '{1}' '{0}' must match by init-only of overridden member '{1}' diff --git a/src/Compilers/CSharp/Test/Semantic/Semantics/RecordTests.cs b/src/Compilers/CSharp/Test/Semantic/Semantics/RecordTests.cs index 1d3fbe50058e2..7b91b9b7a3ba1 100644 --- a/src/Compilers/CSharp/Test/Semantic/Semantics/RecordTests.cs +++ b/src/Compilers/CSharp/Test/Semantic/Semantics/RecordTests.cs @@ -948,9 +948,9 @@ public static void Main() // (12,22): error CS0572: 'X': cannot reference a type through an expression; try 'B.X' instead // b = b with { X = 0 }; Diagnostic(ErrorCode.ERR_BadTypeReference, "X").WithArguments("X", "B.X").WithLocation(12, 22), - // (12,22): error CS0118: 'B.X' is a type but is used like a variable + // (12,22): error CS1913: Member 'X' cannot be initialized. It is not a field or property. // b = b with { X = 0 }; - Diagnostic(ErrorCode.ERR_BadSKknown, "X").WithArguments("B.X", "type", "variable").WithLocation(12, 22) + Diagnostic(ErrorCode.ERR_MemberCannotBeInitialized, "X").WithArguments("X").WithLocation(12, 22) ); } @@ -994,9 +994,9 @@ public static void Main() }"; var comp = CreateCompilation(src); comp.VerifyDiagnostics( - // (12,22): error CS1061: 'B' does not contain a definition for 'Y' and no accessible extension method 'Y' accepting a first argument of type 'B' could be found (are you missing a using directive or an assembly reference?) + // (12,22): error CS0117: 'B' does not contain a definition for 'Y' // b = b with { Y = 2 }; - Diagnostic(ErrorCode.ERR_NoSuchMemberOrExtension, "Y").WithArguments("B", "Y").WithLocation(12, 22) + Diagnostic(ErrorCode.ERR_NoSuchMember, "Y").WithArguments("B", "Y").WithLocation(12, 22) ); } @@ -1381,9 +1381,9 @@ public static void Main() }"; var comp = CreateCompilation(src); comp.VerifyDiagnostics( - // (10,22): error CS1656: Cannot assign to 'X' because it is a 'method group' + // (10,22): error CS1913: Member 'X' cannot be initialized. It is not a field or property. // c = c with { X = 11 }; - Diagnostic(ErrorCode.ERR_AssgReadonlyLocalCause, "X").WithArguments("X", "method group").WithLocation(10, 22) + Diagnostic(ErrorCode.ERR_MemberCannotBeInitialized, "X").WithArguments("X").WithLocation(10, 22) ); } @@ -1437,9 +1437,9 @@ public static void Main() // (13,13): error CS0266: Cannot implicitly convert type 'B' to 'C'. An explicit conversion exists (are you missing a cast?) // c = c with { }; Diagnostic(ErrorCode.ERR_NoImplicitConvCast, "c with { }").WithArguments("B", "C").WithLocation(13, 13), - // (14,22): error CS1061: 'B' does not contain a definition for 'X' and no accessible extension method 'X' accepting a first argument of type 'B' could be found (are you missing a using directive or an assembly reference?) + // (14,22): error CS0117: 'B' does not contain a definition for 'X' // c = c with { X = 11 }; - Diagnostic(ErrorCode.ERR_NoSuchMemberOrExtension, "X").WithArguments("B", "X").WithLocation(14, 22) + Diagnostic(ErrorCode.ERR_NoSuchMember, "X").WithArguments("B", "X").WithLocation(14, 22) ); } @@ -1530,9 +1530,9 @@ public static void Main() var comp = CreateCompilation(src); comp.VerifyDiagnostics( - // (8,22): error CS8811: Arguments to a `with` expression must be simple assignments + // (8,22): error CS0747: Invalid initializer member declarator // c = c with { 5 }; - Diagnostic(ErrorCode.ERR_BadWithExpressionArgument, "5").WithLocation(8, 22), + Diagnostic(ErrorCode.ERR_InvalidInitializerElementInitializer, "5").WithLocation(8, 22), // (9,22): error CS1513: } expected // c = c with { { X = 2 } }; Diagnostic(ErrorCode.ERR_RbraceExpected, "{").WithLocation(9, 22), @@ -1740,5 +1740,24 @@ public static void Main() Diagnostic(ErrorCode.ERR_UnexpectedToken, "ref").WithArguments("ref").WithLocation(19, 26) ); } + + [Fact] + public void WithExpressionSameLHS() + { + var comp = CreateCompilation(@" +data class C(int X) +{ + public static void Main() + { + var c = new C(0); + c = c with { X = 1, X = 2}; + } +}"); + comp.VerifyDiagnostics( + // (7,29): error CS1912: Duplicate initialization of member 'X' + // c = c with { X = 1, X = 2}; + Diagnostic(ErrorCode.ERR_MemberAlreadyInitialized, "X").WithArguments("X").WithLocation(7, 29) + ); + } } } \ No newline at end of file From 1797b75208b5e103dddca9abbf3b5acabfae23b6 Mon Sep 17 00:00:00 2001 From: Rikki Gibson Date: Thu, 28 May 2020 12:44:25 -0700 Subject: [PATCH 06/17] Implement some nullable analysis of with-exprs --- .../Portable/FlowAnalysis/NullableWalker.cs | 107 ++++++---- .../Test/Semantic/Semantics/RecordTests.cs | 201 ++++++++++++++++-- 2 files changed, 250 insertions(+), 58 deletions(-) diff --git a/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs b/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs index 148ab31bba42f..5d05d25b1d80d 100644 --- a/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs +++ b/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs @@ -2222,13 +2222,44 @@ public override BoundNode VisitWhileStatement(BoundWhileStatement node) return base.VisitWhileStatement(node); } - public override BoundNode VisitWithExpression(BoundWithExpression expr) + public override BoundNode VisitWithExpression(BoundWithExpression withExpr) { - // TODO: assignments need to be checked for nullable compatibility - base.VisitWithExpression(expr); - SetResultType(expr, - expr.CloneMethod?.ReturnTypeWithAnnotations.ToTypeWithState() - ?? TypeWithState.Create(expr.Type, NullableFlowState.NotNull)); + Debug.Assert(!IsConditionalState); + + var receiver = withExpr.Receiver; + VisitRvalue(receiver); + _ = CheckPossibleNullReceiver(receiver, checkNullableValueType: false); + + var resultSlot = GetOrCreatePlaceholderSlot(withExpr); + // carry over the null state of members of 'receiver' to the result of the with-expression. + TrackNullableStateForAssignment(receiver, ResultType.ToTypeWithAnnotations(), resultSlot, ResultType, MakeSlot(receiver)); + + foreach (var (leftSymbol, right) in withExpr.Arguments) + { + if (leftSymbol is PropertySymbol prop) + { + // PROTOTYPE: attributes on positional record parameters do not propagate + // to the synthesized properties. Perhaps this is by design? + // If we want them to propagate then perhaps we should move + // members such as SourcePropertySymbol.HasAllowNull, etc. to SourceOrRecordPropertySymbol. + var leftAnnotations = GetPropertySetterAnnotations(prop); + var leftLValueType = ApplyLValueAnnotations(prop.TypeWithAnnotations, leftAnnotations); + + var rightState = VisitOptionalImplicitConversion(right, targetTypeOpt: leftLValueType, useLegacyWarnings: false, trackMembers: true, AssignmentKind.Assignment); + CheckDisallowedNullAssignment(rightState, leftAnnotations, right.Syntax.Location); + var propSlot = GetOrCreateSlot(prop, resultSlot); + TrackNullableStateForAssignment(right, leftLValueType, propSlot, rightState, MakeSlot(right)); + + // PROTOTYPE: Should we AdjustSetValue for properties? + // e.g. a property with [AllowNull, NotNull] would have a NotNull state after assigning a maybe-null. + // This isn't safe at runtime but it is consistent with what we allow users to do with auto properties. + // AdjustSetValue(left, declaredType, leftLValueType, ref rightState); + } + } + + SetResultType(withExpr, + withExpr.CloneMethod?.ReturnTypeWithAnnotations.ToTypeWithState() + ?? TypeWithState.Create(withExpr.Type, NullableFlowState.NotNull)); return null; } @@ -6982,6 +7013,34 @@ FlowAnalysisAnnotations getRValueAnnotations(BoundExpression expr) } } + private static FlowAnalysisAnnotations GetPropertySetterAnnotations(PropertySymbol property) + { + var accessor = property.GetOwnOrInheritedSetMethod(); + if (accessor is object) + { + return accessor.Parameters.Last().FlowAnalysisAnnotations; + } + if (property is SourcePropertySymbol sourceProperty) + { + return getPropertyAnnotations(sourceProperty); + } + return FlowAnalysisAnnotations.None; + + static FlowAnalysisAnnotations getPropertyAnnotations(SourcePropertySymbol property) + { + var annotations = FlowAnalysisAnnotations.None; + if (property.HasAllowNull) + { + annotations |= FlowAnalysisAnnotations.AllowNull; + } + if (property.HasDisallowNull) + { + annotations |= FlowAnalysisAnnotations.DisallowNull; + } + return annotations; + } + } + private FlowAnalysisAnnotations GetLValueAnnotations(BoundExpression expr) { // Annotations are ignored when binding an attribute to avoid cycles. (Members used @@ -6993,10 +7052,10 @@ private FlowAnalysisAnnotations GetLValueAnnotations(BoundExpression expr) var annotations = expr switch { - BoundPropertyAccess property => getSetterAnnotations(property.PropertySymbol), - BoundIndexerAccess indexer => getSetterAnnotations(indexer.Indexer), + BoundPropertyAccess property => GetPropertySetterAnnotations(property.PropertySymbol), + BoundIndexerAccess indexer => GetPropertySetterAnnotations(indexer.Indexer), BoundFieldAccess field => getFieldAnnotations(field.FieldSymbol), - BoundObjectInitializerMember { MemberSymbol: PropertySymbol prop } => getSetterAnnotations(prop), + BoundObjectInitializerMember { MemberSymbol: PropertySymbol prop } => GetPropertySetterAnnotations(prop), BoundObjectInitializerMember { MemberSymbol: FieldSymbol field } => getFieldAnnotations(field), BoundParameter { ParameterSymbol: ParameterSymbol parameter } => ToInwardAnnotations(GetParameterAnnotations(parameter) & ~FlowAnalysisAnnotations.NotNull), // NotNull is enforced upon method exit @@ -7008,37 +7067,9 @@ private FlowAnalysisAnnotations GetLValueAnnotations(BoundExpression expr) static FlowAnalysisAnnotations getFieldAnnotations(FieldSymbol field) { return field.AssociatedSymbol is PropertySymbol property ? - getSetterAnnotations(property) : + GetPropertySetterAnnotations(property) : field.FlowAnalysisAnnotations; } - - static FlowAnalysisAnnotations getSetterAnnotations(PropertySymbol property) - { - var accessor = property.GetOwnOrInheritedSetMethod(); - if (accessor is object) - { - return accessor.Parameters.Last().FlowAnalysisAnnotations; - } - if (property is SourcePropertySymbol sourceProperty) - { - return getPropertyAnnotations(sourceProperty); - } - return FlowAnalysisAnnotations.None; - } - - static FlowAnalysisAnnotations getPropertyAnnotations(SourcePropertySymbol property) - { - var annotations = FlowAnalysisAnnotations.None; - if (property.HasAllowNull) - { - annotations |= FlowAnalysisAnnotations.AllowNull; - } - if (property.HasDisallowNull) - { - annotations |= FlowAnalysisAnnotations.DisallowNull; - } - return annotations; - } } private static FlowAnalysisAnnotations ToInwardAnnotations(FlowAnalysisAnnotations outwardAnnotations) diff --git a/src/Compilers/CSharp/Test/Semantic/Semantics/RecordTests.cs b/src/Compilers/CSharp/Test/Semantic/Semantics/RecordTests.cs index 8215e867687d7..b9249b829dc87 100644 --- a/src/Compilers/CSharp/Test/Semantic/Semantics/RecordTests.cs +++ b/src/Compilers/CSharp/Test/Semantic/Semantics/RecordTests.cs @@ -1691,12 +1691,14 @@ static void M(B b, string? s) } }"; var comp = CreateCompilation(src); - // PROTOTYPE: we fail a Debug.Assert due to a conversion here // PROTOTYPE: records don't auto-generate Clone at the moment comp.VerifyDiagnostics( // (8,13): error CS8858: The receiver type 'B' does not have an accessible parameterless instance method named "Clone". - // _ = b with { X = M(out s) }; - Diagnostic(ErrorCode.ERR_NoSingleCloneMethod, "b").WithArguments("B").WithLocation(8, 13)); + // _ = b with { X = s }; // 1 + Diagnostic(ErrorCode.ERR_NoSingleCloneMethod, "b").WithArguments("B").WithLocation(8, 13), + // (8,26): warning CS8601: Possible null reference assignment. + // _ = b with { X = s }; // 1 + Diagnostic(ErrorCode.WRN_NullReferenceAssignment, "s").WithLocation(8, 26)); } [Fact] @@ -1706,6 +1708,8 @@ public void WithExpr_NullableAnalysis_03() #nullable enable data class B(string? X) { + public B Clone() => new B(X); + static void M1(B b, string s, bool flag) { if (flag) { b.X.ToString(); } // 1 @@ -1715,20 +1719,22 @@ static void M1(B b, string s, bool flag) static void M2(B b, string s, bool flag) { - if (flag) { b.X.ToString(); } // 1 + if (flag) { b.X.ToString(); } // 3 b = b with { X = s }; if (flag) { b.X.ToString(); } } }"; var comp = CreateCompilation(src); - // PROTOTYPE: records don't auto-generate Clone at the moment comp.VerifyDiagnostics( - // (7,9): warning CS8602: Dereference of a possibly null reference. - // b.X.ToString(); // 1 - Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "b.X").WithLocation(7, 9), - // (8,13): error CS8858: The receiver type 'B' does not have an accessible parameterless instance method named "Clone". - // _ = b with { X = s }; - Diagnostic(ErrorCode.ERR_NoSingleCloneMethod, "b").WithArguments("B").WithLocation(8, 13)); + // (9,21): warning CS8602: Dereference of a possibly null reference. + // if (flag) { b.X.ToString(); } // 1 + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "b.X").WithLocation(9, 21), + // (11,21): warning CS8602: Dereference of a possibly null reference. + // if (flag) { b.X.ToString(); } // 2 + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "b.X").WithLocation(11, 21), + // (16,21): warning CS8602: Dereference of a possibly null reference. + // if (flag) { b.X.ToString(); } // 3 + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "b.X").WithLocation(16, 21)); } [Fact] @@ -1738,22 +1744,177 @@ public void WithExpr_NullableAnalysis_04() #nullable enable data class B(int X) { - static void M(B? b) + static void M1(B? b) { - _ = b with { X = 42 }; // 1 + var b1 = b with { X = 42 }; // 1 _ = b.ToString(); + _ = b1.ToString(); + } + + static void M2(B? b) + { + (b with { X = 42 }).ToString(); // 2 } }"; var comp = CreateCompilation(src); - // PROTOTYPE: with-expression should check receiver nullability // PROTOTYPE: records don't auto-generate Clone at the moment comp.VerifyDiagnostics( - // (7,13): error CS8858: The receiver type 'B' does not have an accessible parameterless instance method named "Clone". - // _ = b with { X = 42 }; // 1 - Diagnostic(ErrorCode.ERR_NoSingleCloneMethod, "b").WithArguments("B").WithLocation(7, 13), - // (8,13): warning CS8602: Dereference of a possibly null reference. - // _ = b.ToString(); - Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "b").WithLocation(8, 13)); + // (7,18): error CS8858: The receiver type 'B' does not have an accessible parameterless instance method named "Clone". + // var b1 = b with { X = 42 }; // 1 + Diagnostic(ErrorCode.ERR_NoSingleCloneMethod, "b").WithArguments("B").WithLocation(7, 18), + // (7,18): warning CS8602: Dereference of a possibly null reference. + // var b1 = b with { X = 42 }; // 1 + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "b").WithLocation(7, 18), + // (14,10): error CS8858: The receiver type 'B' does not have an accessible parameterless instance method named "Clone". + // (b with { X = 42 }).ToString(); // 2 + Diagnostic(ErrorCode.ERR_NoSingleCloneMethod, "b").WithArguments("B").WithLocation(14, 10), + // (14,10): warning CS8602: Dereference of a possibly null reference. + // (b with { X = 42 }).ToString(); // 2 + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "b").WithLocation(14, 10)); + } + + [Fact] + public void WithExpr_NullableAnalysis_05() + { + var src = @" +#nullable enable +data class B(string? X, string? Y) +{ + public B Clone() => new B(X, Y); + + static void M1(bool flag) + { + B b = new B(""hello"", null); + if (flag) + { + b.X.ToString(); // PROTOTYPE(records) + b.Y.ToString(); // 1 + } + + b = b with { Y = ""world"" }; + b.X.ToString(); // PROTOTYPE(records) + b.Y.ToString(); + } +}"; + // PROTOTYPE: it feels like records should propagate the nullability of + // the constructor arguments to the corresponding properties. + var comp = CreateCompilation(src); + comp.VerifyDiagnostics( + // (12,13): warning CS8602: Dereference of a possibly null reference. + // b.X.ToString(); // PROTOTYPE(records) + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "b.X").WithLocation(12, 13), + // (13,13): warning CS8602: Dereference of a possibly null reference. + // b.Y.ToString(); // 1 + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "b.Y").WithLocation(13, 13), + // (17,9): warning CS8602: Dereference of a possibly null reference. + // b.X.ToString(); // PROTOTYPE(records) + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "b.X").WithLocation(17, 9)); + } + + [Fact(Skip = "PROTOTYPE(records): with expression using init accessors")] + public void WithExpr_NullableAnalysis_06() + { + var src = @" +#nullable enable +data class B +{ + public string? X { get; init; } + public string? Y { get; init; } + + public B Clone() => new B { X = X, Y = Y }; + + static void M1(bool flag) + { + B b = new B { X = ""hello"", Y = null }; + if (flag) + { + b.X.ToString(); + b.Y.ToString(); // 1 + } + + b = b with { Y = ""world"" }; + + b.X.ToString(); + b.Y.ToString(); + } +}"; + var comp = CreateCompilation(src); + comp.VerifyDiagnostics( + // (9,21): warning CS8602: Dereference of a possibly null reference. + // if (flag) { b.X.ToString(); } // 1 + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "b.X").WithLocation(9, 21), + // (11,21): warning CS8602: Dereference of a possibly null reference. + // if (flag) { b.X.ToString(); } // 2 + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "b.X").WithLocation(11, 21), + // (16,21): warning CS8602: Dereference of a possibly null reference. + // if (flag) { b.X.ToString(); } // 3 + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "b.X").WithLocation(16, 21)); + } + + [Fact(Skip = "PROTOTYPE(records): positional property attributes")] + public void WithExpr_NullableAnalysis_07() + { + var src = @" +#nullable enable +using System.Diagnostics.CodeAnalysis; + +data class B([AllowNull] string X) +{ + public B Clone() => new B(X); + + static void M1(B b) + { + b.X.ToString(); + b = b with { X = null }; + b.X.ToString(); // 1 + } +}"; + var comp = CreateCompilation(new[] { src, AllowNullAttributeDefinition }); + comp.VerifyDiagnostics( + // (13,9): warning CS8602: Dereference of a possibly null reference. + // b.X.ToString(); // 1 + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "b.X").WithLocation(13, 9)); + } + + [Fact] + public void WithExpr_NullableAnalysis_08() + { + var src = @" +#nullable enable +data class B(string? X, string? Y) +{ + public B Clone() => new B(X, Y); + + static void M1(B b1) + { + B b2 = b1 with { X = ""hello"" }; + B b3 = b1 with { Y = ""world"" }; + B b4 = b2 with { Y = ""world"" }; + + b1.X.ToString(); // 1 + b1.Y.ToString(); // 2 + b2.X.ToString(); + b2.Y.ToString(); // 3 + b3.X.ToString(); // 4 + b3.Y.ToString(); + b4.X.ToString(); + b4.Y.ToString(); + } +}"; + var comp = CreateCompilation(src); + comp.VerifyDiagnostics( + // (13,9): warning CS8602: Dereference of a possibly null reference. + // b1.X.ToString(); // 1 + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "b1.X").WithLocation(13, 9), + // (14,9): warning CS8602: Dereference of a possibly null reference. + // b1.Y.ToString(); // 2 + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "b1.Y").WithLocation(14, 9), + // (16,9): warning CS8602: Dereference of a possibly null reference. + // b2.Y.ToString(); // 3 + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "b2.Y").WithLocation(16, 9), + // (17,9): warning CS8602: Dereference of a possibly null reference. + // b3.X.ToString(); // 4 + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "b3.X").WithLocation(17, 9)); } } } \ No newline at end of file From a8536ceaa8fa7b6469988e93d8f9d1c6f84ffb74 Mon Sep 17 00:00:00 2001 From: Rikki Gibson Date: Thu, 28 May 2020 13:25:09 -0700 Subject: [PATCH 07/17] Reduce diff churn --- .../Portable/FlowAnalysis/NullableWalker.cs | 56 +++++++++---------- 1 file changed, 28 insertions(+), 28 deletions(-) diff --git a/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs b/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs index 5d05d25b1d80d..9a15fd1ae33d5 100644 --- a/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs +++ b/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs @@ -7013,34 +7013,6 @@ FlowAnalysisAnnotations getRValueAnnotations(BoundExpression expr) } } - private static FlowAnalysisAnnotations GetPropertySetterAnnotations(PropertySymbol property) - { - var accessor = property.GetOwnOrInheritedSetMethod(); - if (accessor is object) - { - return accessor.Parameters.Last().FlowAnalysisAnnotations; - } - if (property is SourcePropertySymbol sourceProperty) - { - return getPropertyAnnotations(sourceProperty); - } - return FlowAnalysisAnnotations.None; - - static FlowAnalysisAnnotations getPropertyAnnotations(SourcePropertySymbol property) - { - var annotations = FlowAnalysisAnnotations.None; - if (property.HasAllowNull) - { - annotations |= FlowAnalysisAnnotations.AllowNull; - } - if (property.HasDisallowNull) - { - annotations |= FlowAnalysisAnnotations.DisallowNull; - } - return annotations; - } - } - private FlowAnalysisAnnotations GetLValueAnnotations(BoundExpression expr) { // Annotations are ignored when binding an attribute to avoid cycles. (Members used @@ -7072,6 +7044,34 @@ static FlowAnalysisAnnotations getFieldAnnotations(FieldSymbol field) } } + private static FlowAnalysisAnnotations GetPropertySetterAnnotations(PropertySymbol property) + { + var accessor = property.GetOwnOrInheritedSetMethod(); + if (accessor is object) + { + return accessor.Parameters.Last().FlowAnalysisAnnotations; + } + if (property is SourcePropertySymbol sourceProperty) + { + return getPropertyAnnotations(sourceProperty); + } + return FlowAnalysisAnnotations.None; + + static FlowAnalysisAnnotations getPropertyAnnotations(SourcePropertySymbol property) + { + var annotations = FlowAnalysisAnnotations.None; + if (property.HasAllowNull) + { + annotations |= FlowAnalysisAnnotations.AllowNull; + } + if (property.HasDisallowNull) + { + annotations |= FlowAnalysisAnnotations.DisallowNull; + } + return annotations; + } + } + private static FlowAnalysisAnnotations ToInwardAnnotations(FlowAnalysisAnnotations outwardAnnotations) { var annotations = FlowAnalysisAnnotations.None; From a8471fe6174391d3d5ce765aa103eaef8118b61c Mon Sep 17 00:00:00 2001 From: Andy Gocke Date: Thu, 28 May 2020 14:21:22 -0700 Subject: [PATCH 08/17] Respond to PR comments --- .../LocalRewriter_ObjectCreationExpression.cs | 6 +++++- .../Synthesized/Records/SynthesizedRecordClone.cs | 12 ++++++------ .../Synthesized/Records/SynthesizedRecordCopyCtor.cs | 4 ++-- .../Test/Semantic/Semantics/InitOnlyMemberTests.cs | 7 +++---- src/Test/Utilities/Portable/Assert/AssertEx.cs | 3 +++ 5 files changed, 19 insertions(+), 13 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_ObjectCreationExpression.cs b/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_ObjectCreationExpression.cs index 75398d85c2495..7ee0865748fd9 100644 --- a/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_ObjectCreationExpression.cs +++ b/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_ObjectCreationExpression.cs @@ -121,9 +121,13 @@ public override BoundNode VisitWithExpression(BoundWithExpression withExpr) // tmp.P2 = e2; // tmp + var cloneCall = _factory.Call( + VisitExpression(withExpr.Receiver), + withExpr.CloneMethod); + return MakeExpressionWithInitializer( withExpr.Syntax, - _factory.InstanceCall(VisitExpression(withExpr.Receiver), "Clone"), + cloneCall, withExpr.InitializerExpression, withExpr.Type); } diff --git a/src/Compilers/CSharp/Portable/Symbols/Synthesized/Records/SynthesizedRecordClone.cs b/src/Compilers/CSharp/Portable/Symbols/Synthesized/Records/SynthesizedRecordClone.cs index 3880c0e26096b..f99edf8dabdc7 100644 --- a/src/Compilers/CSharp/Portable/Symbols/Synthesized/Records/SynthesizedRecordClone.cs +++ b/src/Compilers/CSharp/Portable/Symbols/Synthesized/Records/SynthesizedRecordClone.cs @@ -70,6 +70,7 @@ public override ImmutableArray TypeArgumentsWithAnnotations public override bool IsStatic => false; + // PROTOTYPE: Inheritance is not handled public override bool IsVirtual => true; public override bool IsOverride => false; @@ -82,8 +83,6 @@ public override ImmutableArray TypeArgumentsWithAnnotations internal override bool HasSpecialName => true; - internal override LexicalSortKey GetLexicalSortKey() => LexicalSortKey.SynthesizedRecordEquals; - internal override MethodImplAttributes ImplementationAttributes => MethodImplAttributes.Managed; internal override bool HasDeclarativeSecurity => false; @@ -114,21 +113,22 @@ internal override void GenerateMethodBody(TypeCompilationState compilationState, { var F = new SyntheticBoundNodeFactory(this, ContainingType.GetNonNullSyntaxNode(), compilationState, diagnostics); - // If this is a class, call the copy ctor. Otherwise, just return `this + // If this is a class, call the copy ctor. Otherwise, just return `this` if (ContainingType.IsStructType()) { - F.CloseMethod(F.Block(F.Return(F.This()))); + F.CloseMethod(F.Return(F.This())); return; } - var members = ContainingType.GetMembers(".ctor"); + // PROTOTYPE: what about base fields? + var members = ContainingType.GetMembers(WellKnownMemberNames.InstanceConstructorName); foreach (var member in members) { var ctor = (MethodSymbol)member; if (ctor.ParameterCount == 1 && ctor.Parameters[0].Type.Equals(ContainingType, TypeCompareKind.ConsiderEverything)) { - F.CloseMethod(F.Block(F.Return(F.New(ctor, F.This())))); + F.CloseMethod(F.Return(F.New(ctor, F.This()))); return; } } diff --git a/src/Compilers/CSharp/Portable/Symbols/Synthesized/Records/SynthesizedRecordCopyCtor.cs b/src/Compilers/CSharp/Portable/Symbols/Synthesized/Records/SynthesizedRecordCopyCtor.cs index b6a5ce7c7d72a..e2e5555bf3fb5 100644 --- a/src/Compilers/CSharp/Portable/Symbols/Synthesized/Records/SynthesizedRecordCopyCtor.cs +++ b/src/Compilers/CSharp/Portable/Symbols/Synthesized/Records/SynthesizedRecordCopyCtor.cs @@ -42,9 +42,9 @@ internal override void GenerateMethodBodyStatements(SyntheticBoundNodeFactory F, // Write assignments to backing fields // // { - // this.backingField1 = arg1 + // this.backingField1 = parameter.backingField1 // ... - // this.backingFieldN = argN + // this.backingFieldN = parameter.backingFieldN // } var param = F.Parameter(Parameters[0]); foreach (var member in ContainingType.GetMembers()) diff --git a/src/Compilers/CSharp/Test/Semantic/Semantics/InitOnlyMemberTests.cs b/src/Compilers/CSharp/Test/Semantic/Semantics/InitOnlyMemberTests.cs index 7784fc171731f..babfae1022098 100644 --- a/src/Compilers/CSharp/Test/Semantic/Semantics/InitOnlyMemberTests.cs +++ b/src/Compilers/CSharp/Test/Semantic/Semantics/InitOnlyMemberTests.cs @@ -2171,19 +2171,18 @@ void M() { } comp.VerifyDiagnostics(); var cMembers = comp.GlobalNamespace.GetMember("C").GetMembers(); - // PROTOTYPE(init-only): expecting an 'init' setter - AssertEx.SetEqual(cMembers.ToTestDisplayStrings(), new[] { + AssertEx.SetEqual(new[] { + "C C.Clone()", "System.Int32 C.k__BackingField", "System.Int32 C.i.get", "void modreq(System.Runtime.CompilerServices.IsExternalInit) C.i.init", "System.Int32 C.i { get; init; }", "void C.M()", - "C C.Clone()", "System.Boolean C.Equals(C? )", "System.Boolean C.Equals(System.Object? )", "System.Int32 C.GetHashCode()", "C..ctor(C )", - "C..ctor(System.Int32 i)" }); + "C..ctor(System.Int32 i)" }, cMembers.ToTestDisplayStrings()); foreach (var member in cMembers) { diff --git a/src/Test/Utilities/Portable/Assert/AssertEx.cs b/src/Test/Utilities/Portable/Assert/AssertEx.cs index 4fb8980b75563..12d85655970b8 100644 --- a/src/Test/Utilities/Portable/Assert/AssertEx.cs +++ b/src/Test/Utilities/Portable/Assert/AssertEx.cs @@ -441,6 +441,9 @@ public static void SetEqual(IEnumerable expected, IEnumerable actual, I } } + public static void SetEqual(T[] expected, T[] actual) + => SetEqual((IEnumerable)actual, expected); + public static void SetEqual(IEnumerable actual, params T[] expected) { var expectedSet = new HashSet(expected); From 9f605e2b70ee9a6ea1e41ebbf582c75956ce89c0 Mon Sep 17 00:00:00 2001 From: Andy Gocke Date: Thu, 28 May 2020 23:49:22 -0700 Subject: [PATCH 09/17] Respond to PR comments --- .../Portable/Binder/Binder_Expressions.cs | 1 + .../Source/SourceMemberContainerSymbol.cs | 5 +- .../Records/SynthesizedRecordClone.cs | 7 -- .../Records/SynthesizedRecordCopyCtor.cs | 15 +-- .../Test/Symbol/Symbols/Source/RecordTests.cs | 117 ++++++++++++++++++ 5 files changed, 125 insertions(+), 20 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Binder/Binder_Expressions.cs b/src/Compilers/CSharp/Portable/Binder/Binder_Expressions.cs index 230b4cbdbd64e..cf0ff3930aaf1 100644 --- a/src/Compilers/CSharp/Portable/Binder/Binder_Expressions.cs +++ b/src/Compilers/CSharp/Portable/Binder/Binder_Expressions.cs @@ -4327,6 +4327,7 @@ private BoundExpression BindInitializerExpressionOrValue( { case SyntaxKind.ObjectInitializerExpression: case SyntaxKind.CollectionInitializerExpression: + Debug.Assert(syntax.Parent.Parent.Kind() != SyntaxKind.WithInitializerExpression); return BindInitializerExpression((InitializerExpressionSyntax)syntax, type, typeSyntax, diagnostics); default: return BindValue(syntax, diagnostics, BindValueKind.RValue); diff --git a/src/Compilers/CSharp/Portable/Symbols/Source/SourceMemberContainerSymbol.cs b/src/Compilers/CSharp/Portable/Symbols/Source/SourceMemberContainerSymbol.cs index e604285f7b697..d556fe26138e2 100644 --- a/src/Compilers/CSharp/Portable/Symbols/Source/SourceMemberContainerSymbol.cs +++ b/src/Compilers/CSharp/Portable/Symbols/Source/SourceMemberContainerSymbol.cs @@ -2954,10 +2954,7 @@ private void AddSynthesizedRecordMembersIfNecessary(MembersAndInitializersBuilde } var ctor = addCtor(paramList); - if (!this.IsStructType()) - { - addCopyCtor(); - } + addCopyCtor(); addCloneMethod(); addProperties(ctor.Parameters); var thisEquals = addThisEquals(); diff --git a/src/Compilers/CSharp/Portable/Symbols/Synthesized/Records/SynthesizedRecordClone.cs b/src/Compilers/CSharp/Portable/Symbols/Synthesized/Records/SynthesizedRecordClone.cs index f99edf8dabdc7..0b6132705eb34 100644 --- a/src/Compilers/CSharp/Portable/Symbols/Synthesized/Records/SynthesizedRecordClone.cs +++ b/src/Compilers/CSharp/Portable/Symbols/Synthesized/Records/SynthesizedRecordClone.cs @@ -113,13 +113,6 @@ internal override void GenerateMethodBody(TypeCompilationState compilationState, { var F = new SyntheticBoundNodeFactory(this, ContainingType.GetNonNullSyntaxNode(), compilationState, diagnostics); - // If this is a class, call the copy ctor. Otherwise, just return `this` - if (ContainingType.IsStructType()) - { - F.CloseMethod(F.Return(F.This())); - return; - } - // PROTOTYPE: what about base fields? var members = ContainingType.GetMembers(WellKnownMemberNames.InstanceConstructorName); foreach (var member in members) diff --git a/src/Compilers/CSharp/Portable/Symbols/Synthesized/Records/SynthesizedRecordCopyCtor.cs b/src/Compilers/CSharp/Portable/Symbols/Synthesized/Records/SynthesizedRecordCopyCtor.cs index e2e5555bf3fb5..b0a7adbac98af 100644 --- a/src/Compilers/CSharp/Portable/Symbols/Synthesized/Records/SynthesizedRecordCopyCtor.cs +++ b/src/Compilers/CSharp/Portable/Symbols/Synthesized/Records/SynthesizedRecordCopyCtor.cs @@ -18,7 +18,6 @@ public SynthesizedRecordCopyCtor( DiagnosticBag diagnostics) : base(containingType) { - Debug.Assert(!containingType.IsStructType(), "only reference types should define copy constructors"); Parameters = ImmutableArray.Create(SynthesizedParameterSymbol.Create( this, TypeWithAnnotations.Create( @@ -39,20 +38,18 @@ internal override LexicalSortKey GetLexicalSortKey() internal override void GenerateMethodBodyStatements(SyntheticBoundNodeFactory F, ArrayBuilder statements, DiagnosticBag diagnostics) { - // Write assignments to backing fields + // PROTOTYPE: Handle inheritance + // Write assignments to fields // // { - // this.backingField1 = parameter.backingField1 + // this.field1 = parameter.field1 // ... - // this.backingFieldN = parameter.backingFieldN + // this.fieldN = parameter.fieldN // } var param = F.Parameter(Parameters[0]); - foreach (var member in ContainingType.GetMembers()) + foreach (var field in ContainingType.GetFieldsToEmit()) { - if (member is FieldSymbol { IsStatic: false } field) - { - statements.Add(F.Assignment(F.Field(F.This(), field), F.Field(param, field))); - } + statements.Add(F.Assignment(F.Field(F.This(), field), F.Field(param, field))); } } } diff --git a/src/Compilers/CSharp/Test/Symbol/Symbols/Source/RecordTests.cs b/src/Compilers/CSharp/Test/Symbol/Symbols/Source/RecordTests.cs index 1c1d66fcb1f9e..386355a48ecb1 100644 --- a/src/Compilers/CSharp/Test/Symbol/Symbols/Source/RecordTests.cs +++ b/src/Compilers/CSharp/Test/Symbol/Symbols/Source/RecordTests.cs @@ -693,6 +693,123 @@ .maxstack 1 IL_0000: ldarg.0 IL_0001: call ""object..ctor()"" IL_0006: ret +}"); + } + + [Fact] + public void RecordClone3() + { + var comp = CreateCompilation(@" +using System; +public data class C(int x, int y) +{ + public event Action E; + public int Z; +}"); + comp.VerifyDiagnostics(); + + var c = comp.GlobalNamespace.GetTypeMember("C"); + var clone = c.GetMethod(WellKnownMemberNames.CloneMethodName); + Assert.Equal(0, clone.Arity); + Assert.Equal(0, clone.ParameterCount); + Assert.Equal(c, clone.ReturnType); + + var ctor = (MethodSymbol)c.GetMembers(".ctor")[1]; + Assert.Equal(1, ctor.ParameterCount); + Assert.True(ctor.Parameters[0].Type.Equals(c, TypeCompareKind.ConsiderEverything)); + + var verifier = CompileAndVerify(comp, verify: Verification.Fails); + verifier.VerifyIL("C." + WellKnownMemberNames.CloneMethodName, @" +{ + // Code size 7 (0x7) + .maxstack 1 + IL_0000: ldarg.0 + IL_0001: newobj ""C..ctor(C)"" + IL_0006: ret +}"); + verifier.VerifyIL("C..ctor(C)", @" +{ + // Code size 55 (0x37) + .maxstack 2 + IL_0000: ldarg.0 + IL_0001: call ""object..ctor()"" + IL_0006: ldarg.0 + IL_0007: ldarg.1 + IL_0008: ldfld ""int C.k__BackingField"" + IL_000d: stfld ""int C.k__BackingField"" + IL_0012: ldarg.0 + IL_0013: ldarg.1 + IL_0014: ldfld ""int C.k__BackingField"" + IL_0019: stfld ""int C.k__BackingField"" + IL_001e: ldarg.0 + IL_001f: ldarg.1 + IL_0020: ldfld ""System.Action C.E"" + IL_0025: stfld ""System.Action C.E"" + IL_002a: ldarg.0 + IL_002b: ldarg.1 + IL_002c: ldfld ""int C.Z"" + IL_0031: stfld ""int C.Z"" + IL_0036: ret +}"); + } + + [Fact] + public void RecordClone4() + { + var comp = CreateCompilation(@" +using System; +public data struct S(int x, int y) +{ + public event Action E; + public int Z; +}"); + comp.VerifyDiagnostics( + // (5,25): warning CS0067: The event 'S.E' is never used + // public event Action E; + Diagnostic(ErrorCode.WRN_UnreferencedEvent, "E").WithArguments("S.E").WithLocation(5, 25) + ); + + var s = comp.GlobalNamespace.GetTypeMember("S"); + var clone = s.GetMethod(WellKnownMemberNames.CloneMethodName); + Assert.Equal(0, clone.Arity); + Assert.Equal(0, clone.ParameterCount); + Assert.Equal(s, clone.ReturnType); + + var ctor = (MethodSymbol)s.GetMembers(".ctor")[1]; + Assert.Equal(1, ctor.ParameterCount); + Assert.True(ctor.Parameters[0].Type.Equals(s, TypeCompareKind.ConsiderEverything)); + + var verifier = CompileAndVerify(comp, verify: Verification.Fails); + verifier.VerifyIL("S." + WellKnownMemberNames.CloneMethodName, @" +{ + // Code size 12 (0xc) + .maxstack 1 + IL_0000: ldarg.0 + IL_0001: ldobj ""S"" + IL_0006: newobj ""S..ctor(S)"" + IL_000b: ret +}"); + verifier.VerifyIL("S..ctor(S)", @" +{ + // Code size 49 (0x31) + .maxstack 2 + IL_0000: ldarg.0 + IL_0001: ldarg.1 + IL_0002: ldfld ""int S.k__BackingField"" + IL_0007: stfld ""int S.k__BackingField"" + IL_000c: ldarg.0 + IL_000d: ldarg.1 + IL_000e: ldfld ""int S.k__BackingField"" + IL_0013: stfld ""int S.k__BackingField"" + IL_0018: ldarg.0 + IL_0019: ldarg.1 + IL_001a: ldfld ""System.Action S.E"" + IL_001f: stfld ""System.Action S.E"" + IL_0024: ldarg.0 + IL_0025: ldarg.1 + IL_0026: ldfld ""int S.Z"" + IL_002b: stfld ""int S.Z"" + IL_0030: ret }"); } } From 891fd421c4488d861c4959bbc91245eb4b118a06 Mon Sep 17 00:00:00 2001 From: Andy Gocke Date: Thu, 28 May 2020 23:52:00 -0700 Subject: [PATCH 10/17] Update src/Compilers/CSharp/Portable/Symbols/Synthesized/Records/SynthesizedRecordClone.cs Co-authored-by: Jared Parsons --- .../Symbols/Synthesized/Records/SynthesizedRecordClone.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Compilers/CSharp/Portable/Symbols/Synthesized/Records/SynthesizedRecordClone.cs b/src/Compilers/CSharp/Portable/Symbols/Synthesized/Records/SynthesizedRecordClone.cs index 0b6132705eb34..fcde617f1d880 100644 --- a/src/Compilers/CSharp/Portable/Symbols/Synthesized/Records/SynthesizedRecordClone.cs +++ b/src/Compilers/CSharp/Portable/Symbols/Synthesized/Records/SynthesizedRecordClone.cs @@ -23,6 +23,7 @@ public SynthesizedRecordClone(NamedTypeSymbol containingType) { ContainingType = containingType; } + public override string Name => WellKnownMemberNames.CloneMethodName; public override MethodKind MethodKind => MethodKind.Ordinary; @@ -129,4 +130,4 @@ internal override void GenerateMethodBody(TypeCompilationState compilationState, throw ExceptionUtilities.Unreachable; } } -} \ No newline at end of file +} From bd18ab7411afd9a314ed93afc1b2ad8eaa900363 Mon Sep 17 00:00:00 2001 From: Andy Gocke Date: Fri, 29 May 2020 11:22:28 -0700 Subject: [PATCH 11/17] Fix formatting --- src/Test/Utilities/Portable/Assert/AssertEx.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Test/Utilities/Portable/Assert/AssertEx.cs b/src/Test/Utilities/Portable/Assert/AssertEx.cs index 12d85655970b8..18a2b6a5ceb5f 100644 --- a/src/Test/Utilities/Portable/Assert/AssertEx.cs +++ b/src/Test/Utilities/Portable/Assert/AssertEx.cs @@ -443,7 +443,7 @@ public static void SetEqual(IEnumerable expected, IEnumerable actual, I public static void SetEqual(T[] expected, T[] actual) => SetEqual((IEnumerable)actual, expected); - + public static void SetEqual(IEnumerable actual, params T[] expected) { var expectedSet = new HashSet(expected); From 3f35bebbd775bb09d38ad0284c071b1cb07df4dd Mon Sep 17 00:00:00 2001 From: Andy Gocke Date: Fri, 29 May 2020 13:26:50 -0700 Subject: [PATCH 12/17] Fix formatting --- .../Symbols/Synthesized/Records/SynthesizedRecordClone.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Compilers/CSharp/Portable/Symbols/Synthesized/Records/SynthesizedRecordClone.cs b/src/Compilers/CSharp/Portable/Symbols/Synthesized/Records/SynthesizedRecordClone.cs index fcde617f1d880..d04cd5cb5646b 100644 --- a/src/Compilers/CSharp/Portable/Symbols/Synthesized/Records/SynthesizedRecordClone.cs +++ b/src/Compilers/CSharp/Portable/Symbols/Synthesized/Records/SynthesizedRecordClone.cs @@ -23,7 +23,7 @@ public SynthesizedRecordClone(NamedTypeSymbol containingType) { ContainingType = containingType; } - + public override string Name => WellKnownMemberNames.CloneMethodName; public override MethodKind MethodKind => MethodKind.Ordinary; From 7f7be732a6dd669c93905663cbd7f0e2a77e0371 Mon Sep 17 00:00:00 2001 From: Rikki Gibson Date: Fri, 29 May 2020 16:47:34 -0700 Subject: [PATCH 13/17] Pull in some changes from with-expr-fix-binding --- .../Portable/FlowAnalysis/AbstractFlowPass.cs | 5 +- .../NullableWalker.DebugVerifier.cs | 14 --- .../Portable/FlowAnalysis/NullableWalker.cs | 64 ++++------ .../Test/Semantic/Semantics/RecordTests.cs | 115 ++++++++---------- 4 files changed, 76 insertions(+), 122 deletions(-) diff --git a/src/Compilers/CSharp/Portable/FlowAnalysis/AbstractFlowPass.cs b/src/Compilers/CSharp/Portable/FlowAnalysis/AbstractFlowPass.cs index ec9bd56d0afff..8577a6c8de7d9 100644 --- a/src/Compilers/CSharp/Portable/FlowAnalysis/AbstractFlowPass.cs +++ b/src/Compilers/CSharp/Portable/FlowAnalysis/AbstractFlowPass.cs @@ -2027,10 +2027,7 @@ public override BoundNode VisitWhileStatement(BoundWhileStatement node) public override BoundNode VisitWithExpression(BoundWithExpression node) { VisitRvalue(node.Receiver); - foreach (var (_, expr) in node.Arguments) - { - VisitRvalue(expr); - } + VisitObjectOrCollectionInitializerExpression(node.InitializerExpression.Initializers); return null; } diff --git a/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.DebugVerifier.cs b/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.DebugVerifier.cs index 9093c4bb71816..c6fa8b084c6d6 100644 --- a/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.DebugVerifier.cs +++ b/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.DebugVerifier.cs @@ -187,20 +187,6 @@ private void VisitBinaryOperatorChildren(BoundBinaryOperatorBase node) Visit(node.BoundContainingTypeOpt); return null; } - - public override BoundNode? VisitWithExpression(BoundWithExpression node) - { - Visit(node.Receiver); - // PROTOTYPE: We shouldn't need to implement this method manually here. - // Either BoundNodeClassWriter should be made aware of tuples, - // or the `BoundWithExpression.Arguments` should be changed to - // `ImmutableArray` as in `BoundObjectInitializerExpressionBase.Initializers`. - foreach (var (_, expr) in node.Arguments) - { - Visit(expr); - } - return null; - } } #endif } diff --git a/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs b/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs index 9a15fd1ae33d5..ec8d93e83c7d4 100644 --- a/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs +++ b/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs @@ -2230,36 +2230,24 @@ public override BoundNode VisitWithExpression(BoundWithExpression withExpr) VisitRvalue(receiver); _ = CheckPossibleNullReceiver(receiver, checkNullableValueType: false); + var resultType = withExpr.CloneMethod?.ReturnTypeWithAnnotations ?? ResultType.ToTypeWithAnnotations(); var resultSlot = GetOrCreatePlaceholderSlot(withExpr); // carry over the null state of members of 'receiver' to the result of the with-expression. - TrackNullableStateForAssignment(receiver, ResultType.ToTypeWithAnnotations(), resultSlot, ResultType, MakeSlot(receiver)); - - foreach (var (leftSymbol, right) in withExpr.Arguments) + TrackNullableStateForAssignment(receiver, resultType, resultSlot, resultType.ToTypeWithState(), MakeSlot(receiver)); + foreach (var expr in withExpr.InitializerExpression.Initializers) { - if (leftSymbol is PropertySymbol prop) + if (expr is BoundAssignmentOperator assignment) { - // PROTOTYPE: attributes on positional record parameters do not propagate - // to the synthesized properties. Perhaps this is by design? - // If we want them to propagate then perhaps we should move - // members such as SourcePropertySymbol.HasAllowNull, etc. to SourceOrRecordPropertySymbol. - var leftAnnotations = GetPropertySetterAnnotations(prop); - var leftLValueType = ApplyLValueAnnotations(prop.TypeWithAnnotations, leftAnnotations); - - var rightState = VisitOptionalImplicitConversion(right, targetTypeOpt: leftLValueType, useLegacyWarnings: false, trackMembers: true, AssignmentKind.Assignment); - CheckDisallowedNullAssignment(rightState, leftAnnotations, right.Syntax.Location); - var propSlot = GetOrCreateSlot(prop, resultSlot); - TrackNullableStateForAssignment(right, leftLValueType, propSlot, rightState, MakeSlot(right)); - - // PROTOTYPE: Should we AdjustSetValue for properties? - // e.g. a property with [AllowNull, NotNull] would have a NotNull state after assigning a maybe-null. - // This isn't safe at runtime but it is consistent with what we allow users to do with auto properties. - // AdjustSetValue(left, declaredType, leftLValueType, ref rightState); + VisitObjectElementInitializer(resultSlot, assignment); + } + else + { + VisitRvalue(expr); } } - SetResultType(withExpr, - withExpr.CloneMethod?.ReturnTypeWithAnnotations.ToTypeWithState() - ?? TypeWithState.Create(withExpr.Type, NullableFlowState.NotNull)); + var placeholder = withExpr.InitializerExpression.Placeholder; + SetNotNullResult(placeholder); return null; } @@ -7024,10 +7012,10 @@ private FlowAnalysisAnnotations GetLValueAnnotations(BoundExpression expr) var annotations = expr switch { - BoundPropertyAccess property => GetPropertySetterAnnotations(property.PropertySymbol), - BoundIndexerAccess indexer => GetPropertySetterAnnotations(indexer.Indexer), + BoundPropertyAccess property => getSetterAnnotations(property.PropertySymbol), + BoundIndexerAccess indexer => getSetterAnnotations(indexer.Indexer), BoundFieldAccess field => getFieldAnnotations(field.FieldSymbol), - BoundObjectInitializerMember { MemberSymbol: PropertySymbol prop } => GetPropertySetterAnnotations(prop), + BoundObjectInitializerMember { MemberSymbol: PropertySymbol prop } => getSetterAnnotations(prop), BoundObjectInitializerMember { MemberSymbol: FieldSymbol field } => getFieldAnnotations(field), BoundParameter { ParameterSymbol: ParameterSymbol parameter } => ToInwardAnnotations(GetParameterAnnotations(parameter) & ~FlowAnalysisAnnotations.NotNull), // NotNull is enforced upon method exit @@ -7039,23 +7027,23 @@ private FlowAnalysisAnnotations GetLValueAnnotations(BoundExpression expr) static FlowAnalysisAnnotations getFieldAnnotations(FieldSymbol field) { return field.AssociatedSymbol is PropertySymbol property ? - GetPropertySetterAnnotations(property) : + getSetterAnnotations(property) : field.FlowAnalysisAnnotations; } - } - private static FlowAnalysisAnnotations GetPropertySetterAnnotations(PropertySymbol property) - { - var accessor = property.GetOwnOrInheritedSetMethod(); - if (accessor is object) + static FlowAnalysisAnnotations getSetterAnnotations(PropertySymbol property) { - return accessor.Parameters.Last().FlowAnalysisAnnotations; - } - if (property is SourcePropertySymbol sourceProperty) - { - return getPropertyAnnotations(sourceProperty); + var accessor = property.GetOwnOrInheritedSetMethod(); + if (accessor is object) + { + return accessor.Parameters.Last().FlowAnalysisAnnotations; + } + if (property is SourcePropertySymbol sourceProperty) + { + return getPropertyAnnotations(sourceProperty); + } + return FlowAnalysisAnnotations.None; } - return FlowAnalysisAnnotations.None; static FlowAnalysisAnnotations getPropertyAnnotations(SourcePropertySymbol property) { diff --git a/src/Compilers/CSharp/Test/Semantic/Semantics/RecordTests.cs b/src/Compilers/CSharp/Test/Semantic/Semantics/RecordTests.cs index 0275eaef166b6..d5e034739e084 100644 --- a/src/Compilers/CSharp/Test/Semantic/Semantics/RecordTests.cs +++ b/src/Compilers/CSharp/Test/Semantic/Semantics/RecordTests.cs @@ -1557,6 +1557,7 @@ public static void Main() ); } + [Fact] public void WithExpr_DefiniteAssignment_01() { var src = @" @@ -1570,11 +1571,7 @@ static void M(B b) } }"; var comp = CreateCompilation(src); - // PROTOTYPE: records don't auto-generate Clone at the moment - comp.VerifyDiagnostics( - // (7,13): error CS8858: The receiver type 'B' does not have an accessible parameterless instance method named "Clone". - // _ = b with { X = y = 42 }; - Diagnostic(ErrorCode.ERR_NoSingleCloneMethod, "b").WithArguments("B").WithLocation(7, 13)); + comp.VerifyDiagnostics(); } [Fact] @@ -1590,11 +1587,7 @@ static void M(B b) } }"; var comp = CreateCompilation(src); - // PROTOTYPE: records don't auto-generate Clone at the moment - comp.VerifyDiagnostics( - // (7,13): error CS8858: The receiver type 'B' does not have an accessible parameterless instance method named "Clone". - // _ = b with { X = y = 42 }; - Diagnostic(ErrorCode.ERR_NoSingleCloneMethod, "b").WithArguments("B").WithLocation(7, 13)); + comp.VerifyDiagnostics(); } [Fact] @@ -1610,11 +1603,7 @@ static void M(B b) } }"; var comp = CreateCompilation(src); - // PROTOTYPE: records don't auto-generate Clone at the moment comp.VerifyDiagnostics( - // (7,13): error CS8858: The receiver type 'B' does not have an accessible parameterless instance method named "Clone". - // _ = b with { Y = z.ToString(), X = z = 42 }; - Diagnostic(ErrorCode.ERR_NoSingleCloneMethod, "b").WithArguments("B").WithLocation(7, 13), // (7,26): error CS0165: Use of unassigned local variable 'z' // _ = b with { Y = z.ToString(), X = z = 42 }; Diagnostic(ErrorCode.ERR_UseDefViolation, "z").WithArguments("z").WithLocation(7, 26)); @@ -1633,11 +1622,7 @@ static void M() } }"; var comp = CreateCompilation(src); - // PROTOTYPE: records don't auto-generate Clone at the moment - comp.VerifyDiagnostics( - // (7,13): error CS8858: The receiver type 'B' does not have an accessible parameterless instance method named "Clone". - // _ = (b = new B(42)) with { X = b.X }; - Diagnostic(ErrorCode.ERR_NoSingleCloneMethod, "(b = new B(42))").WithArguments("B").WithLocation(7, 13)); + comp.VerifyDiagnostics(); } [Fact] @@ -1653,11 +1638,7 @@ static void M() } }"; var comp = CreateCompilation(src); - // PROTOTYPE: records don't auto-generate Clone at the moment comp.VerifyDiagnostics( - // (7,13): error CS8858: The receiver type 'B' does not have an accessible parameterless instance method named "Clone". - // _ = new B(b.X) with { X = new B(42).X }; - Diagnostic(ErrorCode.ERR_NoSingleCloneMethod, "new B(b.X)").WithArguments("B").WithLocation(7, 13), // (7,19): error CS0165: Use of unassigned local variable 'b' // _ = new B(b.X) with { X = new B(42).X }; Diagnostic(ErrorCode.ERR_UseDefViolation, "b").WithArguments("b").WithLocation(7, 19)); @@ -1679,11 +1660,7 @@ static void M(B b) static int M(out int y) { y = 42; return 43; } }"; var comp = CreateCompilation(src); - // PROTOTYPE: records don't auto-generate Clone at the moment - comp.VerifyDiagnostics( - // (7,13): error CS8858: The receiver type 'B' does not have an accessible parameterless instance method named "Clone". - // _ = b with { X = M(out y) }; - Diagnostic(ErrorCode.ERR_NoSingleCloneMethod, "b").WithArguments("B").WithLocation(7, 13)); + comp.VerifyDiagnostics(); } [Fact] @@ -1701,11 +1678,7 @@ static void M(B b) static int M(out int y) { y = 42; return 43; } }"; var comp = CreateCompilation(src); - // PROTOTYPE: records don't auto-generate Clone at the moment - comp.VerifyDiagnostics( - // (6,13): error CS8858: The receiver type 'B' does not have an accessible parameterless instance method named "Clone". - // _ = b with { X = M(out int y) }; - Diagnostic(ErrorCode.ERR_NoSingleCloneMethod, "b").WithArguments("B").WithLocation(6, 13)); + comp.VerifyDiagnostics(); } [Fact] @@ -1725,11 +1698,7 @@ static void M(B b) static int M(out string s) { s = ""a""; return 42; } }"; var comp = CreateCompilation(src); - // PROTOTYPE: records don't auto-generate Clone at the moment - comp.VerifyDiagnostics( - // (8,13): error CS8858: The receiver type 'B' does not have an accessible parameterless instance method named "Clone". - // _ = b with { X = M(out s) }; - Diagnostic(ErrorCode.ERR_NoSingleCloneMethod, "b").WithArguments("B").WithLocation(8, 13)); + comp.VerifyDiagnostics(); } [Fact] @@ -1747,11 +1716,7 @@ static void M(B b, string? s) } }"; var comp = CreateCompilation(src); - // PROTOTYPE: records don't auto-generate Clone at the moment comp.VerifyDiagnostics( - // (8,13): error CS8858: The receiver type 'B' does not have an accessible parameterless instance method named "Clone". - // _ = b with { X = s }; // 1 - Diagnostic(ErrorCode.ERR_NoSingleCloneMethod, "b").WithArguments("B").WithLocation(8, 13), // (8,26): warning CS8601: Possible null reference assignment. // _ = b with { X = s }; // 1 Diagnostic(ErrorCode.WRN_NullReferenceAssignment, "s").WithLocation(8, 26)); @@ -1813,17 +1778,10 @@ static void M2(B? b) } }"; var comp = CreateCompilation(src); - // PROTOTYPE: records don't auto-generate Clone at the moment comp.VerifyDiagnostics( - // (7,18): error CS8858: The receiver type 'B' does not have an accessible parameterless instance method named "Clone". - // var b1 = b with { X = 42 }; // 1 - Diagnostic(ErrorCode.ERR_NoSingleCloneMethod, "b").WithArguments("B").WithLocation(7, 18), // (7,18): warning CS8602: Dereference of a possibly null reference. // var b1 = b with { X = 42 }; // 1 Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "b").WithLocation(7, 18), - // (14,10): error CS8858: The receiver type 'B' does not have an accessible parameterless instance method named "Clone". - // (b with { X = 42 }).ToString(); // 2 - Diagnostic(ErrorCode.ERR_NoSingleCloneMethod, "b").WithArguments("B").WithLocation(14, 10), // (14,10): warning CS8602: Dereference of a possibly null reference. // (b with { X = 42 }).ToString(); // 2 Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "b").WithLocation(14, 10)); @@ -1867,12 +1825,12 @@ static void M1(bool flag) Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "b.X").WithLocation(17, 9)); } - [Fact(Skip = "PROTOTYPE(records): with expression using init accessors")] + [Fact] public void WithExpr_NullableAnalysis_06() { var src = @" #nullable enable -data class B +class B { public string? X { get; init; } public string? Y { get; init; } @@ -1896,18 +1854,12 @@ static void M1(bool flag) }"; var comp = CreateCompilation(src); comp.VerifyDiagnostics( - // (9,21): warning CS8602: Dereference of a possibly null reference. - // if (flag) { b.X.ToString(); } // 1 - Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "b.X").WithLocation(9, 21), - // (11,21): warning CS8602: Dereference of a possibly null reference. - // if (flag) { b.X.ToString(); } // 2 - Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "b.X").WithLocation(11, 21), - // (16,21): warning CS8602: Dereference of a possibly null reference. - // if (flag) { b.X.ToString(); } // 3 - Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "b.X").WithLocation(16, 21)); + // (16,13): warning CS8602: Dereference of a possibly null reference. + // b.Y.ToString(); // 1 + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "b.Y").WithLocation(16, 13)); } - [Fact(Skip = "PROTOTYPE(records): positional property attributes")] + [Fact(Skip = "https://github.com/dotnet/roslyn/issues/44691")] public void WithExpr_NullableAnalysis_07() { var src = @" @@ -1922,14 +1874,13 @@ static void M1(B b) { b.X.ToString(); b = b with { X = null }; - b.X.ToString(); // 1 + b.X.ToString(); // ok + b = new B(null); + b.X.ToString(); // ok } }"; var comp = CreateCompilation(new[] { src, AllowNullAttributeDefinition }); - comp.VerifyDiagnostics( - // (13,9): warning CS8602: Dereference of a possibly null reference. - // b.X.ToString(); // 1 - Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "b.X").WithLocation(13, 9)); + comp.VerifyDiagnostics(); } [Fact] @@ -1973,6 +1924,38 @@ static void M1(B b1) Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "b3.X").WithLocation(17, 9)); } + [Fact] + public void WithExpr_NullableAnalysis_VariantClone() + { + var src = @" +#nullable enable + +class A +{ + public string? Y { get; init; } + public string? Z { get; init; } +} + +data class B(string? X) : A +{ + public A Clone() => new B(X) { Y = Y, Z = Z }; + public new string Z { get; init; } = ""zed""; + + static void M1(B b1) + { + b1.Z.ToString(); + (b1 with { Y = ""hello"" }).Y.ToString(); + (b1 with { Y = ""hello"" }).Z.ToString(); // 1 + } +} +"; + var comp = CreateCompilation(src); + comp.VerifyDiagnostics( + // (19,9): warning CS8602: Dereference of a possibly null reference. + // (b1 with { Y = "hello" }).Z.ToString(); // 1 + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, @"(b1 with { Y = ""hello"" }).Z").WithLocation(19, 9)); + } + [Fact] public void WithExprNotRecord() { From 1b9da526979eaff276b7c2215c7e351cb09e8131 Mon Sep 17 00:00:00 2001 From: Rikki Gibson Date: Mon, 1 Jun 2020 12:45:46 -0700 Subject: [PATCH 14/17] Reuse object initializer analysis --- .../Portable/FlowAnalysis/NullableWalker.cs | 32 +++--- .../Test/Semantic/Semantics/RecordTests.cs | 105 ++++++++++++++++++ 2 files changed, 121 insertions(+), 16 deletions(-) diff --git a/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs b/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs index ec8d93e83c7d4..6fc2e8fbf1039 100644 --- a/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs +++ b/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs @@ -2228,26 +2228,19 @@ public override BoundNode VisitWithExpression(BoundWithExpression withExpr) var receiver = withExpr.Receiver; VisitRvalue(receiver); - _ = CheckPossibleNullReceiver(receiver, checkNullableValueType: false); + _ = CheckPossibleNullReceiver(receiver); var resultType = withExpr.CloneMethod?.ReturnTypeWithAnnotations ?? ResultType.ToTypeWithAnnotations(); + var resultState = ApplyUnconditionalAnnotations(resultType.ToTypeWithState(), GetRValueAnnotations(withExpr.CloneMethod)); var resultSlot = GetOrCreatePlaceholderSlot(withExpr); // carry over the null state of members of 'receiver' to the result of the with-expression. - TrackNullableStateForAssignment(receiver, resultType, resultSlot, resultType.ToTypeWithState(), MakeSlot(receiver)); - foreach (var expr in withExpr.InitializerExpression.Initializers) - { - if (expr is BoundAssignmentOperator assignment) - { - VisitObjectElementInitializer(resultSlot, assignment); - } - else - { - VisitRvalue(expr); - } - } + TrackNullableStateForAssignment(receiver, resultType, resultSlot, resultState, MakeSlot(receiver)); + // use the declared nullability of Clone() for the top-level nullability of the result of the with-expression. + SetResult(withExpr, resultState, resultType); + VisitObjectCreationInitializer(containingSymbol: null, resultSlot, withExpr.InitializerExpression, FlowAnalysisAnnotations.None); - var placeholder = withExpr.InitializerExpression.Placeholder; - SetNotNullResult(placeholder); + // Note: this does not account for the scenario where `Clone()` returns maybe-null and the with-expression has no initializers. + // Tracking in https://github.com/dotnet/roslyn/issues/44759 return null; } @@ -2630,7 +2623,14 @@ void checkImplicitReceiver() { if (!node.Type.IsValueType && State[containingSlot].MayBeNull()) { - ReportDiagnostic(ErrorCode.WRN_NullReferenceInitializer, node.Syntax, containingSymbol); + if (containingSymbol is null) + { + ReportDiagnostic(ErrorCode.WRN_NullReferenceReceiver, node.Syntax); + } + else + { + ReportDiagnostic(ErrorCode.WRN_NullReferenceInitializer, node.Syntax, containingSymbol); + } } } } diff --git a/src/Compilers/CSharp/Test/Semantic/Semantics/RecordTests.cs b/src/Compilers/CSharp/Test/Semantic/Semantics/RecordTests.cs index d5e034739e084..2cfb60003d40a 100644 --- a/src/Compilers/CSharp/Test/Semantic/Semantics/RecordTests.cs +++ b/src/Compilers/CSharp/Test/Semantic/Semantics/RecordTests.cs @@ -1956,6 +1956,111 @@ static void M1(B b1) Diagnostic(ErrorCode.WRN_NullReferenceReceiver, @"(b1 with { Y = ""hello"" }).Z").WithLocation(19, 9)); } + [Fact] + public void WithExpr_NullableAnalysis_NullableClone() + { + var src = @" +#nullable enable + +data class B(string? X) +{ + public B? Clone() => new B(X); + + static void M1(B b1) + { + _ = b1 with { X = null }; // 1 + (b1 with { X = null }).ToString(); // 2 + } +} +"; + var comp = CreateCompilation(src); + comp.VerifyDiagnostics( + // (10,21): warning CS8602: Dereference of a possibly null reference. + // _ = b1 with { X = null }; // 1 + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "{ X = null }").WithLocation(10, 21), + // (11,18): warning CS8602: Dereference of a possibly null reference. + // (b1 with { X = null }).ToString(); // 2 + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "{ X = null }").WithLocation(11, 18)); + } + + [Fact] + public void WithExpr_NullableAnalysis_MaybeNullClone() + { + var src = @" +#nullable enable +using System.Diagnostics.CodeAnalysis; + +data class B(string? X) +{ + [return: MaybeNull] + public B Clone() => new B(X); + + static void M1(B b1) + { + _ = b1 with { }; + _ = b1 with { X = null }; // 1 + (b1 with { X = null }).ToString(); // 2 + } +} +"; + var comp = CreateCompilation(new[] { src, MaybeNullAttributeDefinition }); + comp.VerifyDiagnostics( + // (13,21): warning CS8602: Dereference of a possibly null reference. + // _ = b1 with { X = null }; // 1 + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "{ X = null }").WithLocation(13, 21), + // (14,18): warning CS8602: Dereference of a possibly null reference. + // (b1 with { X = null }).ToString(); // 2 + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "{ X = null }").WithLocation(14, 18)); + } + + [Fact] + public void WithExpr_NullableAnalysis_NotNullClone() + { + var src = @" +#nullable enable +using System.Diagnostics.CodeAnalysis; + +data class B(string? X) +{ + [return: NotNull] + public B? Clone() => new B(X); + + static void M1(B b1) + { + _ = b1 with { }; + _ = b1 with { X = null }; + (b1 with { X = null }).ToString(); + } +} +"; + var comp = CreateCompilation(new[] { src, NotNullAttributeDefinition }); + comp.VerifyDiagnostics(); + } + + [Fact] + public void WithExpr_NullableAnalysis_NullableClone_NoInitializers() + { + var src = @" +#nullable enable + +data class B(string? X) +{ + public B? Clone() => new B(X); + + static void M1(B b1) + { + _ = b1 with { }; + (b1 with { }).ToString(); // 1 + } +} +"; + var comp = CreateCompilation(src); + // Note: we expect to give a warning on `// 1`, but do not currently + // due to limitations of object initializer analysis. + // Tracking in https://github.com/dotnet/roslyn/issues/44759 + comp.VerifyDiagnostics(); + } + [Fact] public void WithExprNotRecord() { From 0c77d6372802545b2c95d6c6e369ba3d9575911e Mon Sep 17 00:00:00 2001 From: Rikki Gibson Date: Mon, 1 Jun 2020 14:15:30 -0700 Subject: [PATCH 15/17] Add simple with-expr dataflow test --- .../FlowAnalysis/RegionAnalysisTests.cs | 34 +++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/src/Compilers/CSharp/Test/Semantic/FlowAnalysis/RegionAnalysisTests.cs b/src/Compilers/CSharp/Test/Semantic/FlowAnalysis/RegionAnalysisTests.cs index dd20e2a29e5e4..f15d3c8922fd5 100644 --- a/src/Compilers/CSharp/Test/Semantic/FlowAnalysis/RegionAnalysisTests.cs +++ b/src/Compilers/CSharp/Test/Semantic/FlowAnalysis/RegionAnalysisTests.cs @@ -2053,6 +2053,40 @@ public static void Main() Assert.Equal("x, z, i", GetSymbolNamesJoined(analysis.WrittenOutside)); } + [Fact] + public void TestWithExpression() + { + var analysis = CompileAndAnalyzeDataFlowExpression(@" +#nullable enable + +data class B(string? X) +{ + static void M1(B b1) + { + var x = ""hello""; + _ = /**/b1 with { X = x }/**/; + } +} +"); + Assert.Null(GetSymbolNamesJoined(analysis.AlwaysAssigned)); + Assert.Null(GetSymbolNamesJoined(analysis.Captured)); + Assert.Null(GetSymbolNamesJoined(analysis.CapturedInside)); + Assert.Null(GetSymbolNamesJoined(analysis.CapturedOutside)); + Assert.Null(GetSymbolNamesJoined(analysis.VariablesDeclared)); + + Assert.Equal("b1, x", GetSymbolNamesJoined(analysis.DataFlowsIn)); + Assert.Null(GetSymbolNamesJoined(analysis.DataFlowsOut)); + + Assert.Equal("b1, x", GetSymbolNamesJoined(analysis.DefinitelyAssignedOnEntry)); + Assert.Equal("b1, x", GetSymbolNamesJoined(analysis.DefinitelyAssignedOnExit)); + + Assert.Equal("b1, x", GetSymbolNamesJoined(analysis.ReadInside)); + Assert.Null(GetSymbolNamesJoined(analysis.ReadOutside)); + + Assert.Null(GetSymbolNamesJoined(analysis.WrittenInside)); + Assert.Equal("b1, x", GetSymbolNamesJoined(analysis.WrittenOutside)); + } + [Fact] public void TestCollectionInitializerExpression() { From 9812a0173957259eb8e7480bc43a6727cf5eba0a Mon Sep 17 00:00:00 2001 From: Rikki Gibson Date: Mon, 1 Jun 2020 14:31:24 -0700 Subject: [PATCH 16/17] Unskip test, change PROTOTYPE to issue link --- .../Test/Semantic/Semantics/RecordTests.cs | 32 ++++++++++++------- 1 file changed, 21 insertions(+), 11 deletions(-) diff --git a/src/Compilers/CSharp/Test/Semantic/Semantics/RecordTests.cs b/src/Compilers/CSharp/Test/Semantic/Semantics/RecordTests.cs index 2cfb60003d40a..81ead88ccd0a9 100644 --- a/src/Compilers/CSharp/Test/Semantic/Semantics/RecordTests.cs +++ b/src/Compilers/CSharp/Test/Semantic/Semantics/RecordTests.cs @@ -1787,7 +1787,7 @@ static void M2(B? b) Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "b").WithLocation(14, 10)); } - [Fact] + [Fact, WorkItem(44763, "https://github.com/dotnet/roslyn/issues/44763")] public void WithExpr_NullableAnalysis_05() { var src = @" @@ -1801,27 +1801,28 @@ static void M1(bool flag) B b = new B(""hello"", null); if (flag) { - b.X.ToString(); // PROTOTYPE(records) + b.X.ToString(); // shouldn't warn b.Y.ToString(); // 1 } b = b with { Y = ""world"" }; - b.X.ToString(); // PROTOTYPE(records) + b.X.ToString(); // shouldn't warn b.Y.ToString(); } }"; - // PROTOTYPE: it feels like records should propagate the nullability of - // the constructor arguments to the corresponding properties. + // records should propagate the nullability of the + // constructor arguments to the corresponding properties. + // https://github.com/dotnet/roslyn/issues/44763 var comp = CreateCompilation(src); comp.VerifyDiagnostics( // (12,13): warning CS8602: Dereference of a possibly null reference. - // b.X.ToString(); // PROTOTYPE(records) + // b.X.ToString(); // shouldn't warn Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "b.X").WithLocation(12, 13), // (13,13): warning CS8602: Dereference of a possibly null reference. // b.Y.ToString(); // 1 Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "b.Y").WithLocation(13, 13), // (17,9): warning CS8602: Dereference of a possibly null reference. - // b.X.ToString(); // PROTOTYPE(records) + // b.X.ToString(); // shouldn't warn Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "b.X").WithLocation(17, 9)); } @@ -1859,7 +1860,7 @@ static void M1(bool flag) Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "b.Y").WithLocation(16, 13)); } - [Fact(Skip = "https://github.com/dotnet/roslyn/issues/44691")] + [Fact, WorkItem(44691, "https://github.com/dotnet/roslyn/issues/44691")] public void WithExpr_NullableAnalysis_07() { var src = @" @@ -1873,14 +1874,23 @@ data class B([AllowNull] string X) static void M1(B b) { b.X.ToString(); - b = b with { X = null }; + b = b with { X = null }; // ok b.X.ToString(); // ok - b = new B(null); + b = new B((string?)null); b.X.ToString(); // ok } }"; + // We should have a way to propagate attributes on + // positional parameters to the corresponding properties. + // https://github.com/dotnet/roslyn/issues/44691 var comp = CreateCompilation(new[] { src, AllowNullAttributeDefinition }); - comp.VerifyDiagnostics(); + comp.VerifyDiagnostics( + // (12,26): warning CS8625: Cannot convert null literal to non-nullable reference type. + // b = b with { X = null }; // ok + Diagnostic(ErrorCode.WRN_NullAsNonNullable, "null").WithLocation(12, 26), + // (13,9): warning CS8602: Dereference of a possibly null reference. + // b.X.ToString(); // ok + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "b.X").WithLocation(13, 9)); } [Fact] From fdfe36555064d7492376097a7df1f3f38913890c Mon Sep 17 00:00:00 2001 From: Rikki Gibson Date: Mon, 1 Jun 2020 14:57:50 -0700 Subject: [PATCH 17/17] Add a few more nullable tests --- .../Test/Semantic/Semantics/RecordTests.cs | 50 +++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/src/Compilers/CSharp/Test/Semantic/Semantics/RecordTests.cs b/src/Compilers/CSharp/Test/Semantic/Semantics/RecordTests.cs index 81ead88ccd0a9..30b4cc944bdee 100644 --- a/src/Compilers/CSharp/Test/Semantic/Semantics/RecordTests.cs +++ b/src/Compilers/CSharp/Test/Semantic/Semantics/RecordTests.cs @@ -1934,6 +1934,56 @@ static void M1(B b1) Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "b3.X").WithLocation(17, 9)); } + [Fact] + public void WithExpr_NullableAnalysis_09() + { + var src = @" +#nullable enable +data class B(string? X, string? Y) +{ + static void M1(B b1) + { + string? local = ""hello""; + _ = b1 with + { + X = local = null, + Y = local.ToString() // 1 + }; + } +}"; + var comp = CreateCompilation(src); + comp.VerifyDiagnostics( + // (11,17): warning CS8602: Dereference of a possibly null reference. + // Y = local.ToString() // 1 + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "local").WithLocation(11, 17)); + } + + [Fact] + public void WithExpr_NullableAnalysis_10() + { + var src = @" +#nullable enable +data class B(string X, string Y) +{ + static string M0(out string? s) { s = null; return ""hello""; } + + static void M1(B b1) + { + string? local = ""world""; + _ = b1 with + { + X = M0(out local), + Y = local // 1 + }; + } +}"; + var comp = CreateCompilation(src); + comp.VerifyDiagnostics( + // (13,17): warning CS8601: Possible null reference assignment. + // Y = local // 1 + Diagnostic(ErrorCode.WRN_NullReferenceAssignment, "local").WithLocation(13, 17)); + } + [Fact] public void WithExpr_NullableAnalysis_VariantClone() {