From 0c41f1d6a4bd3b3638384b773c3dabeda0b62c3d Mon Sep 17 00:00:00 2001 From: vsadov Date: Tue, 23 Jan 2018 18:17:04 -0800 Subject: [PATCH 01/10] cleaned up fixed error reporting --- .../Portable/Binder/Binder_Statements.cs | 114 +++++++----------- .../Portable/CSharpResources.Designer.cs | 9 ++ .../CSharp/Portable/CSharpResources.resx | 3 + .../CSharp/Portable/Errors/ErrorCode.cs | 2 + .../CSharp/Test/Emit/CodeGen/UnsafeTests.cs | 55 ++++++++- .../Semantic/Semantics/SemanticErrorTests.cs | 10 +- .../Semantics/TargetTypedDefaultTests.cs | 8 +- .../Test/Semantic/Semantics/UnsafeTests.cs | 34 +++--- 8 files changed, 139 insertions(+), 96 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Binder/Binder_Statements.cs b/src/Compilers/CSharp/Portable/Binder/Binder_Statements.cs index 658eb47cd7b02..4f1b06d708022 100644 --- a/src/Compilers/CSharp/Portable/Binder/Binder_Statements.cs +++ b/src/Compilers/CSharp/Portable/Binder/Binder_Statements.cs @@ -1041,7 +1041,7 @@ private bool IsValidFixedVariableInitializer(TypeSymbol declType, SourceLocalSym Debug.Assert(!ReferenceEquals(declType, null)); Debug.Assert(declType.IsPointerType()); - if (ReferenceEquals(initializerOpt, null)) + if (initializerOpt?.HasAnyErrors != false) { return false; } @@ -1049,86 +1049,62 @@ private bool IsValidFixedVariableInitializer(TypeSymbol declType, SourceLocalSym TypeSymbol initializerType = initializerOpt.Type; SyntaxNode initializerSyntax = initializerOpt.Syntax; - if (ReferenceEquals(initializerType, null)) + if ((object)initializerType == null) { - initializerOpt = GenerateConversionForAssignment(declType, initializerOpt, diagnostics); - if (!initializerOpt.HasAnyErrors) - { - // Dev10 just reports the assignment conversion error, which must occur, except for these cases: - Debug.Assert( - initializerOpt is BoundConvertedStackAllocExpression || - initializerOpt is BoundConversion conversion && (conversion.Operand.IsLiteralNull() || conversion.Operand.Kind == BoundKind.DefaultExpression), - "All other typeless expressions should have conversion errors"); - - // CONSIDER: this is a very confusing error message, but it's what Dev10 reports. - Error(diagnostics, ErrorCode.ERR_FixedNotNeeded, initializerSyntax); - } + // TODO: VS replace with better error. + // CONSIDER: this is a very confusing error message, but it's what Dev10 reports. + Error(diagnostics, ErrorCode.ERR_ExprCannotBeFixed, initializerSyntax); + return false; } - else + + TypeSymbol elementType; + bool hasErrors = false; + + switch (initializerOpt.Kind) { - TypeSymbol elementType; - bool hasErrors = false; + case BoundKind.AddressOfOperator: + elementType = ((BoundAddressOfOperator)initializerOpt).Operand.Type; + break; - if (initializerType.SpecialType == SpecialType.System_String) - { - elementType = this.GetSpecialType(SpecialType.System_Char, diagnostics, initializerSyntax); - Debug.Assert(!elementType.IsManagedType); - } - else if (initializerType.IsArray()) - { - // See ExpressionBinder::BindPtrToArray (though most of that functionality is now in LocalRewriter). - var arrayType = (ArrayTypeSymbol)initializerType; - elementType = arrayType.ElementType; + case BoundKind.FieldAccess: + var fa = (BoundFieldAccess)initializerOpt; + if (fa.FieldSymbol.IsFixed) + { + elementType = ((PointerTypeSymbol)fa.Type).PointedAtType; + break; + } - if (elementType.IsManagedType) + goto default; + + default: + // fixed (T* variable = ) ... + if (initializerType.SpecialType == SpecialType.System_String) { - Error(diagnostics, ErrorCode.ERR_ManagedAddr, initializerSyntax, elementType); - hasErrors = true; + elementType = this.GetSpecialType(SpecialType.System_Char, diagnostics, initializerSyntax); + Debug.Assert(!elementType.IsManagedType); } - } - else if (!initializerOpt.HasAnyErrors) - { - elementType = initializerOpt.Type; - switch (initializerOpt.Kind) + else if (initializerType.IsArray()) { - case BoundKind.AddressOfOperator: - elementType = ((BoundAddressOfOperator)initializerOpt).Operand.Type; - break; - case BoundKind.FieldAccess: - var fa = (BoundFieldAccess)initializerOpt; - if (!fa.FieldSymbol.IsFixed) - { - Error(diagnostics, ErrorCode.ERR_FixedNotNeeded, initializerSyntax); - return true; - } - else - { - elementType = ((PointerTypeSymbol)fa.Type).PointedAtType; - } - break; - case BoundKind.Conversion: - // The following assertion would not be correct because there might be an implicit conversion after (above) the explicit one. - //Debug.Assert(((BoundConversion)initializerOpt).ExplicitCastInCode, "The assignment conversion hasn't been applied yet, so this must be from source."); - - // NOTE: Dev10 specifically doesn't report this error for the array or string cases. - Error(diagnostics, ErrorCode.ERR_BadCastInFixed, initializerSyntax); - return true; - default: - // CONSIDER: this is a very confusing error message, but it's what Dev10 reports. - Error(diagnostics, ErrorCode.ERR_FixedNotNeeded, initializerSyntax); - return true; + // See ExpressionBinder::BindPtrToArray (though most of that functionality is now in LocalRewriter). + var arrayType = (ArrayTypeSymbol)initializerType; + elementType = arrayType.ElementType; + } + else + { + Error(diagnostics, ErrorCode.ERR_ExprCannotBeFixed, initializerSyntax); + return false; } - } - else - { - // convert to generate any additional conversion errors - initializerOpt = GenerateConversionForAssignment(declType, initializerOpt, diagnostics); - return true; - } - initializerOpt = GetFixedLocalCollectionInitializer(initializerOpt, elementType, declType, hasErrors, diagnostics); + break; + } + + if (elementType.IsManagedType) + { + Error(diagnostics, ErrorCode.ERR_ManagedAddr, initializerSyntax, elementType); + hasErrors = true; } + initializerOpt = GetFixedLocalCollectionInitializer(initializerOpt, elementType, declType, hasErrors, diagnostics); return true; } diff --git a/src/Compilers/CSharp/Portable/CSharpResources.Designer.cs b/src/Compilers/CSharp/Portable/CSharpResources.Designer.cs index 4f5b1d799f80c..1f3352f6cbfc9 100644 --- a/src/Compilers/CSharp/Portable/CSharpResources.Designer.cs +++ b/src/Compilers/CSharp/Portable/CSharpResources.Designer.cs @@ -4282,6 +4282,15 @@ internal static string ERR_ExportedTypesConflict { } } + /// + /// Looks up a localized string similar to The given expression cannot be used in a fixed statement. + /// + internal static string ERR_ExprCannotBeFixed { + get { + return ResourceManager.GetString("ERR_ExprCannotBeFixed", resourceCulture); + } + } + /// /// Looks up a localized string similar to Expected expression. /// diff --git a/src/Compilers/CSharp/Portable/CSharpResources.resx b/src/Compilers/CSharp/Portable/CSharpResources.resx index 3178e2140048f..46cfc210746d7 100644 --- a/src/Compilers/CSharp/Portable/CSharpResources.resx +++ b/src/Compilers/CSharp/Portable/CSharpResources.resx @@ -927,6 +927,9 @@ You cannot use the fixed statement to take the address of an already fixed expression + + The given expression cannot be used in a fixed statement + Pointers and fixed size buffers may only be used in an unsafe context diff --git a/src/Compilers/CSharp/Portable/Errors/ErrorCode.cs b/src/Compilers/CSharp/Portable/Errors/ErrorCode.cs index 61e0310af8e86..c54a851d35f64 100644 --- a/src/Compilers/CSharp/Portable/Errors/ErrorCode.cs +++ b/src/Compilers/CSharp/Portable/Errors/ErrorCode.cs @@ -1558,5 +1558,7 @@ internal enum ErrorCode ERR_DoNotUseFixedBufferAttrOnProperty = 8372, // Note: you will need to re-generate compiler code after adding warnings (build\scripts\generate-compiler-code.cmd) + + ERR_ExprCannotBeFixed = 9365, } } diff --git a/src/Compilers/CSharp/Test/Emit/CodeGen/UnsafeTests.cs b/src/Compilers/CSharp/Test/Emit/CodeGen/UnsafeTests.cs index cd60ab45c49d8..1ddcf583fe003 100644 --- a/src/Compilers/CSharp/Test/Emit/CodeGen/UnsafeTests.cs +++ b/src/Compilers/CSharp/Test/Emit/CodeGen/UnsafeTests.cs @@ -3985,7 +3985,60 @@ .locals init (char* V_0, //p #endregion Fixed statement tests - #region Pointer conversion tests + #region Custom fixed statement tests + + [Fact(Skip = "NYI")] + public void SimpleCaseOfCustomFixed() + { + var text = @" +unsafe class C +{ + public static void Main() + { + fixed (int* p = new Fixable()) + { + System.Console.WriteLine(p[1]); + } + } + + class Fixable + { + public ref int DangerousGetPinnableReference() + { + return ref (new int[]{1,2,3})[0]; + } + } + +}"; + + var compVerifier = CompileAndVerify(text, options: TestOptions.UnsafeReleaseExe, expectedOutput: @"2", verify: Verification.Fails); + + compVerifier.VerifyIL("C.Main", @" +{ + // Code size 30 (0x1e) + .maxstack 3 + .locals init (pinned int& V_0) + IL_0000: newobj ""C..ctor()"" + IL_0005: dup + IL_0006: ldflda ""int C.x"" + IL_000b: stloc.0 + IL_000c: ldloc.0 + IL_000d: conv.u + IL_000e: ldc.i4.1 + IL_000f: stind.i4 + IL_0010: ldc.i4.0 + IL_0011: conv.u + IL_0012: stloc.0 + IL_0013: ldfld ""int C.x"" + IL_0018: call ""void System.Console.WriteLine(int)"" + IL_001d: ret +} +"); + } + + #endregion Custom fixed statement tests + + #region Pointer conversion tests [Fact] public void ConvertNullToPointer() diff --git a/src/Compilers/CSharp/Test/Semantic/Semantics/SemanticErrorTests.cs b/src/Compilers/CSharp/Test/Semantic/Semantics/SemanticErrorTests.cs index 28e1740755c8e..42f508e675f59 100644 --- a/src/Compilers/CSharp/Test/Semantic/Semantics/SemanticErrorTests.cs +++ b/src/Compilers/CSharp/Test/Semantic/Semantics/SemanticErrorTests.cs @@ -8019,10 +8019,10 @@ unsafe public static void Main() CreateStandardCompilation(text, options: TestOptions.UnsafeReleaseDll).VerifyDiagnostics( // (7,23): error CS0213: You cannot use the fixed statement to take the address of an already fixed expression // fixed (int *j = &i) { } // CS0213 - Diagnostic(ErrorCode.ERR_FixedNotNeeded, "&i"), - // (14,26): error CS0213: You cannot use the fixed statement to take the address of an already fixed expression + Diagnostic(ErrorCode.ERR_FixedNotNeeded, "&i").WithLocation(7, 23), + // (14,26): error CS9365: The given expression cannot be used in a fixed statement // fixed (int *c = b) { } // CS0213 - Diagnostic(ErrorCode.ERR_FixedNotNeeded, "b")); + Diagnostic(ErrorCode.ERR_ExprCannotBeFixed, "b").WithLocation(14, 26)); } [Fact] @@ -8734,9 +8734,9 @@ unsafe public static void Main() } "; CreateStandardCompilation(text, options: TestOptions.UnsafeReleaseDll).VerifyDiagnostics( - // (20,23): error CS0254: The right hand side of a fixed statement assignment may not be a cast expression + // (20,23): error CS9365: The given expression cannot be used in a fixed statement // fixed (int* p = (int*)&pt.x) // CS0254 - Diagnostic(ErrorCode.ERR_BadCastInFixed, "(int*)&pt.x")); + Diagnostic(ErrorCode.ERR_ExprCannotBeFixed, "(int*)&pt.x").WithLocation(20, 23)); } [Fact] diff --git a/src/Compilers/CSharp/Test/Semantic/Semantics/TargetTypedDefaultTests.cs b/src/Compilers/CSharp/Test/Semantic/Semantics/TargetTypedDefaultTests.cs index b7a25a32cb663..0eae006312b1e 100644 --- a/src/Compilers/CSharp/Test/Semantic/Semantics/TargetTypedDefaultTests.cs +++ b/src/Compilers/CSharp/Test/Semantic/Semantics/TargetTypedDefaultTests.cs @@ -1458,9 +1458,9 @@ static unsafe void Main() "; var comp = CreateStandardCompilation(source, parseOptions: TestOptions.Regular7_1, options: TestOptions.DebugExe.WithAllowUnsafe(true)); comp.VerifyDiagnostics( - // (6,26): error CS0213: You cannot use the fixed statement to take the address of an already fixed expression + // (6,26): error CS9365: The given expression cannot be used in a fixed statement // fixed (byte* p = default) - Diagnostic(ErrorCode.ERR_FixedNotNeeded, "default"), + Diagnostic(ErrorCode.ERR_ExprCannotBeFixed, "default").WithLocation(6, 26), // (9,27): error CS0211: Cannot take the address of the given expression // fixed (byte* p = &default) Diagnostic(ErrorCode.ERR_InvalidAddrOp, "default").WithLocation(9, 27) @@ -1947,9 +1947,9 @@ unsafe static void Main() // Confusing, but matches Dev10. CreateStandardCompilation(text, options: TestOptions.UnsafeReleaseDll, parseOptions: TestOptions.Regular7_1) .VerifyDiagnostics( - // (6,25): error CS0213: You cannot use the fixed statement to take the address of an already fixed expression + // (6,25): error CS9365: The given expression cannot be used in a fixed statement // fixed (int* p = default) - Diagnostic(ErrorCode.ERR_FixedNotNeeded, "default").WithLocation(6, 25) + Diagnostic(ErrorCode.ERR_ExprCannotBeFixed, "default").WithLocation(6, 25) ); } diff --git a/src/Compilers/CSharp/Test/Semantic/Semantics/UnsafeTests.cs b/src/Compilers/CSharp/Test/Semantic/Semantics/UnsafeTests.cs index 2ef4b96822138..3bca34d09be7c 100644 --- a/src/Compilers/CSharp/Test/Semantic/Semantics/UnsafeTests.cs +++ b/src/Compilers/CSharp/Test/Semantic/Semantics/UnsafeTests.cs @@ -6145,15 +6145,15 @@ class NotPointer } "; CreateStandardCompilation(text, options: TestOptions.UnsafeReleaseDll).VerifyDiagnostics( - // (9,26): error CS0254: The right hand side of a fixed statement assignment may not be a cast expression + // (9,26): error CS9365: The given expression cannot be used in a fixed statement // fixed (byte* p = (byte*)&x) - Diagnostic(ErrorCode.ERR_BadCastInFixed, "(byte*)&x").WithLocation(9, 26), - // (13,25): error CS0213: You cannot use the fixed statement to take the address of an already fixed expression + Diagnostic(ErrorCode.ERR_ExprCannotBeFixed, "(byte*)&x").WithLocation(9, 26), + // (13,25): error CS9365: The given expression cannot be used in a fixed statement // fixed (int* p = n) //CS0213 (confusing, but matches dev10) - Diagnostic(ErrorCode.ERR_FixedNotNeeded, "n").WithLocation(13, 25), - // (17,25): error CS0254: The right hand side of a fixed statement assignment may not be a cast expression + Diagnostic(ErrorCode.ERR_ExprCannotBeFixed, "n").WithLocation(13, 25), + // (17,25): error CS9365: The given expression cannot be used in a fixed statement // fixed (int* p = (int*)n) - Diagnostic(ErrorCode.ERR_BadCastInFixed, "(int*)n").WithLocation(17, 25), + Diagnostic(ErrorCode.ERR_ExprCannotBeFixed, "(int*)n").WithLocation(17, 25), // (5,23): warning CS0649: Field 'C.n' is never assigned to, and will always have its default value null // public NotPointer n; Diagnostic(ErrorCode.WRN_UnassignedInternalField, "n").WithArguments("C.n", "null").WithLocation(5, 23) @@ -6243,9 +6243,9 @@ unsafe static void Main() } "; CreateStandardCompilation(text, options: TestOptions.UnsafeReleaseDll).VerifyDiagnostics( - // (6,25): error CS0213: You cannot use the fixed statement to take the address of an already fixed expression - // fixed (int* p = stackalloc int[2]) - Diagnostic(ErrorCode.ERR_FixedNotNeeded, "stackalloc int[2]")); + // (6,25): error CS9365: The given expression cannot be used in a fixed statement + // fixed (int* p = stackalloc int[2]) //CS0213 - already fixed + Diagnostic(ErrorCode.ERR_ExprCannotBeFixed, "stackalloc int[2]").WithLocation(6, 25)); } [Fact] @@ -6315,9 +6315,9 @@ unsafe static void Main() "; // Confusing, but matches Dev10. CreateStandardCompilation(text, options: TestOptions.UnsafeReleaseDll).VerifyDiagnostics( - // (6,25): error CS0213: You cannot use the fixed statement to take the address of an already fixed expression + // (6,25): error CS9365: The given expression cannot be used in a fixed statement // fixed (int* p = null) - Diagnostic(ErrorCode.ERR_FixedNotNeeded, "null").WithLocation(6, 25), + Diagnostic(ErrorCode.ERR_ExprCannotBeFixed, "null").WithLocation(6, 25), // (10,26): error CS0211: Cannot take the address of the given expression // fixed (int* p = &null) Diagnostic(ErrorCode.ERR_InvalidAddrOp, "null").WithLocation(10, 26), @@ -6327,9 +6327,9 @@ unsafe static void Main() // (18,26): error CS0103: The name '_' does not exist in the current context // fixed (int* p = &_) Diagnostic(ErrorCode.ERR_NameNotInContext, "_").WithArguments("_").WithLocation(18, 26), - // (22,25): error CS1660: Cannot convert lambda expression to type 'int*' because it is not a delegate type + // (22,25): error CS9365: The given expression cannot be used in a fixed statement // fixed (int* p = ()=>throw null) - Diagnostic(ErrorCode.ERR_AnonMethToNonDel, "()=>throw null").WithArguments("lambda expression", "int*").WithLocation(22, 25), + Diagnostic(ErrorCode.ERR_ExprCannotBeFixed, "()=>throw null").WithLocation(22, 25), // (26,27): error CS0211: Cannot take the address of the given expression // fixed (int* p = &(()=>throw null)) Diagnostic(ErrorCode.ERR_InvalidAddrOp, "()=>throw null").WithLocation(26, 27) @@ -6351,9 +6351,9 @@ unsafe static void Main() } "; CreateStandardCompilation(text, options: TestOptions.UnsafeReleaseDll).VerifyDiagnostics( - // (6,26): error CS1660: Cannot convert lambda expression to type 'int*' because it is not a delegate type + // (6,26): error CS9365: The given expression cannot be used in a fixed statement // fixed (int* p = (x => x)) - Diagnostic(ErrorCode.ERR_AnonMethToNonDel, "x => x").WithArguments("lambda expression", "int*")); + Diagnostic(ErrorCode.ERR_ExprCannotBeFixed, "x => x").WithLocation(6, 26)); } [Fact] @@ -6371,9 +6371,9 @@ unsafe static void Main() } "; CreateStandardCompilation(text, options: TestOptions.UnsafeReleaseDll).VerifyDiagnostics( - // (6,25): error CS0428: Cannot convert method group 'Main' to non-delegate type 'int*'. Did you intend to invoke the method? + // (6,25): error CS9365: The given expression cannot be used in a fixed statement // fixed (int* p = Main) - Diagnostic(ErrorCode.ERR_MethGrpToNonDel, "Main").WithArguments("Main", "int*")); + Diagnostic(ErrorCode.ERR_ExprCannotBeFixed, "Main").WithLocation(6, 25)); } [Fact] From 7cb61a28144b237b72f474a182cf674500f65a98 Mon Sep 17 00:00:00 2001 From: vsadov Date: Wed, 24 Jan 2018 18:49:24 -0800 Subject: [PATCH 02/10] simple cases work --- .../Portable/Binder/Binder_Statements.cs | 79 +++++++++++-- .../CSharp/Portable/BoundTree/BoundNodes.xml | 3 + .../Portable/BoundTree/GenerateNodes.bat | 6 +- .../CSharp/Portable/CodeGen/EmitExpression.cs | 4 +- .../Generated/BoundNodes.xml.Generated.cs | 14 ++- .../LocalRewriter_ConditionalAccess.cs | 6 - .../LocalRewriter_FixedStatement.cs | 105 +++++++++++++++++- .../CSharp/Test/Emit/CodeGen/UnsafeTests.cs | 86 +++++++++++--- 8 files changed, 259 insertions(+), 44 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Binder/Binder_Statements.cs b/src/Compilers/CSharp/Portable/Binder/Binder_Statements.cs index 4f1b06d708022..afc27650e2b82 100644 --- a/src/Compilers/CSharp/Portable/Binder/Binder_Statements.cs +++ b/src/Compilers/CSharp/Portable/Binder/Binder_Statements.cs @@ -1059,6 +1059,7 @@ private bool IsValidFixedVariableInitializer(TypeSymbol declType, SourceLocalSym TypeSymbol elementType; bool hasErrors = false; + MethodSymbol getPinnableMethod = null; switch (initializerOpt.Kind) { @@ -1078,24 +1079,38 @@ private bool IsValidFixedVariableInitializer(TypeSymbol declType, SourceLocalSym default: // fixed (T* variable = ) ... - if (initializerType.SpecialType == SpecialType.System_String) - { - elementType = this.GetSpecialType(SpecialType.System_Char, diagnostics, initializerSyntax); - Debug.Assert(!elementType.IsManagedType); - } - else if (initializerType.IsArray()) + + // check for arrays + if (initializerType.IsArray()) { // See ExpressionBinder::BindPtrToArray (though most of that functionality is now in LocalRewriter). var arrayType = (ArrayTypeSymbol)initializerType; elementType = arrayType.ElementType; + break; } - else + + // check for "DangerousGetPinnableReference" + getPinnableMethod = GetDangerousGetPinnableReferenceMethod(initializerOpt); + + // check for String + // NOTE: We will allow DangerousGetPinnableReferenceMethod to take precendence, but only if it is a member of System.String + if (initializerType.SpecialType == SpecialType.System_String && + ((object)getPinnableMethod == null || getPinnableMethod.ContainingType.SpecialType == SpecialType.System_String)) + { + elementType = this.GetSpecialType(SpecialType.System_Char, diagnostics, initializerSyntax); + Debug.Assert(!elementType.IsManagedType); + break; + } + + // not a specially known type, check for getPinnableMethod + if (getPinnableMethod != null) { - Error(diagnostics, ErrorCode.ERR_ExprCannotBeFixed, initializerSyntax); - return false; + elementType = getPinnableMethod.ReturnType; + break; } - break; + Error(diagnostics, ErrorCode.ERR_ExprCannotBeFixed, initializerSyntax); + return false; } if (elementType.IsManagedType) @@ -1104,15 +1119,54 @@ private bool IsValidFixedVariableInitializer(TypeSymbol declType, SourceLocalSym hasErrors = true; } - initializerOpt = GetFixedLocalCollectionInitializer(initializerOpt, elementType, declType, hasErrors, diagnostics); + initializerOpt = GetFixedLocalCollectionInitializer(initializerOpt, elementType, declType, getPinnableMethod, hasErrors, diagnostics); return true; } + private MethodSymbol GetDangerousGetPinnableReferenceMethod(BoundExpression initializer) + { + if (initializer.Type.SpecialType == SpecialType.System_Void) + { + return null; + } + + DiagnosticBag diagnostics = DiagnosticBag.GetInstance(); + var getPinnableReferenceCall = MakeInvocationExpression(initializer.Syntax, initializer, "DangerousGetPinnableReference", ImmutableArray.Empty, diagnostics); + diagnostics.Free(); + + if (getPinnableReferenceCall.HasAnyErrors) + { + return null; + } + + if (getPinnableReferenceCall.Kind != BoundKind.Call) + { + return null; + } + + var getPinnableReferenceMethod = ((BoundCall)getPinnableReferenceCall).Method; + if (getPinnableReferenceMethod is ErrorMethodSymbol || + HasOptionalOrVariableParameters(getPinnableReferenceMethod) || // We might have been able to resolve an overload with optional parameters, so check for that here + getPinnableReferenceMethod.ReturnsVoid || + !getPinnableReferenceMethod.RefKind.IsManagedReference()) + { + return null; + } + + return getPinnableReferenceMethod; + } + /// /// Wrap the initializer in a BoundFixedLocalCollectionInitializer so that the rewriter will have the /// information it needs (e.g. conversions, helper methods). /// - private BoundExpression GetFixedLocalCollectionInitializer(BoundExpression initializer, TypeSymbol elementType, TypeSymbol declType, bool hasErrors, DiagnosticBag diagnostics) + private BoundExpression GetFixedLocalCollectionInitializer( + BoundExpression initializer, + TypeSymbol elementType, + TypeSymbol declType, + MethodSymbol getPinnableMethodOpt, + bool hasErrors, + DiagnosticBag diagnostics) { Debug.Assert(initializer != null); @@ -1134,6 +1188,7 @@ private BoundExpression GetFixedLocalCollectionInitializer(BoundExpression initi pointerType, elementConversion, initializer, + getPinnableMethodOpt, declType, hasErrors); } diff --git a/src/Compilers/CSharp/Portable/BoundTree/BoundNodes.xml b/src/Compilers/CSharp/Portable/BoundTree/BoundNodes.xml index 560e88a72f770..ba738edbc8992 100644 --- a/src/Compilers/CSharp/Portable/BoundTree/BoundNodes.xml +++ b/src/Compilers/CSharp/Portable/BoundTree/BoundNodes.xml @@ -598,6 +598,9 @@ + + + diff --git a/src/Compilers/CSharp/Portable/BoundTree/GenerateNodes.bat b/src/Compilers/CSharp/Portable/BoundTree/GenerateNodes.bat index 184e81437cb50..4bb735b8cc5f5 100644 --- a/src/Compilers/CSharp/Portable/BoundTree/GenerateNodes.bat +++ b/src/Compilers/CSharp/Portable/BoundTree/GenerateNodes.bat @@ -1,7 +1,7 @@ REM Copyright (c) Microsoft. All Rights Reserved. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. REM Run the node generator to create new bound tree node definitions. -REM You must run this in the directory containing Generated.cs. -call tf checkout Generated.cs +REM You must run this in the directory containing BoundNodes.xml + @echo on -..\..\..\..\..\Binaries\Debug\BoundTreeGenerator.exe CSharp BoundNodes.xml Generated.cs +dotnet ..\..\..\..\..\Binaries\Debug\Exes\CompilersBoundTreeGenerator\BoundTreeGenerator.dll CSharp BoundNodes.xml ..\Generated\BoundNodes.xml.Generated.cs diff --git a/src/Compilers/CSharp/Portable/CodeGen/EmitExpression.cs b/src/Compilers/CSharp/Portable/CodeGen/EmitExpression.cs index 01dc17ea262b3..cdb13d66d5c34 100644 --- a/src/Compilers/CSharp/Portable/CodeGen/EmitExpression.cs +++ b/src/Compilers/CSharp/Portable/CodeGen/EmitExpression.cs @@ -359,9 +359,9 @@ private void EmitLoweredConditionalAccessExpression(BoundLoweredConditionalAcces (receiverType.IsNullableType() && expression.HasValueMethodOpt != null), "conditional receiver cannot be a struct"); var receiverConstant = receiver.ConstantValue; - if (receiverConstant != null) + if (receiverConstant?.IsNull == false) { - // const but not default, must be a reference type + // const but not null, must be a reference type Debug.Assert(receiverType.IsVerifierReference()); // receiver is a reference type, so addresskind does not matter, but we do not intend to write. receiverTemp = EmitReceiverRef(receiver, AddressKind.ReadOnly); diff --git a/src/Compilers/CSharp/Portable/Generated/BoundNodes.xml.Generated.cs b/src/Compilers/CSharp/Portable/Generated/BoundNodes.xml.Generated.cs index e3dad0c56ab48..d628e0cabc756 100644 --- a/src/Compilers/CSharp/Portable/Generated/BoundNodes.xml.Generated.cs +++ b/src/Compilers/CSharp/Portable/Generated/BoundNodes.xml.Generated.cs @@ -2038,7 +2038,7 @@ public BoundArgListOperator Update(ImmutableArray arguments, Im internal sealed partial class BoundFixedLocalCollectionInitializer : BoundExpression { - public BoundFixedLocalCollectionInitializer(SyntaxNode syntax, TypeSymbol elementPointerType, Conversion elementPointerTypeConversion, BoundExpression expression, TypeSymbol type, bool hasErrors = false) + public BoundFixedLocalCollectionInitializer(SyntaxNode syntax, TypeSymbol elementPointerType, Conversion elementPointerTypeConversion, BoundExpression expression, MethodSymbol getPinnableOpt, TypeSymbol type, bool hasErrors = false) : base(BoundKind.FixedLocalCollectionInitializer, syntax, type, hasErrors || expression.HasErrors()) { @@ -2049,6 +2049,7 @@ public BoundFixedLocalCollectionInitializer(SyntaxNode syntax, TypeSymbol elemen this.ElementPointerType = elementPointerType; this.ElementPointerTypeConversion = elementPointerTypeConversion; this.Expression = expression; + this.GetPinnableOpt = getPinnableOpt; } @@ -2058,16 +2059,18 @@ public BoundFixedLocalCollectionInitializer(SyntaxNode syntax, TypeSymbol elemen public BoundExpression Expression { get; } + public MethodSymbol GetPinnableOpt { get; } + public override BoundNode Accept(BoundTreeVisitor visitor) { return visitor.VisitFixedLocalCollectionInitializer(this); } - public BoundFixedLocalCollectionInitializer Update(TypeSymbol elementPointerType, Conversion elementPointerTypeConversion, BoundExpression expression, TypeSymbol type) + public BoundFixedLocalCollectionInitializer Update(TypeSymbol elementPointerType, Conversion elementPointerTypeConversion, BoundExpression expression, MethodSymbol getPinnableOpt, TypeSymbol type) { - if (elementPointerType != this.ElementPointerType || elementPointerTypeConversion != this.ElementPointerTypeConversion || expression != this.Expression || type != this.Type) + if (elementPointerType != this.ElementPointerType || elementPointerTypeConversion != this.ElementPointerTypeConversion || expression != this.Expression || getPinnableOpt != this.GetPinnableOpt || type != this.Type) { - var result = new BoundFixedLocalCollectionInitializer(this.Syntax, elementPointerType, elementPointerTypeConversion, expression, type, this.HasErrors); + var result = new BoundFixedLocalCollectionInitializer(this.Syntax, elementPointerType, elementPointerTypeConversion, expression, getPinnableOpt, type, this.HasErrors); result.WasCompilerGenerated = this.WasCompilerGenerated; return result; } @@ -8722,7 +8725,7 @@ public override BoundNode VisitFixedLocalCollectionInitializer(BoundFixedLocalCo BoundExpression expression = (BoundExpression)this.Visit(node.Expression); TypeSymbol elementPointerType = this.VisitType(node.ElementPointerType); TypeSymbol type = this.VisitType(node.Type); - return node.Update(elementPointerType, node.ElementPointerTypeConversion, expression, type); + return node.Update(elementPointerType, node.ElementPointerTypeConversion, expression, node.GetPinnableOpt, type); } public override BoundNode VisitSequencePoint(BoundSequencePoint node) { @@ -9817,6 +9820,7 @@ public override TreeDumperNode VisitFixedLocalCollectionInitializer(BoundFixedLo new TreeDumperNode("elementPointerType", node.ElementPointerType, null), new TreeDumperNode("elementPointerTypeConversion", node.ElementPointerTypeConversion, null), new TreeDumperNode("expression", null, new TreeDumperNode[] { Visit(node.Expression, null) }), + new TreeDumperNode("getPinnableOpt", node.GetPinnableOpt, null), new TreeDumperNode("type", node.Type, null) } ); diff --git a/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_ConditionalAccess.cs b/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_ConditionalAccess.cs index 30ba8e8283e26..6f3d0400420e2 100644 --- a/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_ConditionalAccess.cs +++ b/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_ConditionalAccess.cs @@ -73,7 +73,6 @@ internal BoundExpression RewriteConditionalAccess(BoundConditionalAccess node, b var currentConditionalAccessID = ++_currentConditionalAccessID; LocalSymbol temp = null; - BoundExpression unconditionalAccess = null; switch (loweringKind) { @@ -130,11 +129,6 @@ internal BoundExpression RewriteConditionalAccess(BoundConditionalAccess node, b { Debug.Assert(accessExpressionType == nodeType.GetNullableUnderlyingType()); loweredAccessExpression = _factory.New((NamedTypeSymbol)nodeType, loweredAccessExpression); - - if (unconditionalAccess != null) - { - unconditionalAccess = _factory.New((NamedTypeSymbol)nodeType, unconditionalAccess); - } } else { diff --git a/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_FixedStatement.cs b/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_FixedStatement.cs index 810c2740ec5e3..4e796416f58fa 100644 --- a/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_FixedStatement.cs +++ b/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_FixedStatement.cs @@ -205,7 +205,11 @@ private BoundStatement InitializeFixedStatementLocal( LocalSymbol localSymbol = localDecl.LocalSymbol; var fixedCollectionInitializer = (BoundFixedLocalCollectionInitializer)initializer; - if (fixedCollectionInitializer.Expression.Type.SpecialType == SpecialType.System_String) + if ((object)fixedCollectionInitializer.GetPinnableOpt != null) + { + return InitializeFixedStatementGetPinnable(localDecl, localSymbol, fixedCollectionInitializer, factory, out pinnedTemp); + } + else if(fixedCollectionInitializer.Expression.Type.SpecialType == SpecialType.System_String) { return InitializeFixedStatementStringLocal(localDecl, localSymbol, fixedCollectionInitializer, factory, out pinnedTemp); } @@ -286,6 +290,105 @@ private BoundStatement InitializeFixedStatementRegularLocal( return factory.Block(pinnedTempInit, localInit); } + /// + /// fixed(int* ptr = &v){ ... } == becomes ===> + /// + /// pinned ref int pinnedTemp = ref v; // pinning managed ref + /// int* ptr = (int*)&pinnedTemp; // unsafe cast to unmanaged ptr + /// . . . + /// + private BoundStatement InitializeFixedStatementGetPinnable( + BoundLocalDeclaration localDecl, + LocalSymbol localSymbol, + BoundFixedLocalCollectionInitializer fixedInitializer, + SyntheticBoundNodeFactory factory, + out LocalSymbol pinnedTemp) + { + TypeSymbol localType = localSymbol.Type; + BoundExpression initializerExpr = VisitExpression(fixedInitializer.Expression); + + // intervening parens may have been skipped by the binder; find the declarator + VariableDeclaratorSyntax declarator = fixedInitializer.Syntax.FirstAncestorOrSelf(); + Debug.Assert(declarator != null); + + var initializerType = initializerExpr.Type; + var initializerSyntax = initializerExpr.Syntax; + var getPinnableMethod = fixedInitializer.GetPinnableOpt; + var tempType = getPinnableMethod.ReturnType; + + pinnedTemp = factory.SynthesizedLocal( + tempType, + syntax: declarator, + isPinned: true, + //NOTE: different from the array and string cases + // RefReadOnly to allow referring to readonly variables. (technically we only "read" through the temp anyways) + refKind: RefKind.RefReadOnly, + kind: SynthesizedLocalKind.FixedReference); + + // NOTE: we pin the reference, not the pointer. + Debug.Assert(pinnedTemp.IsPinned); + Debug.Assert(!localSymbol.IsPinned); + + //TODO: VS + // optimize for a null receiver + // handle nonnulable case (no conditional) + // binding for nullable + + // pinnedTemp = ref v?.GetPinnable ?? ref *default(T*) ; + var currentConditionalAccessID = _currentConditionalAccessID++; + + var conditionalReceiver = new BoundConditionalReceiver( + initializerSyntax, + currentConditionalAccessID, + initializerType); + + BoundExpression loweredAccessExpression; + + // .GetPinnable() + loweredAccessExpression = getPinnableMethod.IsStatic? + factory.Call(null, getPinnableMethod, conditionalReceiver): + factory.Call(conditionalReceiver, getPinnableMethod); + + // temp =ref .GetPinnable() + loweredAccessExpression = factory.AssignmentExpression( + factory.Local(pinnedTemp), + loweredAccessExpression, + isRef: true); + + // &pinnedTemp; + var addr = new BoundAddressOfOperator( + factory.Syntax, + factory.Local(pinnedTemp), + type: fixedInitializer.ElementPointerType); + + // (int*)&pinnedTemp + var pointerValue = factory.Convert( + localType, + addr, + fixedInitializer.ElementPointerTypeConversion); + + // {temp =ref .GetPinnable(), (int*)&pinnedTemp} + loweredAccessExpression = factory.Sequence(locals: ImmutableArray.Empty, sideEffects: ImmutableArray.Create(loweredAccessExpression), result: pointerValue); + + + // initializer?.{temp =ref .GetPinnable(), (int*)&pinnedTemp} ?? default; + var conditionalPointerValue = new BoundLoweredConditionalAccess( + initializerSyntax, + initializerExpr, + initializerType.IsNullableType() ? + UnsafeGetNullableMethod(initializerSyntax, initializerType, SpecialMember.System_Nullable_T_get_HasValue) : + null, + whenNotNull: loweredAccessExpression, + whenNullOpt: null, // just return default(T*) + currentConditionalAccessID, + localType); + + // ptr = initializer?.{temp =ref .GetPinnable(), (int*)&pinnedTemp} ?? default; + BoundStatement localInit = InstrumentLocalDeclarationIfNecessary(localDecl, localSymbol, factory.Assignment(factory.Local(localSymbol), conditionalPointerValue)); + + return localInit; + } + /// /// fixed(char* ptr = stringVar){ ... } == becomes ===> /// diff --git a/src/Compilers/CSharp/Test/Emit/CodeGen/UnsafeTests.cs b/src/Compilers/CSharp/Test/Emit/CodeGen/UnsafeTests.cs index 1ddcf583fe003..9fcb4bd170191 100644 --- a/src/Compilers/CSharp/Test/Emit/CodeGen/UnsafeTests.cs +++ b/src/Compilers/CSharp/Test/Emit/CodeGen/UnsafeTests.cs @@ -3987,7 +3987,7 @@ .locals init (char* V_0, //p #region Custom fixed statement tests - [Fact(Skip = "NYI")] + [Fact] public void SimpleCaseOfCustomFixed() { var text = @" @@ -4015,30 +4015,86 @@ public ref int DangerousGetPinnableReference() compVerifier.VerifyIL("C.Main", @" { - // Code size 30 (0x1e) - .maxstack 3 + // Code size 33 (0x21) + .maxstack 2 .locals init (pinned int& V_0) - IL_0000: newobj ""C..ctor()"" + IL_0000: newobj ""C.Fixable..ctor()"" IL_0005: dup - IL_0006: ldflda ""int C.x"" - IL_000b: stloc.0 - IL_000c: ldloc.0 - IL_000d: conv.u - IL_000e: ldc.i4.1 - IL_000f: stind.i4 - IL_0010: ldc.i4.0 - IL_0011: conv.u + IL_0006: brtrue.s IL_000d + IL_0008: pop + IL_0009: ldc.i4.0 + IL_000a: conv.u + IL_000b: br.s IL_0015 + IL_000d: call ""ref int C.Fixable.DangerousGetPinnableReference()"" IL_0012: stloc.0 - IL_0013: ldfld ""int C.x"" + IL_0013: ldloc.0 + IL_0014: conv.u + IL_0015: ldc.i4.4 + IL_0016: add + IL_0017: ldind.i4 IL_0018: call ""void System.Console.WriteLine(int)"" - IL_001d: ret + IL_001d: ldc.i4.0 + IL_001e: conv.u + IL_001f: stloc.0 + IL_0020: ret +} +"); + } + + [Fact] + public void SimpleCaseOfCustomFixedNull() + { + var text = @" +unsafe class C +{ + public static void Main() + { + fixed (int* p = (Fixable)null) + { + System.Console.WriteLine((int)p); + } + } + + class Fixable + { + public ref int DangerousGetPinnableReference() + { + return ref (new int[]{1,2,3})[0]; + } + } + +}"; + + var compVerifier = CompileAndVerify(text, options: TestOptions.UnsafeReleaseExe, expectedOutput: @"0", verify: Verification.Fails); + + compVerifier.VerifyIL("C.Main", @" +{ + // Code size 26 (0x1a) + .maxstack 1 + .locals init (pinned int& V_0) + IL_0000: ldnull + IL_0001: brtrue.s IL_0007 + IL_0003: ldc.i4.0 + IL_0004: conv.u + IL_0005: br.s IL_0010 + IL_0007: ldnull + IL_0008: call ""ref int C.Fixable.DangerousGetPinnableReference()"" + IL_000d: stloc.0 + IL_000e: ldloc.0 + IL_000f: conv.u + IL_0010: conv.i4 + IL_0011: call ""void System.Console.WriteLine(int)"" + IL_0016: ldc.i4.0 + IL_0017: conv.u + IL_0018: stloc.0 + IL_0019: ret } "); } #endregion Custom fixed statement tests - #region Pointer conversion tests + #region Pointer conversion tests [Fact] public void ConvertNullToPointer() From 3ff7674befeaaa81d88992fd11cde276fbe42f47 Mon Sep 17 00:00:00 2001 From: vsadov Date: Wed, 24 Jan 2018 20:00:39 -0800 Subject: [PATCH 03/10] Support structs and generics --- .../LocalRewriter_FixedStatement.cs | 93 +++++++------ .../CSharp/Test/Emit/CodeGen/UnsafeTests.cs | 128 ++++++++++++++++++ 2 files changed, 178 insertions(+), 43 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_FixedStatement.cs b/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_FixedStatement.cs index 4e796416f58fa..552f2bb3ff4aa 100644 --- a/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_FixedStatement.cs +++ b/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_FixedStatement.cs @@ -307,17 +307,16 @@ private BoundStatement InitializeFixedStatementGetPinnable( TypeSymbol localType = localSymbol.Type; BoundExpression initializerExpr = VisitExpression(fixedInitializer.Expression); - // intervening parens may have been skipped by the binder; find the declarator - VariableDeclaratorSyntax declarator = fixedInitializer.Syntax.FirstAncestorOrSelf(); - Debug.Assert(declarator != null); - var initializerType = initializerExpr.Type; var initializerSyntax = initializerExpr.Syntax; var getPinnableMethod = fixedInitializer.GetPinnableOpt; - var tempType = getPinnableMethod.ReturnType; + + // intervening parens may have been skipped by the binder; find the declarator + VariableDeclaratorSyntax declarator = fixedInitializer.Syntax.FirstAncestorOrSelf(); + Debug.Assert(declarator != null); pinnedTemp = factory.SynthesizedLocal( - tempType, + getPinnableMethod.ReturnType, syntax: declarator, isPinned: true, //NOTE: different from the array and string cases @@ -325,41 +324,44 @@ private BoundStatement InitializeFixedStatementGetPinnable( refKind: RefKind.RefReadOnly, kind: SynthesizedLocalKind.FixedReference); - // NOTE: we pin the reference, not the pointer. - Debug.Assert(pinnedTemp.IsPinned); - Debug.Assert(!localSymbol.IsPinned); - //TODO: VS - // optimize for a null receiver - // handle nonnulable case (no conditional) // binding for nullable + // error on ref extensions vs. not writeable receiver - // pinnedTemp = ref v?.GetPinnable ?? ref *default(T*) ; - var currentConditionalAccessID = _currentConditionalAccessID++; + BoundExpression callReceiver; + int currentConditionalAccessID = 0; - var conditionalReceiver = new BoundConditionalReceiver( - initializerSyntax, - currentConditionalAccessID, - initializerType); + bool needNullCheck = !initializerType.IsValueType || initializerType.IsNullableType(); - BoundExpression loweredAccessExpression; + if (needNullCheck) + { + currentConditionalAccessID = _currentConditionalAccessID++; + callReceiver = new BoundConditionalReceiver( + initializerSyntax, + currentConditionalAccessID, + initializerType); + } + else + { + callReceiver = initializerExpr; + } // .GetPinnable() - loweredAccessExpression = getPinnableMethod.IsStatic? - factory.Call(null, getPinnableMethod, conditionalReceiver): - factory.Call(conditionalReceiver, getPinnableMethod); + var getPinnableCall = getPinnableMethod.IsStatic? + factory.Call(null, getPinnableMethod, callReceiver): + factory.Call(callReceiver, getPinnableMethod); // temp =ref .GetPinnable() - loweredAccessExpression = factory.AssignmentExpression( - factory.Local(pinnedTemp), - loweredAccessExpression, - isRef: true); + var tempAssignment = factory.AssignmentExpression( + factory.Local(pinnedTemp), + getPinnableCall, + isRef: true); // &pinnedTemp; var addr = new BoundAddressOfOperator( factory.Syntax, - factory.Local(pinnedTemp), - type: fixedInitializer.ElementPointerType); + factory.Local(pinnedTemp), + type: fixedInitializer.ElementPointerType); // (int*)&pinnedTemp var pointerValue = factory.Convert( @@ -367,24 +369,29 @@ private BoundStatement InitializeFixedStatementGetPinnable( addr, fixedInitializer.ElementPointerTypeConversion); - // {temp =ref .GetPinnable(), (int*)&pinnedTemp} - loweredAccessExpression = factory.Sequence(locals: ImmutableArray.Empty, sideEffects: ImmutableArray.Create(loweredAccessExpression), result: pointerValue); - + // {pinnedTemp =ref .GetPinnable(), (int*)&pinnedTemp} + BoundExpression pinAndGetPtr = factory.Sequence( + locals: ImmutableArray.Empty, + sideEffects: ImmutableArray.Create(tempAssignment), + result: pointerValue); - // initializer?.{temp =ref .GetPinnable(), (int*)&pinnedTemp} ?? default; - var conditionalPointerValue = new BoundLoweredConditionalAccess( - initializerSyntax, - initializerExpr, - initializerType.IsNullableType() ? - UnsafeGetNullableMethod(initializerSyntax, initializerType, SpecialMember.System_Nullable_T_get_HasValue) : - null, - whenNotNull: loweredAccessExpression, - whenNullOpt: null, // just return default(T*) - currentConditionalAccessID, - localType); + if (needNullCheck) + { + // initializer?.{temp =ref .GetPinnable(), (int*)&pinnedTemp} ?? default; + pinAndGetPtr = new BoundLoweredConditionalAccess( + initializerSyntax, + initializerExpr, + initializerType.IsNullableType() ? + UnsafeGetNullableMethod(initializerSyntax, initializerType, SpecialMember.System_Nullable_T_get_HasValue) : + null, + whenNotNull: pinAndGetPtr, + whenNullOpt: null, // just return default(T*) + currentConditionalAccessID, + localType); + } // ptr = initializer?.{temp =ref .GetPinnable(), (int*)&pinnedTemp} ?? default; - BoundStatement localInit = InstrumentLocalDeclarationIfNecessary(localDecl, localSymbol, factory.Assignment(factory.Local(localSymbol), conditionalPointerValue)); + BoundStatement localInit = InstrumentLocalDeclarationIfNecessary(localDecl, localSymbol, factory.Assignment(factory.Local(localSymbol), pinAndGetPtr)); return localInit; } diff --git a/src/Compilers/CSharp/Test/Emit/CodeGen/UnsafeTests.cs b/src/Compilers/CSharp/Test/Emit/CodeGen/UnsafeTests.cs index 9fcb4bd170191..26ea1b1ba7618 100644 --- a/src/Compilers/CSharp/Test/Emit/CodeGen/UnsafeTests.cs +++ b/src/Compilers/CSharp/Test/Emit/CodeGen/UnsafeTests.cs @@ -4092,6 +4092,134 @@ .locals init (pinned int& V_0) "); } + [Fact] + public void SimpleCaseOfCustomFixedStruct() + { + var text = @" +unsafe class C +{ + public static void Main() + { + fixed (int* p = new Fixable()) + { + System.Console.WriteLine(p[1]); + } + } + + struct Fixable + { + public ref int DangerousGetPinnableReference() + { + return ref (new int[]{1,2,3})[0]; + } + } + +}"; + + var compVerifier = CompileAndVerify(text, options: TestOptions.UnsafeReleaseExe, expectedOutput: @"2", verify: Verification.Fails); + + compVerifier.VerifyIL("C.Main", @" +{ + // Code size 29 (0x1d) + .maxstack 2 + .locals init (pinned int& V_0, + C.Fixable V_1) + IL_0000: ldloca.s V_1 + IL_0002: dup + IL_0003: initobj ""C.Fixable"" + IL_0009: call ""ref int C.Fixable.DangerousGetPinnableReference()"" + IL_000e: stloc.0 + IL_000f: ldloc.0 + IL_0010: conv.u + IL_0011: ldc.i4.4 + IL_0012: add + IL_0013: ldind.i4 + IL_0014: call ""void System.Console.WriteLine(int)"" + IL_0019: ldc.i4.0 + IL_001a: conv.u + IL_001b: stloc.0 + IL_001c: ret +} +"); + } + + [Fact] + public void SimpleCaseOfCustomFixedGeneric() + { + var text = @" +unsafe class C +{ + public static void Main() + { + Test(42); + Test((object)null); + } + + public static void Test(T arg) + { + fixed (int* p = arg) + { + System.Console.Write(p == null? 0: p[1]); + } + } +} + +static class FixAllExt +{ + public static ref int DangerousGetPinnableReference(this T dummy) + { + return ref (new int[]{1,2,3})[0]; + } +} +"; + + var compVerifier = CompileAndVerify(text, additionalRefs: new[] { ExtensionAssemblyRef }, options: TestOptions.UnsafeReleaseExe, expectedOutput: @"20", verify: Verification.Fails); + + compVerifier.VerifyIL("C.Test(T)", @" +{ + // Code size 49 (0x31) + .maxstack 2 + .locals init (int* V_0, //p + pinned int& V_1) + IL_0000: ldarg.0 + IL_0001: box ""T"" + IL_0006: brtrue.s IL_000c + IL_0008: ldc.i4.0 + IL_0009: conv.u + IL_000a: br.s IL_001b + IL_000c: ldarga.s V_0 + IL_000e: ldobj ""T"" + IL_0013: call ""ref int FixAllExt.DangerousGetPinnableReference(T)"" + IL_0018: stloc.1 + IL_0019: ldloc.1 + IL_001a: conv.u + IL_001b: stloc.0 + IL_001c: ldloc.0 + IL_001d: ldc.i4.0 + IL_001e: conv.u + IL_001f: beq.s IL_0027 + IL_0021: ldloc.0 + IL_0022: ldc.i4.4 + IL_0023: add + IL_0024: ldind.i4 + IL_0025: br.s IL_0028 + IL_0027: ldc.i4.0 + IL_0028: call ""void System.Console.Write(int)"" + IL_002d: ldc.i4.0 + IL_002e: conv.u + IL_002f: stloc.1 + IL_0030: ret +} +"); + } + + // TODO: VS: + // struct + sideeffects + // interfaces + generic sideeffects + // in extension + // in generic + // ref extension + #endregion Custom fixed statement tests #region Pointer conversion tests From 2b88c1d5dfd69de35fbc645518a8a60a0de4d0d8 Mon Sep 17 00:00:00 2001 From: vsadov Date: Thu, 25 Jan 2018 22:39:09 -0800 Subject: [PATCH 04/10] more tests --- .../CSharp/Test/Emit/CodeGen/UnsafeTests.cs | 222 +++++++++++++++++- 1 file changed, 220 insertions(+), 2 deletions(-) diff --git a/src/Compilers/CSharp/Test/Emit/CodeGen/UnsafeTests.cs b/src/Compilers/CSharp/Test/Emit/CodeGen/UnsafeTests.cs index 26ea1b1ba7618..7fbfc5cd2fe4f 100644 --- a/src/Compilers/CSharp/Test/Emit/CodeGen/UnsafeTests.cs +++ b/src/Compilers/CSharp/Test/Emit/CodeGen/UnsafeTests.cs @@ -4213,9 +4213,227 @@ .locals init (int* V_0, //p "); } + [Fact] + public void CustomFixedStructSideeffects() + { + var text = @" + unsafe class C + { + public static void Main() + { + var b = new FixableStruct(); + Test(ref b); + System.Console.WriteLine(b.x); + } + + public static void Test(ref FixableStruct arg) + { + fixed (int* p = arg) + { + System.Console.Write(p[1]); + } + } + } + + struct FixableStruct + { + public int x; + + public ref int DangerousGetPinnableReference() + { + x = 456; + return ref (new int[] { 4, 5, 6 })[0]; + } + } +"; + + var compVerifier = CompileAndVerify(text, options: TestOptions.UnsafeReleaseExe, verify: Verification.Fails, expectedOutput: @"5456"); + + compVerifier.VerifyIL("C.Test(ref FixableStruct)", @" +{ + // Code size 21 (0x15) + .maxstack 2 + .locals init (pinned int& V_0) + IL_0000: ldarg.0 + IL_0001: call ""ref int FixableStruct.DangerousGetPinnableReference()"" + IL_0006: stloc.0 + IL_0007: ldloc.0 + IL_0008: conv.u + IL_0009: ldc.i4.4 + IL_000a: add + IL_000b: ldind.i4 + IL_000c: call ""void System.Console.Write(int)"" + IL_0011: ldc.i4.0 + IL_0012: conv.u + IL_0013: stloc.0 + IL_0014: ret +} +"); + } + + [Fact] + public void CustomFixedClassSideeffects() + { + var text = @" + unsafe class C + { + public static void Main() + { + var b = new FixableClass(); + Test(ref b); + System.Console.WriteLine(b.x); + } + + public static void Test(ref FixableClass arg) + { + fixed (int* p = arg) + { + System.Console.Write(p[1]); + } + } + } + + class FixableClass + { + public int x; + + public ref int DangerousGetPinnableReference() + { + x = 456; + return ref (new int[] { 4, 5, 6 })[0]; + } + } +"; + + var compVerifier = CompileAndVerify(text, options: TestOptions.UnsafeReleaseExe, verify: Verification.Fails, expectedOutput: @"5456"); + + // note that defensive copy is created + compVerifier.VerifyIL("C.Test(ref FixableClass)", @" +{ + // Code size 30 (0x1e) + .maxstack 2 + .locals init (pinned int& V_0) + IL_0000: ldarg.0 + IL_0001: ldind.ref + IL_0002: dup + IL_0003: brtrue.s IL_000a + IL_0005: pop + IL_0006: ldc.i4.0 + IL_0007: conv.u + IL_0008: br.s IL_0012 + IL_000a: call ""ref int FixableClass.DangerousGetPinnableReference()"" + IL_000f: stloc.0 + IL_0010: ldloc.0 + IL_0011: conv.u + IL_0012: ldc.i4.4 + IL_0013: add + IL_0014: ldind.i4 + IL_0015: call ""void System.Console.Write(int)"" + IL_001a: ldc.i4.0 + IL_001b: conv.u + IL_001c: stloc.0 + IL_001d: ret +} +"); + } + + [Fact] + public void CustomFixedGenericSideeffects() + { + var text = @" + unsafe class C + { + public static void Main() + { + var a = new FixableClass(); + Test(ref a); + System.Console.WriteLine(a.x); + var b = new FixableStruct(); + Test(ref b); + System.Console.WriteLine(b.x); + } + + public static void Test(ref T arg) where T: IFixable + { + fixed (int* p = arg) + { + System.Console.Write(p[1]); + } + } + } + + interface IFixable + { + ref int DangerousGetPinnableReference(); + } + + class FixableClass : IFixable + { + + public int x; + + public ref int DangerousGetPinnableReference() + { + x = 123; + return ref (new int[] { 1, 2, 3 })[0]; + } + } + + struct FixableStruct : IFixable + { + public int x; + + public ref int DangerousGetPinnableReference() + { + x = 456; + return ref (new int[] { 4, 5, 6 })[0]; + } + } +"; + + var compVerifier = CompileAndVerify(text, options: TestOptions.UnsafeReleaseExe, verify: Verification.Fails, expectedOutput: @"2123 +5456"); + + compVerifier.VerifyIL("C.Test(ref T)", @" +{ + // Code size 64 (0x40) + .maxstack 2 + .locals init (pinned int& V_0, + T V_1) + IL_0000: ldarg.0 + IL_0001: ldloca.s V_1 + IL_0003: initobj ""T"" + IL_0009: ldloc.1 + IL_000a: box ""T"" + IL_000f: brtrue.s IL_0026 + IL_0011: ldobj ""T"" + IL_0016: stloc.1 + IL_0017: ldloca.s V_1 + IL_0019: ldloc.1 + IL_001a: box ""T"" + IL_001f: brtrue.s IL_0026 + IL_0021: pop + IL_0022: ldc.i4.0 + IL_0023: conv.u + IL_0024: br.s IL_0034 + IL_0026: constrained. ""T"" + IL_002c: callvirt ""ref int IFixable.DangerousGetPinnableReference()"" + IL_0031: stloc.0 + IL_0032: ldloc.0 + IL_0033: conv.u + IL_0034: ldc.i4.4 + IL_0035: add + IL_0036: ldind.i4 + IL_0037: call ""void System.Console.Write(int)"" + IL_003c: ldc.i4.0 + IL_003d: conv.u + IL_003e: stloc.0 + IL_003f: ret +} +"); + } + // TODO: VS: - // struct + sideeffects - // interfaces + generic sideeffects // in extension // in generic // ref extension From 3097ec9982dfc51e0cd2cff6225afa4f1a191253 Mon Sep 17 00:00:00 2001 From: vsadov Date: Thu, 25 Jan 2018 22:50:05 -0800 Subject: [PATCH 05/10] resources --- src/Compilers/CSharp/Portable/xlf/CSharpResources.cs.xlf | 5 +++++ src/Compilers/CSharp/Portable/xlf/CSharpResources.de.xlf | 5 +++++ src/Compilers/CSharp/Portable/xlf/CSharpResources.es.xlf | 5 +++++ src/Compilers/CSharp/Portable/xlf/CSharpResources.fr.xlf | 5 +++++ src/Compilers/CSharp/Portable/xlf/CSharpResources.it.xlf | 5 +++++ src/Compilers/CSharp/Portable/xlf/CSharpResources.ja.xlf | 5 +++++ src/Compilers/CSharp/Portable/xlf/CSharpResources.ko.xlf | 5 +++++ src/Compilers/CSharp/Portable/xlf/CSharpResources.pl.xlf | 5 +++++ src/Compilers/CSharp/Portable/xlf/CSharpResources.pt-BR.xlf | 5 +++++ src/Compilers/CSharp/Portable/xlf/CSharpResources.ru.xlf | 5 +++++ src/Compilers/CSharp/Portable/xlf/CSharpResources.tr.xlf | 5 +++++ .../CSharp/Portable/xlf/CSharpResources.zh-Hans.xlf | 5 +++++ .../CSharp/Portable/xlf/CSharpResources.zh-Hant.xlf | 5 +++++ 13 files changed, 65 insertions(+) diff --git a/src/Compilers/CSharp/Portable/xlf/CSharpResources.cs.xlf b/src/Compilers/CSharp/Portable/xlf/CSharpResources.cs.xlf index 295b41afcae2e..cdd6de00300db 100644 --- a/src/Compilers/CSharp/Portable/xlf/CSharpResources.cs.xlf +++ b/src/Compilers/CSharp/Portable/xlf/CSharpResources.cs.xlf @@ -8610,6 +8610,11 @@ Pokud chcete odstranit toto varování, můžete místo toho použít /reference ref conditional expression + + The given expression cannot be used in a fixed statement + The given expression cannot be used in a fixed statement + + \ No newline at end of file diff --git a/src/Compilers/CSharp/Portable/xlf/CSharpResources.de.xlf b/src/Compilers/CSharp/Portable/xlf/CSharpResources.de.xlf index f83a782a786fa..b03b4728d798e 100644 --- a/src/Compilers/CSharp/Portable/xlf/CSharpResources.de.xlf +++ b/src/Compilers/CSharp/Portable/xlf/CSharpResources.de.xlf @@ -8610,6 +8610,11 @@ Um die Warnung zu beheben, können Sie stattdessen /reference verwenden (Einbett ref conditional expression + + The given expression cannot be used in a fixed statement + The given expression cannot be used in a fixed statement + + \ No newline at end of file diff --git a/src/Compilers/CSharp/Portable/xlf/CSharpResources.es.xlf b/src/Compilers/CSharp/Portable/xlf/CSharpResources.es.xlf index 8d4feb9eacc7e..81d9e46e584e8 100644 --- a/src/Compilers/CSharp/Portable/xlf/CSharpResources.es.xlf +++ b/src/Compilers/CSharp/Portable/xlf/CSharpResources.es.xlf @@ -8610,6 +8610,11 @@ Para eliminar la advertencia puede usar /reference (establezca la propiedad Embe ref conditional expression + + The given expression cannot be used in a fixed statement + The given expression cannot be used in a fixed statement + + \ No newline at end of file diff --git a/src/Compilers/CSharp/Portable/xlf/CSharpResources.fr.xlf b/src/Compilers/CSharp/Portable/xlf/CSharpResources.fr.xlf index 4bd455eb0860a..3e8ce7dab7924 100644 --- a/src/Compilers/CSharp/Portable/xlf/CSharpResources.fr.xlf +++ b/src/Compilers/CSharp/Portable/xlf/CSharpResources.fr.xlf @@ -8610,6 +8610,11 @@ Pour supprimer l'avertissement, vous pouvez utiliser la commande /reference (dé ref conditional expression + + The given expression cannot be used in a fixed statement + The given expression cannot be used in a fixed statement + + \ No newline at end of file diff --git a/src/Compilers/CSharp/Portable/xlf/CSharpResources.it.xlf b/src/Compilers/CSharp/Portable/xlf/CSharpResources.it.xlf index 6b71e3ed4adcf..de936b9cdbe95 100644 --- a/src/Compilers/CSharp/Portable/xlf/CSharpResources.it.xlf +++ b/src/Compilers/CSharp/Portable/xlf/CSharpResources.it.xlf @@ -8610,6 +8610,11 @@ Per rimuovere l'avviso, è invece possibile usare /reference (impostare la propr ref conditional expression + + The given expression cannot be used in a fixed statement + The given expression cannot be used in a fixed statement + + \ No newline at end of file diff --git a/src/Compilers/CSharp/Portable/xlf/CSharpResources.ja.xlf b/src/Compilers/CSharp/Portable/xlf/CSharpResources.ja.xlf index b0ba89a18fde3..cec8d7d9d72c3 100644 --- a/src/Compilers/CSharp/Portable/xlf/CSharpResources.ja.xlf +++ b/src/Compilers/CSharp/Portable/xlf/CSharpResources.ja.xlf @@ -8610,6 +8610,11 @@ To remove the warning, you can use /reference instead (set the Embed Interop Typ ref conditional expression + + The given expression cannot be used in a fixed statement + The given expression cannot be used in a fixed statement + + \ No newline at end of file diff --git a/src/Compilers/CSharp/Portable/xlf/CSharpResources.ko.xlf b/src/Compilers/CSharp/Portable/xlf/CSharpResources.ko.xlf index 5964ba1d21d98..3d96d16bd9970 100644 --- a/src/Compilers/CSharp/Portable/xlf/CSharpResources.ko.xlf +++ b/src/Compilers/CSharp/Portable/xlf/CSharpResources.ko.xlf @@ -8610,6 +8610,11 @@ To remove the warning, you can use /reference instead (set the Embed Interop Typ ref conditional expression + + The given expression cannot be used in a fixed statement + The given expression cannot be used in a fixed statement + + \ No newline at end of file diff --git a/src/Compilers/CSharp/Portable/xlf/CSharpResources.pl.xlf b/src/Compilers/CSharp/Portable/xlf/CSharpResources.pl.xlf index 3fa8d88eb9e3c..88e2f12c05ed1 100644 --- a/src/Compilers/CSharp/Portable/xlf/CSharpResources.pl.xlf +++ b/src/Compilers/CSharp/Portable/xlf/CSharpResources.pl.xlf @@ -8610,6 +8610,11 @@ Aby usunąć ostrzeżenie, możesz zamiast tego użyć opcji /reference (ustaw w ref conditional expression + + The given expression cannot be used in a fixed statement + The given expression cannot be used in a fixed statement + + \ No newline at end of file diff --git a/src/Compilers/CSharp/Portable/xlf/CSharpResources.pt-BR.xlf b/src/Compilers/CSharp/Portable/xlf/CSharpResources.pt-BR.xlf index 8da7eec0a3595..6408c7ceb85b7 100644 --- a/src/Compilers/CSharp/Portable/xlf/CSharpResources.pt-BR.xlf +++ b/src/Compilers/CSharp/Portable/xlf/CSharpResources.pt-BR.xlf @@ -8610,6 +8610,11 @@ Para incorporar informações de tipo de interoperabilidade para os dois assembl ref conditional expression + + The given expression cannot be used in a fixed statement + The given expression cannot be used in a fixed statement + + \ No newline at end of file diff --git a/src/Compilers/CSharp/Portable/xlf/CSharpResources.ru.xlf b/src/Compilers/CSharp/Portable/xlf/CSharpResources.ru.xlf index 395bfa09caa9e..a97ff3beca8b3 100644 --- a/src/Compilers/CSharp/Portable/xlf/CSharpResources.ru.xlf +++ b/src/Compilers/CSharp/Portable/xlf/CSharpResources.ru.xlf @@ -8610,6 +8610,11 @@ To remove the warning, you can use /reference instead (set the Embed Interop Typ ref conditional expression + + The given expression cannot be used in a fixed statement + The given expression cannot be used in a fixed statement + + \ No newline at end of file diff --git a/src/Compilers/CSharp/Portable/xlf/CSharpResources.tr.xlf b/src/Compilers/CSharp/Portable/xlf/CSharpResources.tr.xlf index d0f8bdb24ce06..d8de689939cd1 100644 --- a/src/Compilers/CSharp/Portable/xlf/CSharpResources.tr.xlf +++ b/src/Compilers/CSharp/Portable/xlf/CSharpResources.tr.xlf @@ -8610,6 +8610,11 @@ Uyarıyı kaldırmak için, /reference kullanabilirsiniz (Birlikte Çalışma T ref conditional expression + + The given expression cannot be used in a fixed statement + The given expression cannot be used in a fixed statement + + \ No newline at end of file diff --git a/src/Compilers/CSharp/Portable/xlf/CSharpResources.zh-Hans.xlf b/src/Compilers/CSharp/Portable/xlf/CSharpResources.zh-Hans.xlf index a6567020b924a..a427fc9dc7f5f 100644 --- a/src/Compilers/CSharp/Portable/xlf/CSharpResources.zh-Hans.xlf +++ b/src/Compilers/CSharp/Portable/xlf/CSharpResources.zh-Hans.xlf @@ -8610,6 +8610,11 @@ To remove the warning, you can use /reference instead (set the Embed Interop Typ ref conditional expression + + The given expression cannot be used in a fixed statement + The given expression cannot be used in a fixed statement + + \ No newline at end of file diff --git a/src/Compilers/CSharp/Portable/xlf/CSharpResources.zh-Hant.xlf b/src/Compilers/CSharp/Portable/xlf/CSharpResources.zh-Hant.xlf index 862c51a1a77e7..41533e8e23d1d 100644 --- a/src/Compilers/CSharp/Portable/xlf/CSharpResources.zh-Hant.xlf +++ b/src/Compilers/CSharp/Portable/xlf/CSharpResources.zh-Hant.xlf @@ -8610,6 +8610,11 @@ To remove the warning, you can use /reference instead (set the Embed Interop Typ ref conditional expression + + The given expression cannot be used in a fixed statement + The given expression cannot be used in a fixed statement + + \ No newline at end of file From 9ab10c25cb68cb614bd382d73c8e1ea217881028 Mon Sep 17 00:00:00 2001 From: vsadov Date: Wed, 7 Feb 2018 18:20:44 -0800 Subject: [PATCH 06/10] not lifting to nullable --- .../Portable/Binder/Binder_Statements.cs | 2 - .../LocalRewriter_FixedStatement.cs | 7 +- .../CSharp/Test/Emit/CodeGen/UnsafeTests.cs | 99 +++++++++++++++++++ 3 files changed, 101 insertions(+), 7 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Binder/Binder_Statements.cs b/src/Compilers/CSharp/Portable/Binder/Binder_Statements.cs index afc27650e2b82..d782630be84a1 100644 --- a/src/Compilers/CSharp/Portable/Binder/Binder_Statements.cs +++ b/src/Compilers/CSharp/Portable/Binder/Binder_Statements.cs @@ -1051,8 +1051,6 @@ private bool IsValidFixedVariableInitializer(TypeSymbol declType, SourceLocalSym if ((object)initializerType == null) { - // TODO: VS replace with better error. - // CONSIDER: this is a very confusing error message, but it's what Dev10 reports. Error(diagnostics, ErrorCode.ERR_ExprCannotBeFixed, initializerSyntax); return false; } diff --git a/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_FixedStatement.cs b/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_FixedStatement.cs index 552f2bb3ff4aa..21305682bb632 100644 --- a/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_FixedStatement.cs +++ b/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_FixedStatement.cs @@ -325,13 +325,12 @@ private BoundStatement InitializeFixedStatementGetPinnable( kind: SynthesizedLocalKind.FixedReference); //TODO: VS - // binding for nullable // error on ref extensions vs. not writeable receiver BoundExpression callReceiver; int currentConditionalAccessID = 0; - bool needNullCheck = !initializerType.IsValueType || initializerType.IsNullableType(); + bool needNullCheck = !initializerType.IsValueType; if (needNullCheck) { @@ -381,9 +380,7 @@ private BoundStatement InitializeFixedStatementGetPinnable( pinAndGetPtr = new BoundLoweredConditionalAccess( initializerSyntax, initializerExpr, - initializerType.IsNullableType() ? - UnsafeGetNullableMethod(initializerSyntax, initializerType, SpecialMember.System_Nullable_T_get_HasValue) : - null, + hasValueMethodOpt: null, whenNotNull: pinAndGetPtr, whenNullOpt: null, // just return default(T*) currentConditionalAccessID, diff --git a/src/Compilers/CSharp/Test/Emit/CodeGen/UnsafeTests.cs b/src/Compilers/CSharp/Test/Emit/CodeGen/UnsafeTests.cs index 7fbfc5cd2fe4f..c2673ce1f8ebd 100644 --- a/src/Compilers/CSharp/Test/Emit/CodeGen/UnsafeTests.cs +++ b/src/Compilers/CSharp/Test/Emit/CodeGen/UnsafeTests.cs @@ -4143,6 +4143,105 @@ .locals init (pinned int& V_0, "); } + [Fact] + public void CustomFixedStructNullable() + { + var text = @" +unsafe class C +{ + public static void Main() + { + Fixable? f = new Fixable(); + + fixed (int* p = f) + { + System.Console.WriteLine(p[1]); + } + } +} + +public struct Fixable +{ + public ref int DangerousGetPinnableReference() + { + return ref (new int[]{1,2,3})[0]; + } +} + +public static class FixableExt +{ + public static ref int DangerousGetPinnableReference(this Fixable? f) + { + return ref f.Value.DangerousGetPinnableReference(); + } +} + +"; + + var compVerifier = CompileAndVerify(text, additionalRefs: new[] { ExtensionAssemblyRef },options: TestOptions.UnsafeReleaseExe, expectedOutput: @"2", verify: Verification.Fails); + + compVerifier.VerifyIL("C.Main", @" +{ + // Code size 34 (0x22) + .maxstack 2 + .locals init (Fixable V_0, + pinned int& V_1) + IL_0000: ldloca.s V_0 + IL_0002: initobj ""Fixable"" + IL_0008: ldloc.0 + IL_0009: newobj ""Fixable?..ctor(Fixable)"" + IL_000e: call ""ref int FixableExt.DangerousGetPinnableReference(Fixable?)"" + IL_0013: stloc.1 + IL_0014: ldloc.1 + IL_0015: conv.u + IL_0016: ldc.i4.4 + IL_0017: add + IL_0018: ldind.i4 + IL_0019: call ""void System.Console.WriteLine(int)"" + IL_001e: ldc.i4.0 + IL_001f: conv.u + IL_0020: stloc.1 + IL_0021: ret +} +"); + } + + [Fact] + public void CustomFixedStructNullableErr() + { + var text = @" +unsafe class C +{ + public static void Main() + { + Fixable? f = new Fixable(); + + fixed (int* p = f) + { + System.Console.WriteLine(p[1]); + } + } +} + +public struct Fixable +{ + public ref int DangerousGetPinnableReference() + { + return ref (new int[]{1,2,3})[0]; + } +} + +"; + + var compVerifier = CreateCompilationWithMscorlib46(text, options: TestOptions.UnsafeReleaseExe); + + compVerifier.VerifyDiagnostics( + // (8,25): error CS9365: The given expression cannot be used in a fixed statement + // fixed (int* p = f) + Diagnostic(ErrorCode.ERR_ExprCannotBeFixed, "f").WithLocation(8, 25) + ); + } + [Fact] public void SimpleCaseOfCustomFixedGeneric() { From f2a1db6d67c649956a04eb9d0546ece1234cd699 Mon Sep 17 00:00:00 2001 From: vsadov Date: Wed, 7 Feb 2018 19:13:20 -0800 Subject: [PATCH 07/10] Extra check for ref extensions. Various diagnostics and tests --- .../Portable/Binder/Binder_Statements.cs | 122 +++-- .../Portable/CSharpResources.Designer.cs | 18 + .../CSharp/Portable/CSharpResources.resx | 6 + .../CSharp/Portable/Errors/ErrorCode.cs | 1 + .../CSharp/Portable/Errors/MessageID.cs | 2 + .../LocalRewriter_FixedStatement.cs | 3 - .../Portable/xlf/CSharpResources.cs.xlf | 10 + .../Portable/xlf/CSharpResources.de.xlf | 10 + .../Portable/xlf/CSharpResources.es.xlf | 10 + .../Portable/xlf/CSharpResources.fr.xlf | 10 + .../Portable/xlf/CSharpResources.it.xlf | 10 + .../Portable/xlf/CSharpResources.ja.xlf | 10 + .../Portable/xlf/CSharpResources.ko.xlf | 10 + .../Portable/xlf/CSharpResources.pl.xlf | 10 + .../Portable/xlf/CSharpResources.pt-BR.xlf | 10 + .../Portable/xlf/CSharpResources.ru.xlf | 10 + .../Portable/xlf/CSharpResources.tr.xlf | 10 + .../Portable/xlf/CSharpResources.zh-Hans.xlf | 10 + .../Portable/xlf/CSharpResources.zh-Hant.xlf | 10 + .../CSharp/Test/Emit/CodeGen/UnsafeTests.cs | 489 +++++++++++++++++- 20 files changed, 729 insertions(+), 42 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Binder/Binder_Statements.cs b/src/Compilers/CSharp/Portable/Binder/Binder_Statements.cs index d782630be84a1..128d17b1f8ba8 100644 --- a/src/Compilers/CSharp/Portable/Binder/Binder_Statements.cs +++ b/src/Compilers/CSharp/Portable/Binder/Binder_Statements.cs @@ -1088,26 +1088,48 @@ private bool IsValidFixedVariableInitializer(TypeSymbol declType, SourceLocalSym } // check for "DangerousGetPinnableReference" - getPinnableMethod = GetDangerousGetPinnableReferenceMethod(initializerOpt); - - // check for String - // NOTE: We will allow DangerousGetPinnableReferenceMethod to take precendence, but only if it is a member of System.String - if (initializerType.SpecialType == SpecialType.System_String && - ((object)getPinnableMethod == null || getPinnableMethod.ContainingType.SpecialType == SpecialType.System_String)) + var additionalDiagnostics = DiagnosticBag.GetInstance(); + try { - elementType = this.GetSpecialType(SpecialType.System_Char, diagnostics, initializerSyntax); - Debug.Assert(!elementType.IsManagedType); - break; - } + getPinnableMethod = GetDangerousGetPinnableReferenceMethod(initializerOpt, additionalDiagnostics); + + bool extensisbleFixedEnabled = ((CSharpParseOptions)initializerOpt.SyntaxTree.Options)?.IsFeatureEnabled(MessageID.IDS_FeatureExtensibleFixedStatement) != false; - // not a specially known type, check for getPinnableMethod - if (getPinnableMethod != null) + // check for String + // NOTE: We will allow DangerousGetPinnableReferenceMethod to take precendence, but only if it is a member of System.String + if (initializerType.SpecialType == SpecialType.System_String && + ((object)getPinnableMethod == null || !extensisbleFixedEnabled || getPinnableMethod.ContainingType.SpecialType != SpecialType.System_String)) + { + elementType = this.GetSpecialType(SpecialType.System_Char, diagnostics, initializerSyntax); + Debug.Assert(!elementType.IsManagedType); + break; + } + + if (extensisbleFixedEnabled) + { + // not a specially known type, check for getPinnableMethod + if (getPinnableMethod != null) + { + elementType = getPinnableMethod.ReturnType; + break; + } + else if (additionalDiagnostics != null) + { + diagnostics.AddRange(additionalDiagnostics); + } + } + else if (getPinnableMethod != null) + { + CheckFeatureAvailability(initializerOpt.Syntax, MessageID.IDS_FeatureExtensibleFixedStatement, diagnostics); + } + + Error(diagnostics, ErrorCode.ERR_ExprCannotBeFixed, initializerSyntax); + } + finally { - elementType = getPinnableMethod.ReturnType; - break; + additionalDiagnostics.Free(); } - Error(diagnostics, ErrorCode.ERR_ExprCannotBeFixed, initializerSyntax); return false; } @@ -1121,37 +1143,67 @@ private bool IsValidFixedVariableInitializer(TypeSymbol declType, SourceLocalSym return true; } - private MethodSymbol GetDangerousGetPinnableReferenceMethod(BoundExpression initializer) + private MethodSymbol GetDangerousGetPinnableReferenceMethod(BoundExpression initializer, DiagnosticBag additionalDiagnostics) { if (initializer.Type.SpecialType == SpecialType.System_Void) { return null; } - DiagnosticBag diagnostics = DiagnosticBag.GetInstance(); - var getPinnableReferenceCall = MakeInvocationExpression(initializer.Syntax, initializer, "DangerousGetPinnableReference", ImmutableArray.Empty, diagnostics); - diagnostics.Free(); - - if (getPinnableReferenceCall.HasAnyErrors) + DiagnosticBag bindingDiagnostics = DiagnosticBag.GetInstance(); + try { - return null; - } + var boundAccess = BindInstanceMemberAccess(initializer.Syntax, initializer.Syntax, initializer, "DangerousGetPinnableReference", rightArity:0, typeArgumentsSyntax: default, typeArguments: default, invoked:true, bindingDiagnostics); - if (getPinnableReferenceCall.Kind != BoundKind.Call) - { - return null; - } + if (boundAccess.Kind != BoundKind.MethodGroup) + { + // the thing is not even a method + return null; + } - var getPinnableReferenceMethod = ((BoundCall)getPinnableReferenceCall).Method; - if (getPinnableReferenceMethod is ErrorMethodSymbol || - HasOptionalOrVariableParameters(getPinnableReferenceMethod) || // We might have been able to resolve an overload with optional parameters, so check for that here - getPinnableReferenceMethod.ReturnsVoid || - !getPinnableReferenceMethod.RefKind.IsManagedReference()) + var analyzedArguments = AnalyzedArguments.GetInstance(); + BoundExpression getPinnableReferenceCall = BindMethodGroupInvocation(initializer.Syntax, initializer.Syntax, "DangerousGetPinnableReference", (BoundMethodGroup)boundAccess, analyzedArguments, bindingDiagnostics, queryClause: null, allowUnexpandedForm: false); + analyzedArguments.Free(); + + if (getPinnableReferenceCall.Kind != BoundKind.Call) + { + // did not find anythig callable + return null; + } + + var call = (BoundCall)getPinnableReferenceCall; + if (call.ResultKind == LookupResultKind.Empty) + { + // did not find any methods that even remotely fit + return null; + } + + var getPinnableReferenceMethod = call.Method; + if (getPinnableReferenceMethod is ErrorMethodSymbol || + getPinnableReferenceCall.HasAnyErrors) + { + // we almost succeded, this is unusual and may be hard to diagnose. + // report additional errors on why we failed to bind the helper + additionalDiagnostics.AddRange(bindingDiagnostics); + return null; + } + + if (HasOptionalOrVariableParameters(getPinnableReferenceMethod) || + getPinnableReferenceMethod.ReturnsVoid || + !getPinnableReferenceMethod.RefKind.IsManagedReference() || + !(getPinnableReferenceMethod.ParameterCount == 0 || getPinnableReferenceMethod.IsStatic && getPinnableReferenceMethod.ParameterCount == 1)) + { + // the method does not fit the pattern + additionalDiagnostics.Add(ErrorCode.WRN_PatternBadSignature, initializer.Syntax.Location, initializer.Type, "fixed", getPinnableReferenceMethod); + return null; + } + + return getPinnableReferenceMethod; + } + finally { - return null; + bindingDiagnostics.Free(); } - - return getPinnableReferenceMethod; } /// diff --git a/src/Compilers/CSharp/Portable/CSharpResources.Designer.cs b/src/Compilers/CSharp/Portable/CSharpResources.Designer.cs index 1f3352f6cbfc9..d63ed42b11544 100644 --- a/src/Compilers/CSharp/Portable/CSharpResources.Designer.cs +++ b/src/Compilers/CSharp/Portable/CSharpResources.Designer.cs @@ -4750,6 +4750,15 @@ internal static string ERR_FileNotFound { } } + /// + /// Looks up a localized string similar to The method '{0}' does not meet requirements necessary . + /// + internal static string ERR_FixableHelperDoesNotFitThePattern { + get { + return ResourceManager.GetString("ERR_FixableHelperDoesNotFitThePattern", resourceCulture); + } + } + /// /// Looks up a localized string similar to You cannot use fixed size buffers contained in unfixed expressions. Try using the fixed statement.. /// @@ -10448,6 +10457,15 @@ internal static string IDS_FeatureExpressionBodiedProperty { } } + /// + /// Looks up a localized string similar to extensible fixed statement. + /// + internal static string IDS_FeatureExtensibleFixedStatement { + get { + return ResourceManager.GetString("IDS_FeatureExtensibleFixedStatement", resourceCulture); + } + } + /// /// Looks up a localized string similar to extension method. /// diff --git a/src/Compilers/CSharp/Portable/CSharpResources.resx b/src/Compilers/CSharp/Portable/CSharpResources.resx index 46cfc210746d7..d727d9e0624e8 100644 --- a/src/Compilers/CSharp/Portable/CSharpResources.resx +++ b/src/Compilers/CSharp/Portable/CSharpResources.resx @@ -930,6 +930,9 @@ The given expression cannot be used in a fixed statement + + The method '{0}' does not meet requirements necessary + Pointers and fixed size buffers may only be used in an unsafe context @@ -4270,6 +4273,9 @@ To remove the warning, you can use /reference instead (set the Embed Interop Typ ref conditional expression + + extensible fixed statement + Compilation (C#): diff --git a/src/Compilers/CSharp/Portable/Errors/ErrorCode.cs b/src/Compilers/CSharp/Portable/Errors/ErrorCode.cs index c54a851d35f64..e3265fad00756 100644 --- a/src/Compilers/CSharp/Portable/Errors/ErrorCode.cs +++ b/src/Compilers/CSharp/Portable/Errors/ErrorCode.cs @@ -1560,5 +1560,6 @@ internal enum ErrorCode // Note: you will need to re-generate compiler code after adding warnings (build\scripts\generate-compiler-code.cmd) ERR_ExprCannotBeFixed = 9365, + ERR_FixableHelperDoesNotFitThePattern = 9366, } } diff --git a/src/Compilers/CSharp/Portable/Errors/MessageID.cs b/src/Compilers/CSharp/Portable/Errors/MessageID.cs index 3cc8de8a2d211..1d3d0fc6a7f26 100644 --- a/src/Compilers/CSharp/Portable/Errors/MessageID.cs +++ b/src/Compilers/CSharp/Portable/Errors/MessageID.cs @@ -148,6 +148,7 @@ internal enum MessageID IDS_FeatureRefConditional = MessageBase + 12731, IDS_FeatureAttributesOnBackingFields = MessageBase + 12732, + IDS_FeatureExtensibleFixedStatement = MessageBase + 12733, } // Message IDs may refer to strings that need to be localized. @@ -189,6 +190,7 @@ internal static LanguageVersion RequiredVersion(this MessageID feature) { // C# 7.3 features. case MessageID.IDS_FeatureAttributesOnBackingFields: // semantic check + case MessageID.IDS_FeatureExtensibleFixedStatement: // semantic check return LanguageVersion.CSharp7_3; // C# 7.2 features. diff --git a/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_FixedStatement.cs b/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_FixedStatement.cs index 21305682bb632..150aaf5459461 100644 --- a/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_FixedStatement.cs +++ b/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_FixedStatement.cs @@ -324,9 +324,6 @@ private BoundStatement InitializeFixedStatementGetPinnable( refKind: RefKind.RefReadOnly, kind: SynthesizedLocalKind.FixedReference); - //TODO: VS - // error on ref extensions vs. not writeable receiver - BoundExpression callReceiver; int currentConditionalAccessID = 0; diff --git a/src/Compilers/CSharp/Portable/xlf/CSharpResources.cs.xlf b/src/Compilers/CSharp/Portable/xlf/CSharpResources.cs.xlf index cdd6de00300db..2bcaf056acbc3 100644 --- a/src/Compilers/CSharp/Portable/xlf/CSharpResources.cs.xlf +++ b/src/Compilers/CSharp/Portable/xlf/CSharpResources.cs.xlf @@ -8615,6 +8615,16 @@ Pokud chcete odstranit toto varování, můžete místo toho použít /reference The given expression cannot be used in a fixed statement + + The method '{0}' does not meet requirements necessary + The method '{0}' does not meet requirements necessary + + + + extensible fixed statement + extensible fixed statement + + \ No newline at end of file diff --git a/src/Compilers/CSharp/Portable/xlf/CSharpResources.de.xlf b/src/Compilers/CSharp/Portable/xlf/CSharpResources.de.xlf index b03b4728d798e..ee8984722a37e 100644 --- a/src/Compilers/CSharp/Portable/xlf/CSharpResources.de.xlf +++ b/src/Compilers/CSharp/Portable/xlf/CSharpResources.de.xlf @@ -8615,6 +8615,16 @@ Um die Warnung zu beheben, können Sie stattdessen /reference verwenden (Einbett The given expression cannot be used in a fixed statement + + The method '{0}' does not meet requirements necessary + The method '{0}' does not meet requirements necessary + + + + extensible fixed statement + extensible fixed statement + + \ No newline at end of file diff --git a/src/Compilers/CSharp/Portable/xlf/CSharpResources.es.xlf b/src/Compilers/CSharp/Portable/xlf/CSharpResources.es.xlf index 81d9e46e584e8..723ea486a31c9 100644 --- a/src/Compilers/CSharp/Portable/xlf/CSharpResources.es.xlf +++ b/src/Compilers/CSharp/Portable/xlf/CSharpResources.es.xlf @@ -8615,6 +8615,16 @@ Para eliminar la advertencia puede usar /reference (establezca la propiedad Embe The given expression cannot be used in a fixed statement + + The method '{0}' does not meet requirements necessary + The method '{0}' does not meet requirements necessary + + + + extensible fixed statement + extensible fixed statement + + \ No newline at end of file diff --git a/src/Compilers/CSharp/Portable/xlf/CSharpResources.fr.xlf b/src/Compilers/CSharp/Portable/xlf/CSharpResources.fr.xlf index 3e8ce7dab7924..dfc9d6bcb48b9 100644 --- a/src/Compilers/CSharp/Portable/xlf/CSharpResources.fr.xlf +++ b/src/Compilers/CSharp/Portable/xlf/CSharpResources.fr.xlf @@ -8615,6 +8615,16 @@ Pour supprimer l'avertissement, vous pouvez utiliser la commande /reference (dé The given expression cannot be used in a fixed statement + + The method '{0}' does not meet requirements necessary + The method '{0}' does not meet requirements necessary + + + + extensible fixed statement + extensible fixed statement + + \ No newline at end of file diff --git a/src/Compilers/CSharp/Portable/xlf/CSharpResources.it.xlf b/src/Compilers/CSharp/Portable/xlf/CSharpResources.it.xlf index de936b9cdbe95..17b5bd96fd898 100644 --- a/src/Compilers/CSharp/Portable/xlf/CSharpResources.it.xlf +++ b/src/Compilers/CSharp/Portable/xlf/CSharpResources.it.xlf @@ -8615,6 +8615,16 @@ Per rimuovere l'avviso, è invece possibile usare /reference (impostare la propr The given expression cannot be used in a fixed statement + + The method '{0}' does not meet requirements necessary + The method '{0}' does not meet requirements necessary + + + + extensible fixed statement + extensible fixed statement + + \ No newline at end of file diff --git a/src/Compilers/CSharp/Portable/xlf/CSharpResources.ja.xlf b/src/Compilers/CSharp/Portable/xlf/CSharpResources.ja.xlf index cec8d7d9d72c3..8d55e2f52fc5e 100644 --- a/src/Compilers/CSharp/Portable/xlf/CSharpResources.ja.xlf +++ b/src/Compilers/CSharp/Portable/xlf/CSharpResources.ja.xlf @@ -8615,6 +8615,16 @@ To remove the warning, you can use /reference instead (set the Embed Interop Typ The given expression cannot be used in a fixed statement + + The method '{0}' does not meet requirements necessary + The method '{0}' does not meet requirements necessary + + + + extensible fixed statement + extensible fixed statement + + \ No newline at end of file diff --git a/src/Compilers/CSharp/Portable/xlf/CSharpResources.ko.xlf b/src/Compilers/CSharp/Portable/xlf/CSharpResources.ko.xlf index 3d96d16bd9970..e08a02c181ef3 100644 --- a/src/Compilers/CSharp/Portable/xlf/CSharpResources.ko.xlf +++ b/src/Compilers/CSharp/Portable/xlf/CSharpResources.ko.xlf @@ -8615,6 +8615,16 @@ To remove the warning, you can use /reference instead (set the Embed Interop Typ The given expression cannot be used in a fixed statement + + The method '{0}' does not meet requirements necessary + The method '{0}' does not meet requirements necessary + + + + extensible fixed statement + extensible fixed statement + + \ No newline at end of file diff --git a/src/Compilers/CSharp/Portable/xlf/CSharpResources.pl.xlf b/src/Compilers/CSharp/Portable/xlf/CSharpResources.pl.xlf index 88e2f12c05ed1..52d6a8fdb59e1 100644 --- a/src/Compilers/CSharp/Portable/xlf/CSharpResources.pl.xlf +++ b/src/Compilers/CSharp/Portable/xlf/CSharpResources.pl.xlf @@ -8615,6 +8615,16 @@ Aby usunąć ostrzeżenie, możesz zamiast tego użyć opcji /reference (ustaw w The given expression cannot be used in a fixed statement + + The method '{0}' does not meet requirements necessary + The method '{0}' does not meet requirements necessary + + + + extensible fixed statement + extensible fixed statement + + \ No newline at end of file diff --git a/src/Compilers/CSharp/Portable/xlf/CSharpResources.pt-BR.xlf b/src/Compilers/CSharp/Portable/xlf/CSharpResources.pt-BR.xlf index 6408c7ceb85b7..b033648a5c3e2 100644 --- a/src/Compilers/CSharp/Portable/xlf/CSharpResources.pt-BR.xlf +++ b/src/Compilers/CSharp/Portable/xlf/CSharpResources.pt-BR.xlf @@ -8615,6 +8615,16 @@ Para incorporar informações de tipo de interoperabilidade para os dois assembl The given expression cannot be used in a fixed statement + + The method '{0}' does not meet requirements necessary + The method '{0}' does not meet requirements necessary + + + + extensible fixed statement + extensible fixed statement + + \ No newline at end of file diff --git a/src/Compilers/CSharp/Portable/xlf/CSharpResources.ru.xlf b/src/Compilers/CSharp/Portable/xlf/CSharpResources.ru.xlf index a97ff3beca8b3..a9b59f78f6f05 100644 --- a/src/Compilers/CSharp/Portable/xlf/CSharpResources.ru.xlf +++ b/src/Compilers/CSharp/Portable/xlf/CSharpResources.ru.xlf @@ -8615,6 +8615,16 @@ To remove the warning, you can use /reference instead (set the Embed Interop Typ The given expression cannot be used in a fixed statement + + The method '{0}' does not meet requirements necessary + The method '{0}' does not meet requirements necessary + + + + extensible fixed statement + extensible fixed statement + + \ No newline at end of file diff --git a/src/Compilers/CSharp/Portable/xlf/CSharpResources.tr.xlf b/src/Compilers/CSharp/Portable/xlf/CSharpResources.tr.xlf index d8de689939cd1..3bb956927b20a 100644 --- a/src/Compilers/CSharp/Portable/xlf/CSharpResources.tr.xlf +++ b/src/Compilers/CSharp/Portable/xlf/CSharpResources.tr.xlf @@ -8615,6 +8615,16 @@ Uyarıyı kaldırmak için, /reference kullanabilirsiniz (Birlikte Çalışma T The given expression cannot be used in a fixed statement + + The method '{0}' does not meet requirements necessary + The method '{0}' does not meet requirements necessary + + + + extensible fixed statement + extensible fixed statement + + \ No newline at end of file diff --git a/src/Compilers/CSharp/Portable/xlf/CSharpResources.zh-Hans.xlf b/src/Compilers/CSharp/Portable/xlf/CSharpResources.zh-Hans.xlf index a427fc9dc7f5f..bab3fe3a49c5e 100644 --- a/src/Compilers/CSharp/Portable/xlf/CSharpResources.zh-Hans.xlf +++ b/src/Compilers/CSharp/Portable/xlf/CSharpResources.zh-Hans.xlf @@ -8615,6 +8615,16 @@ To remove the warning, you can use /reference instead (set the Embed Interop Typ The given expression cannot be used in a fixed statement + + The method '{0}' does not meet requirements necessary + The method '{0}' does not meet requirements necessary + + + + extensible fixed statement + extensible fixed statement + + \ No newline at end of file diff --git a/src/Compilers/CSharp/Portable/xlf/CSharpResources.zh-Hant.xlf b/src/Compilers/CSharp/Portable/xlf/CSharpResources.zh-Hant.xlf index 41533e8e23d1d..2ff8e49bdca8a 100644 --- a/src/Compilers/CSharp/Portable/xlf/CSharpResources.zh-Hant.xlf +++ b/src/Compilers/CSharp/Portable/xlf/CSharpResources.zh-Hant.xlf @@ -8615,6 +8615,16 @@ To remove the warning, you can use /reference instead (set the Embed Interop Typ The given expression cannot be used in a fixed statement + + The method '{0}' does not meet requirements necessary + The method '{0}' does not meet requirements necessary + + + + extensible fixed statement + extensible fixed statement + + \ No newline at end of file diff --git a/src/Compilers/CSharp/Test/Emit/CodeGen/UnsafeTests.cs b/src/Compilers/CSharp/Test/Emit/CodeGen/UnsafeTests.cs index c2673ce1f8ebd..e064d26d3dad3 100644 --- a/src/Compilers/CSharp/Test/Emit/CodeGen/UnsafeTests.cs +++ b/src/Compilers/CSharp/Test/Emit/CodeGen/UnsafeTests.cs @@ -4041,6 +4041,43 @@ .locals init (pinned int& V_0) "); } + [Fact] + public void SimpleCaseOfCustomFixed_oldVersion() + { + var text = @" +unsafe class C +{ + public static void Main() + { + fixed (int* p = new Fixable()) + { + System.Console.WriteLine(p[1]); + } + } + + class Fixable + { + public ref int DangerousGetPinnableReference() + { + return ref (new int[]{1,2,3})[0]; + } + } + +} +"; + + var compVerifier = CreateCompilationWithMscorlib46(text, options: TestOptions.UnsafeReleaseExe, parseOptions: TestOptions.Regular7_2); + + compVerifier.VerifyDiagnostics( + // (6,25): error CS8320: Feature 'extensible fixed statement' is not available in C# 7.2. Please use language version 7.3 or greater. + // fixed (int* p = new Fixable()) + Diagnostic(ErrorCode.ERR_FeatureNotAvailableInVersion7_2, "new Fixable()").WithArguments("extensible fixed statement", "7.3").WithLocation(6, 25), + // (6,25): error CS9365: The given expression cannot be used in a fixed statement + // fixed (int* p = new Fixable()) + Diagnostic(ErrorCode.ERR_ExprCannotBeFixed, "new Fixable()").WithLocation(6, 25) + ); + } + [Fact] public void SimpleCaseOfCustomFixedNull() { @@ -4532,10 +4569,454 @@ .locals init (pinned int& V_0, "); } - // TODO: VS: - // in extension - // in generic - // ref extension + [Fact] + public void CustomFixedGenericRefExtension() + { + var text = @" + unsafe class C + { + public static void Main() + { + var b = new FixableStruct(); + Test(ref b); + System.Console.WriteLine(b.x); + } + + public static void Test(ref T arg) where T: struct, IFixable + { + fixed (int* p = arg) + { + System.Console.Write(p[1]); + } + } + } + + public interface IFixable + { + ref int DangerousGetPinnableReferenceImpl(); + } + + public struct FixableStruct : IFixable + { + public int x; + + public ref int DangerousGetPinnableReferenceImpl() + { + x = 456; + return ref (new int[] { 4, 5, 6 })[0]; + } + } + + public static class FixableExt + { + public static ref int DangerousGetPinnableReference(ref this T f) where T: struct, IFixable + { + return ref f.DangerousGetPinnableReferenceImpl(); + } + } +"; + + var compVerifier = CompileAndVerify(text, additionalRefs: new[] { ExtensionAssemblyRef }, options: TestOptions.UnsafeReleaseExe, verify: Verification.Fails, expectedOutput: @"5456"); + + compVerifier.VerifyIL("C.Test(ref T)", @" +{ + // Code size 21 (0x15) + .maxstack 2 + .locals init (pinned int& V_0) + IL_0000: ldarg.0 + IL_0001: call ""ref int FixableExt.DangerousGetPinnableReference(ref T)"" + IL_0006: stloc.0 + IL_0007: ldloc.0 + IL_0008: conv.u + IL_0009: ldc.i4.4 + IL_000a: add + IL_000b: ldind.i4 + IL_000c: call ""void System.Console.Write(int)"" + IL_0011: ldc.i4.0 + IL_0012: conv.u + IL_0013: stloc.0 + IL_0014: ret +} +"); + } + + [Fact] + public void CustomFixedStructInExtension() + { + var text = @" +unsafe class C +{ + public static void Main() + { + fixed (int* p = new Fixable(1)) + { + System.Console.Write(p[1]); + } + + var f = new Fixable(1); + fixed (int* p = f) + { + System.Console.Write(p[2]); + } + } +} + +public struct Fixable +{ + public Fixable(int arg){} +} + +public static class FixableExt +{ + public static ref int DangerousGetPinnableReference(in this Fixable f) + { + return ref (new int[]{1,2,3})[0]; + } +} + +"; + + var compVerifier = CompileAndVerify(text, additionalRefs: new[] { ExtensionAssemblyRef }, options: TestOptions.UnsafeReleaseExe, expectedOutput: @"23", verify: Verification.Fails); + + compVerifier.VerifyIL("C.Main", @" +{ + // Code size 61 (0x3d) + .maxstack 3 + .locals init (Fixable V_0, //f + pinned int& V_1, + Fixable V_2) + IL_0000: ldc.i4.1 + IL_0001: newobj ""Fixable..ctor(int)"" + IL_0006: stloc.2 + IL_0007: ldloca.s V_2 + IL_0009: call ""ref int FixableExt.DangerousGetPinnableReference(in Fixable)"" + IL_000e: stloc.1 + IL_000f: ldloc.1 + IL_0010: conv.u + IL_0011: ldc.i4.4 + IL_0012: add + IL_0013: ldind.i4 + IL_0014: call ""void System.Console.Write(int)"" + IL_0019: ldc.i4.0 + IL_001a: conv.u + IL_001b: stloc.1 + IL_001c: ldloca.s V_0 + IL_001e: ldc.i4.1 + IL_001f: call ""Fixable..ctor(int)"" + IL_0024: ldloca.s V_0 + IL_0026: call ""ref int FixableExt.DangerousGetPinnableReference(in Fixable)"" + IL_002b: stloc.1 + IL_002c: ldloc.1 + IL_002d: conv.u + IL_002e: ldc.i4.2 + IL_002f: conv.i + IL_0030: ldc.i4.4 + IL_0031: mul + IL_0032: add + IL_0033: ldind.i4 + IL_0034: call ""void System.Console.Write(int)"" + IL_0039: ldc.i4.0 + IL_003a: conv.u + IL_003b: stloc.1 + IL_003c: ret +} +"); + } + + [Fact] + public void CustomFixedStructRefExtension() + { + var text = @" +unsafe class C +{ + public static void Main() + { + var f = new Fixable(1); + fixed (int* p = f) + { + System.Console.Write(p[2]); + } + } +} + +public struct Fixable +{ + public Fixable(int arg){} +} + +public static class FixableExt +{ + public static ref int DangerousGetPinnableReference(ref this Fixable f) + { + return ref (new int[]{1,2,3})[0]; + } +} + +"; + + var compVerifier = CompileAndVerify(text, additionalRefs: new[] { ExtensionAssemblyRef }, options: TestOptions.UnsafeReleaseExe, expectedOutput: @"3", verify: Verification.Fails); + + compVerifier.VerifyIL("C.Main", @" +{ + // Code size 33 (0x21) + .maxstack 3 + .locals init (Fixable V_0, //f + pinned int& V_1) + IL_0000: ldloca.s V_0 + IL_0002: ldc.i4.1 + IL_0003: call ""Fixable..ctor(int)"" + IL_0008: ldloca.s V_0 + IL_000a: call ""ref int FixableExt.DangerousGetPinnableReference(ref Fixable)"" + IL_000f: stloc.1 + IL_0010: ldloc.1 + IL_0011: conv.u + IL_0012: ldc.i4.2 + IL_0013: conv.i + IL_0014: ldc.i4.4 + IL_0015: mul + IL_0016: add + IL_0017: ldind.i4 + IL_0018: call ""void System.Console.Write(int)"" + IL_001d: ldc.i4.0 + IL_001e: conv.u + IL_001f: stloc.1 + IL_0020: ret +} +"); + } + + [Fact] + public void CustomFixedStructRefExtensionErr() + { + var text = @" +unsafe class C +{ + public static void Main() + { + fixed (int* p = new Fixable(1)) + { + System.Console.Write(p[1]); + } + } +} + +public struct Fixable +{ + public Fixable(int arg){} +} + +public static class FixableExt +{ + public static ref int DangerousGetPinnableReference(ref this Fixable f) + { + return ref (new int[]{1,2,3})[0]; + } +} + +"; + + var compVerifier = CreateCompilationWithMscorlib46(text, options: TestOptions.UnsafeReleaseExe); + + compVerifier.VerifyDiagnostics( + // (6,25): error CS1510: A ref or out value must be an assignable variable + // fixed (int* p = new Fixable(1)) + Diagnostic(ErrorCode.ERR_RefLvalueExpected, "new Fixable(1)").WithLocation(6, 25), + // (6,25): error CS9365: The given expression cannot be used in a fixed statement + // fixed (int* p = new Fixable(1)) + Diagnostic(ErrorCode.ERR_ExprCannotBeFixed, "new Fixable(1)").WithLocation(6, 25) + ); + } + + [Fact] + public void CustomFixedStructVariousErr01() + { + var text = @" +unsafe class C +{ + public static void Main() + { + fixed (int* p = new Fixable(1)) + { + System.Console.Write(p[1]); + } + } +} + +public struct Fixable +{ + public Fixable(int arg){} +} + +public static class FixableExt +{ + private static ref int DangerousGetPinnableReference(this Fixable f) + { + return ref (new int[]{1,2,3})[0]; + } +} + +"; + + var compVerifier = CreateCompilationWithMscorlib46(text, options: TestOptions.UnsafeReleaseExe); + + compVerifier.VerifyDiagnostics( + // (6,25): error CS9365: The given expression cannot be used in a fixed statement + // fixed (int* p = new Fixable(1)) + Diagnostic(ErrorCode.ERR_ExprCannotBeFixed, "new Fixable(1)").WithLocation(6, 25), + // (6,25): error CS0122: 'FixableExt.DangerousGetPinnableReference(Fixable)' is inaccessible due to its protection level + // fixed (int* p = new Fixable(1)) + Diagnostic(ErrorCode.ERR_BadAccess, "new Fixable(1)").WithArguments("FixableExt.DangerousGetPinnableReference(Fixable)").WithLocation(6, 25) + ); + } + + [Fact] + public void CustomFixedStructVariousErr01_oldVersion() + { + var text = @" +unsafe class C +{ + public static void Main() + { + fixed (int* p = new Fixable(1)) + { + System.Console.Write(p[1]); + } + } +} + +public struct Fixable +{ + public Fixable(int arg){} +} + +public static class FixableExt +{ + private static ref int DangerousGetPinnableReference(this Fixable f) + { + return ref (new int[]{1,2,3})[0]; + } +} + +"; + + var compVerifier = CreateCompilationWithMscorlib46(text, options: TestOptions.UnsafeReleaseExe, parseOptions: TestOptions.Regular7_2); + + compVerifier.VerifyDiagnostics( + // (6,25): error CS9365: The given expression cannot be used in a fixed statement + // fixed (int* p = new Fixable(1)) + Diagnostic(ErrorCode.ERR_ExprCannotBeFixed, "new Fixable(1)").WithLocation(6, 25) + ); + } + + [Fact] + public void CustomFixedStructVariousErr02() + { + var text = @" +unsafe class C +{ + public static void Main() + { + fixed (int* p = new Fixable(1)) + { + System.Console.Write(p[1]); + } + } +} + +public struct Fixable +{ + public Fixable(int arg){} + + public static ref int DangerousGetPinnableReference() + { + return ref (new int[]{1,2,3})[0]; + } +} + +"; + + var compVerifier = CreateCompilationWithMscorlib46(text, options: TestOptions.UnsafeReleaseExe); + + compVerifier.VerifyDiagnostics( + // (6,25): error CS9365: The given expression cannot be used in a fixed statement + // fixed (int* p = new Fixable(1)) + Diagnostic(ErrorCode.ERR_ExprCannotBeFixed, "new Fixable(1)").WithLocation(6, 25), + // (6,25): error CS0176: Member 'Fixable.DangerousGetPinnableReference()' cannot be accessed with an instance reference; qualify it with a type name instead + // fixed (int* p = new Fixable(1)) + Diagnostic(ErrorCode.ERR_ObjectProhibited, "new Fixable(1)").WithArguments("Fixable.DangerousGetPinnableReference()").WithLocation(6, 25) + ); + } + + [Fact] + public void CustomFixedStructVariousErr03() + { + var text = @" +unsafe class C +{ + public static void Main() + { + fixed (int* p = new Fixable(1)) + { + System.Console.Write(p[1]); + } + } +} + +public struct Fixable +{ + public Fixable(int arg){} + + public ref int DangerousGetPinnableReference => ref (new int[]{1,2,3})[0]; +} + +"; + + var compVerifier = CreateCompilationWithMscorlib46(text, options: TestOptions.UnsafeReleaseExe); + + compVerifier.VerifyDiagnostics( + // (6,25): error CS1955: Non-invocable member 'Fixable.DangerousGetPinnableReference' cannot be used like a method. + // fixed (int* p = new Fixable(1)) + Diagnostic(ErrorCode.ERR_NonInvocableMemberCalled, "new Fixable(1)").WithArguments("Fixable.DangerousGetPinnableReference").WithLocation(6, 25), + // (6,25): error CS9365: The given expression cannot be used in a fixed statement + // fixed (int* p = new Fixable(1)) + Diagnostic(ErrorCode.ERR_ExprCannotBeFixed, "new Fixable(1)").WithLocation(6, 25) + ); + } + + [Fact] + public void CustomFixedDelegateErr() + { + var text = @" +unsafe class C +{ + public static void Main() + { + fixed (int* p = new Fixable()) + { + System.Console.Write(p[1]); + } + } +} + +public delegate ref int ReturnsRef(); + +public struct Fixable +{ + public Fixable(int arg){} + + public ReturnsRef DangerousGetPinnableReference => null; +} + +"; + + var compVerifier = CreateCompilationWithMscorlib46(text, options: TestOptions.UnsafeReleaseExe); + + compVerifier.VerifyDiagnostics( + // (6,25): error CS9365: The given expression cannot be used in a fixed statement + // fixed (int* p = new Fixable()) + Diagnostic(ErrorCode.ERR_ExprCannotBeFixed, "new Fixable()").WithLocation(6, 25) + ); + } #endregion Custom fixed statement tests From d810de61ceaf63af052eaac03a6ae5e267e5ce94 Mon Sep 17 00:00:00 2001 From: vsadov Date: Thu, 8 Feb 2018 17:49:07 -0800 Subject: [PATCH 08/10] moved method name to a constant --- src/Compilers/CSharp/Portable/Binder/Binder_Statements.cs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Binder/Binder_Statements.cs b/src/Compilers/CSharp/Portable/Binder/Binder_Statements.cs index 128d17b1f8ba8..e73616c46eada 100644 --- a/src/Compilers/CSharp/Portable/Binder/Binder_Statements.cs +++ b/src/Compilers/CSharp/Portable/Binder/Binder_Statements.cs @@ -1087,7 +1087,7 @@ private bool IsValidFixedVariableInitializer(TypeSymbol declType, SourceLocalSym break; } - // check for "DangerousGetPinnableReference" + // check for a special ref-returning method var additionalDiagnostics = DiagnosticBag.GetInstance(); try { @@ -1150,10 +1150,12 @@ private MethodSymbol GetDangerousGetPinnableReferenceMethod(BoundExpression init return null; } + const string methodName = "DangerousGetPinnableReference"; + DiagnosticBag bindingDiagnostics = DiagnosticBag.GetInstance(); try { - var boundAccess = BindInstanceMemberAccess(initializer.Syntax, initializer.Syntax, initializer, "DangerousGetPinnableReference", rightArity:0, typeArgumentsSyntax: default, typeArguments: default, invoked:true, bindingDiagnostics); + var boundAccess = BindInstanceMemberAccess(initializer.Syntax, initializer.Syntax, initializer, methodName, rightArity:0, typeArgumentsSyntax: default, typeArguments: default, invoked:true, bindingDiagnostics); if (boundAccess.Kind != BoundKind.MethodGroup) { @@ -1162,7 +1164,7 @@ private MethodSymbol GetDangerousGetPinnableReferenceMethod(BoundExpression init } var analyzedArguments = AnalyzedArguments.GetInstance(); - BoundExpression getPinnableReferenceCall = BindMethodGroupInvocation(initializer.Syntax, initializer.Syntax, "DangerousGetPinnableReference", (BoundMethodGroup)boundAccess, analyzedArguments, bindingDiagnostics, queryClause: null, allowUnexpandedForm: false); + BoundExpression getPinnableReferenceCall = BindMethodGroupInvocation(initializer.Syntax, initializer.Syntax, methodName, (BoundMethodGroup)boundAccess, analyzedArguments, bindingDiagnostics, queryClause: null, allowUnexpandedForm: false); analyzedArguments.Free(); if (getPinnableReferenceCall.Kind != BoundKind.Call) From 276da81e9d8bcd02ea0cdb51a7e6bb2c6e68a0c6 Mon Sep 17 00:00:00 2001 From: vsadov Date: Tue, 20 Feb 2018 14:15:00 -0800 Subject: [PATCH 09/10] PR feedback --- .../Portable/Binder/Binder_Statements.cs | 88 ++++++----- .../Portable/BoundTree/GenerateNodes.bat | 7 - .../Portable/CSharpResources.Designer.cs | 2 +- .../CSharp/Portable/CSharpResources.resx | 2 +- .../CSharp/Portable/Errors/ErrorCode.cs | 5 +- .../LocalRewriter_FixedStatement.cs | 13 +- .../Portable/xlf/CSharpResources.cs.xlf | 4 +- .../Portable/xlf/CSharpResources.de.xlf | 4 +- .../Portable/xlf/CSharpResources.es.xlf | 4 +- .../Portable/xlf/CSharpResources.fr.xlf | 4 +- .../Portable/xlf/CSharpResources.it.xlf | 4 +- .../Portable/xlf/CSharpResources.ja.xlf | 4 +- .../Portable/xlf/CSharpResources.ko.xlf | 4 +- .../Portable/xlf/CSharpResources.pl.xlf | 4 +- .../Portable/xlf/CSharpResources.pt-BR.xlf | 4 +- .../Portable/xlf/CSharpResources.ru.xlf | 4 +- .../Portable/xlf/CSharpResources.tr.xlf | 4 +- .../Portable/xlf/CSharpResources.zh-Hans.xlf | 4 +- .../Portable/xlf/CSharpResources.zh-Hant.xlf | 4 +- .../CSharp/Test/Emit/CodeGen/UnsafeTests.cs | 148 +++++++++++++++++- 20 files changed, 227 insertions(+), 90 deletions(-) delete mode 100644 src/Compilers/CSharp/Portable/BoundTree/GenerateNodes.bat diff --git a/src/Compilers/CSharp/Portable/Binder/Binder_Statements.cs b/src/Compilers/CSharp/Portable/Binder/Binder_Statements.cs index e73616c46eada..b9e4745c7209b 100644 --- a/src/Compilers/CSharp/Portable/Binder/Binder_Statements.cs +++ b/src/Compilers/CSharp/Portable/Binder/Binder_Statements.cs @@ -1082,55 +1082,43 @@ private bool IsValidFixedVariableInitializer(TypeSymbol declType, SourceLocalSym if (initializerType.IsArray()) { // See ExpressionBinder::BindPtrToArray (though most of that functionality is now in LocalRewriter). - var arrayType = (ArrayTypeSymbol)initializerType; - elementType = arrayType.ElementType; + elementType = ((ArrayTypeSymbol)initializerType).ElementType; break; } // check for a special ref-returning method var additionalDiagnostics = DiagnosticBag.GetInstance(); - try - { - getPinnableMethod = GetDangerousGetPinnableReferenceMethod(initializerOpt, additionalDiagnostics); - - bool extensisbleFixedEnabled = ((CSharpParseOptions)initializerOpt.SyntaxTree.Options)?.IsFeatureEnabled(MessageID.IDS_FeatureExtensibleFixedStatement) != false; + getPinnableMethod = GetDangerousGetPinnableReferenceMethodOpt(initializerOpt, additionalDiagnostics); - // check for String - // NOTE: We will allow DangerousGetPinnableReferenceMethod to take precendence, but only if it is a member of System.String - if (initializerType.SpecialType == SpecialType.System_String && - ((object)getPinnableMethod == null || !extensisbleFixedEnabled || getPinnableMethod.ContainingType.SpecialType != SpecialType.System_String)) - { - elementType = this.GetSpecialType(SpecialType.System_Char, diagnostics, initializerSyntax); - Debug.Assert(!elementType.IsManagedType); - break; - } + // check for String + // NOTE: We will allow DangerousGetPinnableReferenceMethod to take precendence, but only if it is a member of System.String + if (initializerType.SpecialType == SpecialType.System_String && + ((object)getPinnableMethod == null || getPinnableMethod.ContainingType.SpecialType != SpecialType.System_String)) + { + elementType = this.GetSpecialType(SpecialType.System_Char, diagnostics, initializerSyntax); + additionalDiagnostics.Free(); + break; + } - if (extensisbleFixedEnabled) - { - // not a specially known type, check for getPinnableMethod - if (getPinnableMethod != null) - { - elementType = getPinnableMethod.ReturnType; - break; - } - else if (additionalDiagnostics != null) - { - diagnostics.AddRange(additionalDiagnostics); - } - } - else if (getPinnableMethod != null) + if ((object)getPinnableMethod != null) + { + elementType = getPinnableMethod.ReturnType; + CheckFeatureAvailability(initializerOpt.Syntax, MessageID.IDS_FeatureExtensibleFixedStatement, diagnostics); + additionalDiagnostics.Free(); + break; + } + else + { + // if feature was enabled but somethng went wrong with the method, report that + bool extensibleFixedEnabled = ((CSharpParseOptions)initializerOpt.SyntaxTree.Options)?.IsFeatureEnabled(MessageID.IDS_FeatureExtensibleFixedStatement) != false; + if (extensibleFixedEnabled && additionalDiagnostics != null) { - CheckFeatureAvailability(initializerOpt.Syntax, MessageID.IDS_FeatureExtensibleFixedStatement, diagnostics); + diagnostics.AddRange(additionalDiagnostics); } - Error(diagnostics, ErrorCode.ERR_ExprCannotBeFixed, initializerSyntax); - } - finally - { additionalDiagnostics.Free(); + return false; } - - return false; } if (elementType.IsManagedType) @@ -1143,7 +1131,7 @@ private bool IsValidFixedVariableInitializer(TypeSymbol declType, SourceLocalSym return true; } - private MethodSymbol GetDangerousGetPinnableReferenceMethod(BoundExpression initializer, DiagnosticBag additionalDiagnostics) + private MethodSymbol GetDangerousGetPinnableReferenceMethodOpt(BoundExpression initializer, DiagnosticBag additionalDiagnostics) { if (initializer.Type.SpecialType == SpecialType.System_Void) { @@ -1155,7 +1143,16 @@ private MethodSymbol GetDangerousGetPinnableReferenceMethod(BoundExpression init DiagnosticBag bindingDiagnostics = DiagnosticBag.GetInstance(); try { - var boundAccess = BindInstanceMemberAccess(initializer.Syntax, initializer.Syntax, initializer, methodName, rightArity:0, typeArgumentsSyntax: default, typeArguments: default, invoked:true, bindingDiagnostics); + var boundAccess = BindInstanceMemberAccess( + initializer.Syntax, + initializer.Syntax, + initializer, + methodName, + rightArity: 0, + typeArgumentsSyntax: default, + typeArguments: default, + invoked: true, + bindingDiagnostics); if (boundAccess.Kind != BoundKind.MethodGroup) { @@ -1164,12 +1161,21 @@ private MethodSymbol GetDangerousGetPinnableReferenceMethod(BoundExpression init } var analyzedArguments = AnalyzedArguments.GetInstance(); - BoundExpression getPinnableReferenceCall = BindMethodGroupInvocation(initializer.Syntax, initializer.Syntax, methodName, (BoundMethodGroup)boundAccess, analyzedArguments, bindingDiagnostics, queryClause: null, allowUnexpandedForm: false); + BoundExpression getPinnableReferenceCall = BindMethodGroupInvocation( + initializer.Syntax, + initializer.Syntax, + methodName, + (BoundMethodGroup)boundAccess, + analyzedArguments, + bindingDiagnostics, + queryClause: null, + allowUnexpandedForm: false); + analyzedArguments.Free(); if (getPinnableReferenceCall.Kind != BoundKind.Call) { - // did not find anythig callable + // did not find anything callable return null; } diff --git a/src/Compilers/CSharp/Portable/BoundTree/GenerateNodes.bat b/src/Compilers/CSharp/Portable/BoundTree/GenerateNodes.bat deleted file mode 100644 index 4bb735b8cc5f5..0000000000000 --- a/src/Compilers/CSharp/Portable/BoundTree/GenerateNodes.bat +++ /dev/null @@ -1,7 +0,0 @@ -REM Copyright (c) Microsoft. All Rights Reserved. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. - -REM Run the node generator to create new bound tree node definitions. -REM You must run this in the directory containing BoundNodes.xml - -@echo on -dotnet ..\..\..\..\..\Binaries\Debug\Exes\CompilersBoundTreeGenerator\BoundTreeGenerator.dll CSharp BoundNodes.xml ..\Generated\BoundNodes.xml.Generated.cs diff --git a/src/Compilers/CSharp/Portable/CSharpResources.Designer.cs b/src/Compilers/CSharp/Portable/CSharpResources.Designer.cs index d63ed42b11544..dd44ee12b9d7a 100644 --- a/src/Compilers/CSharp/Portable/CSharpResources.Designer.cs +++ b/src/Compilers/CSharp/Portable/CSharpResources.Designer.cs @@ -4751,7 +4751,7 @@ internal static string ERR_FileNotFound { } /// - /// Looks up a localized string similar to The method '{0}' does not meet requirements necessary . + /// Looks up a localized string similar to The method '{0}' does not meet necessary requirements. /// internal static string ERR_FixableHelperDoesNotFitThePattern { get { diff --git a/src/Compilers/CSharp/Portable/CSharpResources.resx b/src/Compilers/CSharp/Portable/CSharpResources.resx index d727d9e0624e8..4e995e64a3d14 100644 --- a/src/Compilers/CSharp/Portable/CSharpResources.resx +++ b/src/Compilers/CSharp/Portable/CSharpResources.resx @@ -931,7 +931,7 @@ The given expression cannot be used in a fixed statement - The method '{0}' does not meet requirements necessary + The method '{0}' does not meet necessary requirements Pointers and fixed size buffers may only be used in an unsafe context diff --git a/src/Compilers/CSharp/Portable/Errors/ErrorCode.cs b/src/Compilers/CSharp/Portable/Errors/ErrorCode.cs index e3265fad00756..bfd07d88ea230 100644 --- a/src/Compilers/CSharp/Portable/Errors/ErrorCode.cs +++ b/src/Compilers/CSharp/Portable/Errors/ErrorCode.cs @@ -1557,9 +1557,8 @@ internal enum ErrorCode WRN_AttributesOnBackingFieldsNotAvailable = 8371, ERR_DoNotUseFixedBufferAttrOnProperty = 8372, - // Note: you will need to re-generate compiler code after adding warnings (build\scripts\generate-compiler-code.cmd) - ERR_ExprCannotBeFixed = 9365, - ERR_FixableHelperDoesNotFitThePattern = 9366, + ERR_FixableHelperDoesNotFitThePattern = 9366, } + // Note: you will need to re-generate compiler code after adding warnings (build\scripts\generate-compiler-code.cmd) } diff --git a/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_FixedStatement.cs b/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_FixedStatement.cs index 150aaf5459461..5482d950ac725 100644 --- a/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_FixedStatement.cs +++ b/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_FixedStatement.cs @@ -224,11 +224,13 @@ private BoundStatement InitializeFixedStatementLocal( } /// + /// /// /// pinned ref int pinnedTemp = ref v; // pinning managed ref - /// int* ptr = (int*)&pinnedTemp; // unsafe cast to unmanaged ptr + /// int* ptr = (int*)&pinnedTemp; // unsafe cast to unmanaged ptr /// . . . + /// ]]> /// private BoundStatement InitializeFixedStatementRegularLocal( BoundLocalDeclaration localDecl, @@ -291,11 +293,13 @@ private BoundStatement InitializeFixedStatementRegularLocal( } /// + /// /// /// pinned ref int pinnedTemp = ref v; // pinning managed ref - /// int* ptr = (int*)&pinnedTemp; // unsafe cast to unmanaged ptr + /// int* ptr = (int*)&pinnedTemp; // unsafe cast to unmanaged ptr /// . . . + /// ]]> /// private BoundStatement InitializeFixedStatementGetPinnable( BoundLocalDeclaration localDecl, @@ -315,6 +319,7 @@ private BoundStatement InitializeFixedStatementGetPinnable( VariableDeclaratorSyntax declarator = fixedInitializer.Syntax.FirstAncestorOrSelf(); Debug.Assert(declarator != null); + // pinned ref int pinnedTemp pinnedTemp = factory.SynthesizedLocal( getPinnableMethod.ReturnType, syntax: declarator, @@ -344,8 +349,8 @@ private BoundStatement InitializeFixedStatementGetPinnable( // .GetPinnable() var getPinnableCall = getPinnableMethod.IsStatic? - factory.Call(null, getPinnableMethod, callReceiver): - factory.Call(callReceiver, getPinnableMethod); + factory.Call(null, getPinnableMethod, callReceiver): + factory.Call(callReceiver, getPinnableMethod); // temp =ref .GetPinnable() var tempAssignment = factory.AssignmentExpression( diff --git a/src/Compilers/CSharp/Portable/xlf/CSharpResources.cs.xlf b/src/Compilers/CSharp/Portable/xlf/CSharpResources.cs.xlf index 2bcaf056acbc3..2233b3e185654 100644 --- a/src/Compilers/CSharp/Portable/xlf/CSharpResources.cs.xlf +++ b/src/Compilers/CSharp/Portable/xlf/CSharpResources.cs.xlf @@ -8616,8 +8616,8 @@ Pokud chcete odstranit toto varování, můžete místo toho použít /reference - The method '{0}' does not meet requirements necessary - The method '{0}' does not meet requirements necessary + The method '{0}' does not meet necessary requirements + The method '{0}' does not meet necessary requirements diff --git a/src/Compilers/CSharp/Portable/xlf/CSharpResources.de.xlf b/src/Compilers/CSharp/Portable/xlf/CSharpResources.de.xlf index ee8984722a37e..5c9766a0b176e 100644 --- a/src/Compilers/CSharp/Portable/xlf/CSharpResources.de.xlf +++ b/src/Compilers/CSharp/Portable/xlf/CSharpResources.de.xlf @@ -8616,8 +8616,8 @@ Um die Warnung zu beheben, können Sie stattdessen /reference verwenden (Einbett - The method '{0}' does not meet requirements necessary - The method '{0}' does not meet requirements necessary + The method '{0}' does not meet necessary requirements + The method '{0}' does not meet necessary requirements diff --git a/src/Compilers/CSharp/Portable/xlf/CSharpResources.es.xlf b/src/Compilers/CSharp/Portable/xlf/CSharpResources.es.xlf index 723ea486a31c9..067ed843e71e6 100644 --- a/src/Compilers/CSharp/Portable/xlf/CSharpResources.es.xlf +++ b/src/Compilers/CSharp/Portable/xlf/CSharpResources.es.xlf @@ -8616,8 +8616,8 @@ Para eliminar la advertencia puede usar /reference (establezca la propiedad Embe - The method '{0}' does not meet requirements necessary - The method '{0}' does not meet requirements necessary + The method '{0}' does not meet necessary requirements + The method '{0}' does not meet necessary requirements diff --git a/src/Compilers/CSharp/Portable/xlf/CSharpResources.fr.xlf b/src/Compilers/CSharp/Portable/xlf/CSharpResources.fr.xlf index dfc9d6bcb48b9..5cc8bd6680ff1 100644 --- a/src/Compilers/CSharp/Portable/xlf/CSharpResources.fr.xlf +++ b/src/Compilers/CSharp/Portable/xlf/CSharpResources.fr.xlf @@ -8616,8 +8616,8 @@ Pour supprimer l'avertissement, vous pouvez utiliser la commande /reference (dé - The method '{0}' does not meet requirements necessary - The method '{0}' does not meet requirements necessary + The method '{0}' does not meet necessary requirements + The method '{0}' does not meet necessary requirements diff --git a/src/Compilers/CSharp/Portable/xlf/CSharpResources.it.xlf b/src/Compilers/CSharp/Portable/xlf/CSharpResources.it.xlf index 17b5bd96fd898..8b7f6942fddac 100644 --- a/src/Compilers/CSharp/Portable/xlf/CSharpResources.it.xlf +++ b/src/Compilers/CSharp/Portable/xlf/CSharpResources.it.xlf @@ -8616,8 +8616,8 @@ Per rimuovere l'avviso, è invece possibile usare /reference (impostare la propr - The method '{0}' does not meet requirements necessary - The method '{0}' does not meet requirements necessary + The method '{0}' does not meet necessary requirements + The method '{0}' does not meet necessary requirements diff --git a/src/Compilers/CSharp/Portable/xlf/CSharpResources.ja.xlf b/src/Compilers/CSharp/Portable/xlf/CSharpResources.ja.xlf index 8d55e2f52fc5e..e382639bdb7ef 100644 --- a/src/Compilers/CSharp/Portable/xlf/CSharpResources.ja.xlf +++ b/src/Compilers/CSharp/Portable/xlf/CSharpResources.ja.xlf @@ -8616,8 +8616,8 @@ To remove the warning, you can use /reference instead (set the Embed Interop Typ - The method '{0}' does not meet requirements necessary - The method '{0}' does not meet requirements necessary + The method '{0}' does not meet necessary requirements + The method '{0}' does not meet necessary requirements diff --git a/src/Compilers/CSharp/Portable/xlf/CSharpResources.ko.xlf b/src/Compilers/CSharp/Portable/xlf/CSharpResources.ko.xlf index e08a02c181ef3..ba3b7ba8f9288 100644 --- a/src/Compilers/CSharp/Portable/xlf/CSharpResources.ko.xlf +++ b/src/Compilers/CSharp/Portable/xlf/CSharpResources.ko.xlf @@ -8616,8 +8616,8 @@ To remove the warning, you can use /reference instead (set the Embed Interop Typ - The method '{0}' does not meet requirements necessary - The method '{0}' does not meet requirements necessary + The method '{0}' does not meet necessary requirements + The method '{0}' does not meet necessary requirements diff --git a/src/Compilers/CSharp/Portable/xlf/CSharpResources.pl.xlf b/src/Compilers/CSharp/Portable/xlf/CSharpResources.pl.xlf index 52d6a8fdb59e1..ec2bba8f15bee 100644 --- a/src/Compilers/CSharp/Portable/xlf/CSharpResources.pl.xlf +++ b/src/Compilers/CSharp/Portable/xlf/CSharpResources.pl.xlf @@ -8616,8 +8616,8 @@ Aby usunąć ostrzeżenie, możesz zamiast tego użyć opcji /reference (ustaw w - The method '{0}' does not meet requirements necessary - The method '{0}' does not meet requirements necessary + The method '{0}' does not meet necessary requirements + The method '{0}' does not meet necessary requirements diff --git a/src/Compilers/CSharp/Portable/xlf/CSharpResources.pt-BR.xlf b/src/Compilers/CSharp/Portable/xlf/CSharpResources.pt-BR.xlf index b033648a5c3e2..3ea5bbf30a6a5 100644 --- a/src/Compilers/CSharp/Portable/xlf/CSharpResources.pt-BR.xlf +++ b/src/Compilers/CSharp/Portable/xlf/CSharpResources.pt-BR.xlf @@ -8616,8 +8616,8 @@ Para incorporar informações de tipo de interoperabilidade para os dois assembl - The method '{0}' does not meet requirements necessary - The method '{0}' does not meet requirements necessary + The method '{0}' does not meet necessary requirements + The method '{0}' does not meet necessary requirements diff --git a/src/Compilers/CSharp/Portable/xlf/CSharpResources.ru.xlf b/src/Compilers/CSharp/Portable/xlf/CSharpResources.ru.xlf index a9b59f78f6f05..eb8dda4f79954 100644 --- a/src/Compilers/CSharp/Portable/xlf/CSharpResources.ru.xlf +++ b/src/Compilers/CSharp/Portable/xlf/CSharpResources.ru.xlf @@ -8616,8 +8616,8 @@ To remove the warning, you can use /reference instead (set the Embed Interop Typ - The method '{0}' does not meet requirements necessary - The method '{0}' does not meet requirements necessary + The method '{0}' does not meet necessary requirements + The method '{0}' does not meet necessary requirements diff --git a/src/Compilers/CSharp/Portable/xlf/CSharpResources.tr.xlf b/src/Compilers/CSharp/Portable/xlf/CSharpResources.tr.xlf index 3bb956927b20a..f482864ea3873 100644 --- a/src/Compilers/CSharp/Portable/xlf/CSharpResources.tr.xlf +++ b/src/Compilers/CSharp/Portable/xlf/CSharpResources.tr.xlf @@ -8616,8 +8616,8 @@ Uyarıyı kaldırmak için, /reference kullanabilirsiniz (Birlikte Çalışma T - The method '{0}' does not meet requirements necessary - The method '{0}' does not meet requirements necessary + The method '{0}' does not meet necessary requirements + The method '{0}' does not meet necessary requirements diff --git a/src/Compilers/CSharp/Portable/xlf/CSharpResources.zh-Hans.xlf b/src/Compilers/CSharp/Portable/xlf/CSharpResources.zh-Hans.xlf index bab3fe3a49c5e..df5e91f7d947d 100644 --- a/src/Compilers/CSharp/Portable/xlf/CSharpResources.zh-Hans.xlf +++ b/src/Compilers/CSharp/Portable/xlf/CSharpResources.zh-Hans.xlf @@ -8616,8 +8616,8 @@ To remove the warning, you can use /reference instead (set the Embed Interop Typ - The method '{0}' does not meet requirements necessary - The method '{0}' does not meet requirements necessary + The method '{0}' does not meet necessary requirements + The method '{0}' does not meet necessary requirements diff --git a/src/Compilers/CSharp/Portable/xlf/CSharpResources.zh-Hant.xlf b/src/Compilers/CSharp/Portable/xlf/CSharpResources.zh-Hant.xlf index 2ff8e49bdca8a..f4f65416cd017 100644 --- a/src/Compilers/CSharp/Portable/xlf/CSharpResources.zh-Hant.xlf +++ b/src/Compilers/CSharp/Portable/xlf/CSharpResources.zh-Hant.xlf @@ -8616,8 +8616,8 @@ To remove the warning, you can use /reference instead (set the Embed Interop Typ - The method '{0}' does not meet requirements necessary - The method '{0}' does not meet requirements necessary + The method '{0}' does not meet necessary requirements + The method '{0}' does not meet necessary requirements diff --git a/src/Compilers/CSharp/Test/Emit/CodeGen/UnsafeTests.cs b/src/Compilers/CSharp/Test/Emit/CodeGen/UnsafeTests.cs index e064d26d3dad3..bd93c932780b2 100644 --- a/src/Compilers/CSharp/Test/Emit/CodeGen/UnsafeTests.cs +++ b/src/Compilers/CSharp/Test/Emit/CodeGen/UnsafeTests.cs @@ -4071,10 +4071,7 @@ public ref int DangerousGetPinnableReference() compVerifier.VerifyDiagnostics( // (6,25): error CS8320: Feature 'extensible fixed statement' is not available in C# 7.2. Please use language version 7.3 or greater. // fixed (int* p = new Fixable()) - Diagnostic(ErrorCode.ERR_FeatureNotAvailableInVersion7_2, "new Fixable()").WithArguments("extensible fixed statement", "7.3").WithLocation(6, 25), - // (6,25): error CS9365: The given expression cannot be used in a fixed statement - // fixed (int* p = new Fixable()) - Diagnostic(ErrorCode.ERR_ExprCannotBeFixed, "new Fixable()").WithLocation(6, 25) + Diagnostic(ErrorCode.ERR_FeatureNotAvailableInVersion7_2, "new Fixable()").WithArguments("extensible fixed statement", "7.3").WithLocation(6, 25) ); } @@ -4279,6 +4276,143 @@ public ref int DangerousGetPinnableReference() ); } + [Fact] + public void CustomFixedErrAmbiguous() + { + var text = @" +unsafe class C +{ + public static void Main() + { + fixed (int* p = new Fixable(1)) + { + System.Console.Write(p[1]); + } + + var f = new Fixable(1); + fixed (int* p = f) + { + System.Console.Write(p[2]); + } + } +} + +public struct Fixable +{ + public Fixable(int arg){} +} + +public static class FixableExt +{ + public static ref readonly int DangerousGetPinnableReference(in this Fixable f) + { + return ref (new int[]{1,2,3})[0]; + } +} + +public static class FixableExt1 +{ + public static ref readonly int DangerousGetPinnableReference(in this Fixable f) + { + return ref (new int[]{1,2,3})[0]; + } +} +"; + + var compVerifier = CreateCompilationWithMscorlib46(text, options: TestOptions.UnsafeReleaseExe); + + compVerifier.VerifyDiagnostics( + // (6,25): error CS0121: The call is ambiguous between the following methods or properties: 'FixableExt.DangerousGetPinnableReference(in Fixable)' and 'FixableExt1.DangerousGetPinnableReference(in Fixable)' + // fixed (int* p = new Fixable(1)) + Diagnostic(ErrorCode.ERR_AmbigCall, "new Fixable(1)").WithArguments("FixableExt.DangerousGetPinnableReference(in Fixable)", "FixableExt1.DangerousGetPinnableReference(in Fixable)").WithLocation(6, 25), + // (6,25): error CS9365: The given expression cannot be used in a fixed statement + // fixed (int* p = new Fixable(1)) + Diagnostic(ErrorCode.ERR_ExprCannotBeFixed, "new Fixable(1)").WithLocation(6, 25), + // (12,25): error CS0121: The call is ambiguous between the following methods or properties: 'FixableExt.DangerousGetPinnableReference(in Fixable)' and 'FixableExt1.DangerousGetPinnableReference(in Fixable)' + // fixed (int* p = f) + Diagnostic(ErrorCode.ERR_AmbigCall, "f").WithArguments("FixableExt.DangerousGetPinnableReference(in Fixable)", "FixableExt1.DangerousGetPinnableReference(in Fixable)").WithLocation(12, 25), + // (12,25): error CS9365: The given expression cannot be used in a fixed statement + // fixed (int* p = f) + Diagnostic(ErrorCode.ERR_ExprCannotBeFixed, "f").WithLocation(12, 25) + ); + } + + [Fact] + public void CustomFixedErrDynamic() + { + var text = @" +unsafe class C +{ + public static void Main() + { + fixed (int* p = (dynamic)(new Fixable(1))) + { + System.Console.Write(p[1]); + } + } +} + +public struct Fixable +{ + public Fixable(int arg){} +} + +public static class FixableExt +{ + public static ref readonly int DangerousGetPinnableReference(in this Fixable f) + { + return ref (new int[]{1,2,3})[0]; + } +} +"; + + var compVerifier = CreateCompilationWithMscorlib46(text, options: TestOptions.UnsafeReleaseExe); + + compVerifier.VerifyDiagnostics( + // (6,25): error CS9365: The given expression cannot be used in a fixed statement + // fixed (int* p = (dynamic)(new Fixable(1))) + Diagnostic(ErrorCode.ERR_ExprCannotBeFixed, "(dynamic)(new Fixable(1))").WithLocation(6, 25) + ); + } + + [Fact] + public void CustomFixedErrBad() + { + var text = @" +unsafe class C +{ + public static void Main() + { + fixed (int* p = (HocusPocus)(new Fixable(1))) + { + System.Console.Write(p[1]); + } + } +} + +public struct Fixable +{ + public Fixable(int arg){} +} + +public static class FixableExt +{ + public static ref readonly int DangerousGetPinnableReference(in this Fixable f) + { + return ref (new int[]{1,2,3})[0]; + } +} +"; + + var compVerifier = CreateCompilationWithMscorlib46(text, options: TestOptions.UnsafeReleaseExe); + + compVerifier.VerifyDiagnostics( + // (6,26): error CS0246: The type or namespace name 'HocusPocus' could not be found (are you missing a using directive or an assembly reference?) + // fixed (int* p = (HocusPocus)(new Fixable(1))) + Diagnostic(ErrorCode.ERR_SingleTypeNameNotFound, "HocusPocus").WithArguments("HocusPocus").WithLocation(6, 26) + ); + } + [Fact] public void SimpleCaseOfCustomFixedGeneric() { @@ -4668,7 +4802,7 @@ public Fixable(int arg){} public static class FixableExt { - public static ref int DangerousGetPinnableReference(in this Fixable f) + public static ref readonly int DangerousGetPinnableReference(in this Fixable f) { return ref (new int[]{1,2,3})[0]; } @@ -4689,7 +4823,7 @@ .locals init (Fixable V_0, //f IL_0001: newobj ""Fixable..ctor(int)"" IL_0006: stloc.2 IL_0007: ldloca.s V_2 - IL_0009: call ""ref int FixableExt.DangerousGetPinnableReference(in Fixable)"" + IL_0009: call ""ref readonly int FixableExt.DangerousGetPinnableReference(in Fixable)"" IL_000e: stloc.1 IL_000f: ldloc.1 IL_0010: conv.u @@ -4704,7 +4838,7 @@ .locals init (Fixable V_0, //f IL_001e: ldc.i4.1 IL_001f: call ""Fixable..ctor(int)"" IL_0024: ldloca.s V_0 - IL_0026: call ""ref int FixableExt.DangerousGetPinnableReference(in Fixable)"" + IL_0026: call ""ref readonly int FixableExt.DangerousGetPinnableReference(in Fixable)"" IL_002b: stloc.1 IL_002c: ldloc.1 IL_002d: conv.u From 528b1712b8ea15396f2f07b30667b3fb6148ee3f Mon Sep 17 00:00:00 2001 From: vsadov Date: Tue, 20 Feb 2018 15:59:27 -0800 Subject: [PATCH 10/10] More PR feedback --- .../CSharp/Portable/Binder/Binder_Statements.cs | 2 +- .../LocalRewriter/LocalRewriter_FixedStatement.cs | 14 ++++++++------ 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Binder/Binder_Statements.cs b/src/Compilers/CSharp/Portable/Binder/Binder_Statements.cs index b9e4745c7209b..c229b12773d96 100644 --- a/src/Compilers/CSharp/Portable/Binder/Binder_Statements.cs +++ b/src/Compilers/CSharp/Portable/Binder/Binder_Statements.cs @@ -1109,7 +1109,7 @@ private bool IsValidFixedVariableInitializer(TypeSymbol declType, SourceLocalSym } else { - // if feature was enabled but somethng went wrong with the method, report that + // if the feature was enabled but something went wrong with the method, report that bool extensibleFixedEnabled = ((CSharpParseOptions)initializerOpt.SyntaxTree.Options)?.IsFeatureEnabled(MessageID.IDS_FeatureExtensibleFixedStatement) != false; if (extensibleFixedEnabled && additionalDiagnostics != null) { diff --git a/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_FixedStatement.cs b/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_FixedStatement.cs index 5482d950ac725..dded4032e1b2e 100644 --- a/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_FixedStatement.cs +++ b/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_FixedStatement.cs @@ -225,7 +225,7 @@ private BoundStatement InitializeFixedStatementLocal( /// /// + /// fixed(int* ptr = &v){ ... } == becomes ===> /// /// pinned ref int pinnedTemp = ref v; // pinning managed ref /// int* ptr = (int*)&pinnedTemp; // unsafe cast to unmanaged ptr @@ -273,7 +273,7 @@ private BoundStatement InitializeFixedStatementRegularLocal( // pinnedTemp = ref v; BoundStatement pinnedTempInit = factory.Assignment(factory.Local(pinnedTemp), initializerExpr, isRef: true); - // &pinnedTemp; + // &pinnedTemp var addr = new BoundAddressOfOperator( factory.Syntax, factory.Local(pinnedTemp), @@ -294,7 +294,7 @@ private BoundStatement InitializeFixedStatementRegularLocal( /// /// + /// fixed(int* ptr = &v){ ... } == becomes ===> /// /// pinned ref int pinnedTemp = ref v; // pinning managed ref /// int* ptr = (int*)&pinnedTemp; // unsafe cast to unmanaged ptr @@ -358,7 +358,7 @@ private BoundStatement InitializeFixedStatementGetPinnable( getPinnableCall, isRef: true); - // &pinnedTemp; + // &pinnedTemp var addr = new BoundAddressOfOperator( factory.Syntax, factory.Local(pinnedTemp), @@ -464,13 +464,15 @@ private BoundStatement InitializeFixedStatementStringLocal( } /// + /// /// /// pinned int[] pinnedTemp = arr; // pinning managed ref - /// int* ptr = pinnedTemp != null && pinnedTemp.Length != 0 - /// (int*)&pinnedTemp[0]: // unsafe cast to unmanaged ptr + /// int* ptr = pinnedTemp != null && pinnedTemp.Length != 0 + /// (int*)&pinnedTemp[0]: // unsafe cast to unmanaged ptr /// 0; /// . . . + /// ]]> /// private BoundStatement InitializeFixedStatementArrayLocal( BoundLocalDeclaration localDecl,