Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change natural type of UTF-8 string literals to ReadOnlySpan<byte> and null terminate the underlying blob. #61532

Merged
merged 3 commits into from
May 27, 2022
Merged

Change natural type of UTF-8 string literals to ReadOnlySpan<byte> and null terminate the underlying blob. #61532

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
afe8eb0
Change natural type of UTF-8 string literals to `ReadOnlySpan<byte>` …
AlekseyTs May 26, 2022
6abcd69
PR feedback and test adjustments
AlekseyTs May 26, 2022
848008d
Merge two parameters
AlekseyTs May 27, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 0 additions & 53 deletions docs/compilers/CSharp/Compiler Breaking Changes - DotNet 7.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,59 +97,6 @@ A possible workaround is to switch to using `>>>` operator:
static C1 Test1(C1 x, int y) => x >>> y;
```

## UTF8 String Literal conversion

***Introduced in .NET SDK 6.0.400, Visual Studio 2022 version 17.3.***
The language added conversions between `string` constants and `byte` sequences
where the text is converted into the equivalent UTF8 byte representation.
Specifically the compiler allowed an implicit conversions from **`string` constants**
to `byte[]`, `Span<byte>`, and `ReadOnlySpan<byte>` types.

The conversions can lead to an overload resolution failure due to an ambiguity for a code
that compiled successfully before. For example:
``` C#
Test("s"); // error CS0121: The call is ambiguous between the following methods or properties: 'C.Test(ReadOnlySpan<char>)' and 'C.Test(byte[])'

static string Test(ReadOnlySpan<char> a) => "ReadOnlySpan";
static string Test(byte[] a) => "array";
```

A possible workaround is to apply an explicit cast to the constant string argument.

The conversions can lead to an invocation of a different member. For example:
``` C#
Test("s", (int)1); // Used to call `Test(ReadOnlySpan<char> a, long x)`, but calls `Test(byte[] a, int x)` now

static string Test(ReadOnlySpan<char> a, long x) => "ReadOnlySpan";
static string Test(byte[] a, int x) => "array";
```

A possible workaround is to apply an explicit cast to the constant string argument.

The conversions can lead to an invocation of an instance member where an extension method used to be invoked.
For example:
``` C#
class Program
{
static void Main()
{
var p = new Program();
p.M(""); // Used to call E.M, but calls Program.M now
}

public string M(byte[] b) => "byte[]";
}

static class E
{
public static string M(this object o, string s) => "string";
}
```

Possible workarounds are:
1. Apply an explicit cast to the constant string argument.
2. Call the extension method by using static method invocation syntax.

## Foreach enumerator as a ref struct

***Introduced in .NET SDK 6.0.300, Visual Studio 2022 version 17.2.*** A `foreach` using a ref struct enumerator type reports an error if the language version is set to 7.3 or earlier.
Expand Down
3 changes: 2 additions & 1 deletion ...Analyzers/CSharp/Analyzers/UseUTF8StringLiteral/UseUTF8StringLiteralDiagnosticAnalyzer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,8 @@ protected override void InitializeWorker(AnalysisContext context)

var expressionType = context.Compilation.GetTypeByMetadataName(typeof(System.Linq.Expressions.Expression<>).FullName!);

context.RegisterOperationAction(c => AnalyzeOperation(c, expressionType), OperationKind.ArrayCreation);
// Temporarily disabling, https://github.com/dotnet/roslyn/issues/61517 tracks the follow up work
// context.RegisterOperationAction(c => AnalyzeOperation(c, expressionType), OperationKind.ArrayCreation);
});

private void AnalyzeOperation(OperationAnalysisContext context, INamedTypeSymbol? expressionType)
Expand Down
64 changes: 32 additions & 32 deletions src/Analyzers/CSharp/Tests/UseUTF8StringLiteral/UseUTF8StringLiteralTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ public void M()
}.RunAsync();
}

[Fact, Trait(Traits.Feature, Traits.Features.CodeActionsUseUTF8StringLiteral)]
[Fact(Skip = "https://github.com/dotnet/roslyn/issues/61517"), Trait(Traits.Feature, Traits.Features.CodeActionsUseUTF8StringLiteral)]
public async Task TestSimpleByteArray()
{
await new VerifyCS.Test
Expand Down Expand Up @@ -211,7 +211,7 @@ public void M()
}.RunAsync();
}

[Fact, Trait(Traits.Feature, Traits.Features.CodeActionsUseUTF8StringLiteral)]
[Fact(Skip = "https://github.com/dotnet/roslyn/issues/61517"), Trait(Traits.Feature, Traits.Features.CodeActionsUseUTF8StringLiteral)]
public async Task TestConstant()
{
await new VerifyCS.Test
Expand Down Expand Up @@ -241,7 +241,7 @@ public void M()
}.RunAsync();
}

[Fact, Trait(Traits.Feature, Traits.Features.CodeActionsUseUTF8StringLiteral)]
[Fact(Skip = "https://github.com/dotnet/roslyn/issues/61517"), Trait(Traits.Feature, Traits.Features.CodeActionsUseUTF8StringLiteral)]
public async Task TestImplicitArray()
{
await new VerifyCS.Test
Expand Down Expand Up @@ -269,7 +269,7 @@ public void M()
}.RunAsync();
}

[Fact, Trait(Traits.Feature, Traits.Features.CodeActionsUseUTF8StringLiteral)]
[Fact(Skip = "https://github.com/dotnet/roslyn/issues/61517"), Trait(Traits.Feature, Traits.Features.CodeActionsUseUTF8StringLiteral)]
public async Task TestExplicitCast()
{
await new VerifyCS.Test
Expand Down Expand Up @@ -297,7 +297,7 @@ public void M()
}.RunAsync();
}

[Fact, Trait(Traits.Feature, Traits.Features.CodeActionsUseUTF8StringLiteral)]
[Fact(Skip = "https://github.com/dotnet/roslyn/issues/61517"), Trait(Traits.Feature, Traits.Features.CodeActionsUseUTF8StringLiteral)]
public async Task TestHexLiteral()
{
await new VerifyCS.Test
Expand Down Expand Up @@ -325,7 +325,7 @@ public void M()
}.RunAsync();
}

[Fact, Trait(Traits.Feature, Traits.Features.CodeActionsUseUTF8StringLiteral)]
[Fact(Skip = "https://github.com/dotnet/roslyn/issues/61517"), Trait(Traits.Feature, Traits.Features.CodeActionsUseUTF8StringLiteral)]
public async Task TestBinaryExpression()
{
await new VerifyCS.Test
Expand Down Expand Up @@ -353,7 +353,7 @@ public void M()
}.RunAsync();
}

[Fact, Trait(Traits.Feature, Traits.Features.CodeActionsUseUTF8StringLiteral)]
[Fact(Skip = "https://github.com/dotnet/roslyn/issues/61517"), Trait(Traits.Feature, Traits.Features.CodeActionsUseUTF8StringLiteral)]
public async Task TestEmptyArray()
{
await new VerifyCS.Test
Expand All @@ -380,7 +380,7 @@ public void M()
}.RunAsync();
}

[Fact, Trait(Traits.Feature, Traits.Features.CodeActionsUseUTF8StringLiteral)]
[Fact(Skip = "https://github.com/dotnet/roslyn/issues/61517"), Trait(Traits.Feature, Traits.Features.CodeActionsUseUTF8StringLiteral)]
public async Task TestTrivia1()
{
await new VerifyCS.Test
Expand Down Expand Up @@ -408,7 +408,7 @@ public void M()
}.RunAsync();
}

[Fact, Trait(Traits.Feature, Traits.Features.CodeActionsUseUTF8StringLiteral)]
[Fact(Skip = "https://github.com/dotnet/roslyn/issues/61517"), Trait(Traits.Feature, Traits.Features.CodeActionsUseUTF8StringLiteral)]
public async Task TestTrivia2()
{
await new VerifyCS.Test
Expand Down Expand Up @@ -436,7 +436,7 @@ public void M(byte[] b)
}.RunAsync();
}

[Fact, Trait(Traits.Feature, Traits.Features.CodeActionsUseUTF8StringLiteral)]
[Fact(Skip = "https://github.com/dotnet/roslyn/issues/61517"), Trait(Traits.Feature, Traits.Features.CodeActionsUseUTF8StringLiteral)]
public async Task TestMultiple()
{
await new VerifyCS.Test
Expand Down Expand Up @@ -468,7 +468,7 @@ public void M()
}.RunAsync();
}

[Fact, Trait(Traits.Feature, Traits.Features.CodeActionsUseUTF8StringLiteral)]
[Fact(Skip = "https://github.com/dotnet/roslyn/issues/61517"), Trait(Traits.Feature, Traits.Features.CodeActionsUseUTF8StringLiteral)]
public async Task TestEscapeChars()
{
await new VerifyCS.Test
Expand Down Expand Up @@ -496,7 +496,7 @@ public void M()
}.RunAsync();
}

[Fact, Trait(Traits.Feature, Traits.Features.CodeActionsUseUTF8StringLiteral)]
[Fact(Skip = "https://github.com/dotnet/roslyn/issues/61517"), Trait(Traits.Feature, Traits.Features.CodeActionsUseUTF8StringLiteral)]
public async Task TestEmoji()
{
await new VerifyCS.Test
Expand Down Expand Up @@ -581,7 +581,7 @@ public void M()
}.RunAsync();
}

[Fact, Trait(Traits.Feature, Traits.Features.CodeActionsUseUTF8StringLiteral)]
[Fact(Skip = "https://github.com/dotnet/roslyn/issues/61517"), Trait(Traits.Feature, Traits.Features.CodeActionsUseUTF8StringLiteral)]
public async Task TestUnicodeReplacementChar()
{
// The unicode replacement character is what is returned when, for example, an unpaired
Expand Down Expand Up @@ -612,7 +612,7 @@ public void M()
}.RunAsync();
}

[Fact, Trait(Traits.Feature, Traits.Features.CodeActionsUseUTF8StringLiteral)]
[Fact(Skip = "https://github.com/dotnet/roslyn/issues/61517"), Trait(Traits.Feature, Traits.Features.CodeActionsUseUTF8StringLiteral)]
public async Task TestCollectionInitializer()
{
await new VerifyCS.Test
Expand Down Expand Up @@ -716,7 +716,7 @@ public void Dispose(int a = 1, bool b = true, params byte[] others) { }
}.RunAsync();
}

[Theory, Trait(Traits.Feature, Traits.Features.CodeActionsUseUTF8StringLiteral)]
[Theory(Skip = "https://github.com/dotnet/roslyn/issues/61517"), Trait(Traits.Feature, Traits.Features.CodeActionsUseUTF8StringLiteral)]
// Various cases copied from https://github.com/dotnet/runtime/blob/main/src/libraries/Common/tests/Tests/System/Net/aspnetcore/Http3/QPackDecoderTest.cs
[InlineData(new byte[] { 0x37, 0x02, 0x74, 0x72, 0x61, 0x6e, 0x73, 0x6c, 0x61, 0x74, 0x65 }, "7translate")]
[InlineData(new byte[] { 0x3f, 0x01 }, "?")]
Expand Down Expand Up @@ -782,7 +782,7 @@ public class C
Assert.NotEqual(bytes, newBytes);
}

[Fact, Trait(Traits.Feature, Traits.Features.CodeActionsUseUTF8StringLiteral)]
[Fact(Skip = "https://github.com/dotnet/roslyn/issues/61517"), Trait(Traits.Feature, Traits.Features.CodeActionsUseUTF8StringLiteral)]
public async Task TestParamArray1()
{
await new VerifyCS.Test
Expand Down Expand Up @@ -812,7 +812,7 @@ public void M(params byte[] b)
}.RunAsync();
}

[Fact, Trait(Traits.Feature, Traits.Features.CodeActionsUseUTF8StringLiteral)]
[Fact(Skip = "https://github.com/dotnet/roslyn/issues/61517"), Trait(Traits.Feature, Traits.Features.CodeActionsUseUTF8StringLiteral)]
public async Task TestParamArray2()
{
await new VerifyCS.Test
Expand Down Expand Up @@ -842,7 +842,7 @@ public void M(int i, params byte[] b)
}.RunAsync();
}

[Fact, Trait(Traits.Feature, Traits.Features.CodeActionsUseUTF8StringLiteral)]
[Fact(Skip = "https://github.com/dotnet/roslyn/issues/61517"), Trait(Traits.Feature, Traits.Features.CodeActionsUseUTF8StringLiteral)]
public async Task TestParamArray3()
{
await new VerifyCS.Test
Expand Down Expand Up @@ -872,7 +872,7 @@ public void M(params byte[] b)
}.RunAsync();
}

[Fact, Trait(Traits.Feature, Traits.Features.CodeActionsUseUTF8StringLiteral)]
[Fact(Skip = "https://github.com/dotnet/roslyn/issues/61517"), Trait(Traits.Feature, Traits.Features.CodeActionsUseUTF8StringLiteral)]
public async Task TestParamArray4()
{
await new VerifyCS.Test
Expand Down Expand Up @@ -902,7 +902,7 @@ public void M(params byte[] b)
}.RunAsync();
}

[Fact, Trait(Traits.Feature, Traits.Features.CodeActionsUseUTF8StringLiteral)]
[Fact(Skip = "https://github.com/dotnet/roslyn/issues/61517"), Trait(Traits.Feature, Traits.Features.CodeActionsUseUTF8StringLiteral)]
public async Task TestParamArray5()
{
await new VerifyCS.Test
Expand Down Expand Up @@ -932,7 +932,7 @@ public void M(params byte[] b)
}.RunAsync();
}

[Fact, Trait(Traits.Feature, Traits.Features.CodeActionsUseUTF8StringLiteral)]
[Fact(Skip = "https://github.com/dotnet/roslyn/issues/61517"), Trait(Traits.Feature, Traits.Features.CodeActionsUseUTF8StringLiteral)]
public async Task TestParamArray6()
{
await new VerifyCS.Test
Expand Down Expand Up @@ -981,7 +981,7 @@ public void M(int x, params byte[] b)
}.RunAsync();
}

[Fact, Trait(Traits.Feature, Traits.Features.CodeActionsUseUTF8StringLiteral)]
[Fact(Skip = "https://github.com/dotnet/roslyn/issues/61517"), Trait(Traits.Feature, Traits.Features.CodeActionsUseUTF8StringLiteral)]
public async Task TestParamArray8()
{
await new VerifyCS.Test
Expand Down Expand Up @@ -1011,7 +1011,7 @@ public void M(int x, params byte[] b)
}.RunAsync();
}

[Fact, Trait(Traits.Feature, Traits.Features.CodeActionsUseUTF8StringLiteral)]
[Fact(Skip = "https://github.com/dotnet/roslyn/issues/61517"), Trait(Traits.Feature, Traits.Features.CodeActionsUseUTF8StringLiteral)]
public async Task TestParamArray9()
{
await new VerifyCS.Test
Expand Down Expand Up @@ -1041,7 +1041,7 @@ public void M(int x, params byte[] b)
}.RunAsync();
}

[Fact, Trait(Traits.Feature, Traits.Features.CodeActionsUseUTF8StringLiteral)]
[Fact(Skip = "https://github.com/dotnet/roslyn/issues/61517"), Trait(Traits.Feature, Traits.Features.CodeActionsUseUTF8StringLiteral)]
public async Task TestParamArray10()
{
await new VerifyCS.Test
Expand Down Expand Up @@ -1071,7 +1071,7 @@ public void M(int x, params byte[] b)
}.RunAsync();
}

[Fact, Trait(Traits.Feature, Traits.Features.CodeActionsUseUTF8StringLiteral)]
[Fact(Skip = "https://github.com/dotnet/roslyn/issues/61517"), Trait(Traits.Feature, Traits.Features.CodeActionsUseUTF8StringLiteral)]
public async Task TestParamArray11()
{
await new VerifyCS.Test
Expand Down Expand Up @@ -1101,7 +1101,7 @@ public void M(int x, int y, int z, params byte[] b)
}.RunAsync();
}

[Fact, Trait(Traits.Feature, Traits.Features.CodeActionsUseUTF8StringLiteral)]
[Fact(Skip = "https://github.com/dotnet/roslyn/issues/61517"), Trait(Traits.Feature, Traits.Features.CodeActionsUseUTF8StringLiteral)]
public async Task TestParamArray12()
{
await new VerifyCS.Test
Expand Down Expand Up @@ -1131,7 +1131,7 @@ public C(params byte[] b)
}.RunAsync();
}

[Fact, Trait(Traits.Feature, Traits.Features.CodeActionsUseUTF8StringLiteral)]
[Fact(Skip = "https://github.com/dotnet/roslyn/issues/61517"), Trait(Traits.Feature, Traits.Features.CodeActionsUseUTF8StringLiteral)]
public async Task TestParamArray13()
{
await new VerifyCS.Test
Expand Down Expand Up @@ -1171,7 +1171,7 @@ public void M()
}.RunAsync();
}

[Fact, Trait(Traits.Feature, Traits.Features.CodeActionsUseUTF8StringLiteral)]
[Fact(Skip = "https://github.com/dotnet/roslyn/issues/61517"), Trait(Traits.Feature, Traits.Features.CodeActionsUseUTF8StringLiteral)]
public async Task TestParamArray14()
{
await new VerifyCS.Test
Expand Down Expand Up @@ -1225,7 +1225,7 @@ public sealed class IsExternalInit
}.RunAsync();
}

[Fact, Trait(Traits.Feature, Traits.Features.CodeActionsUseUTF8StringLiteral)]
[Fact(Skip = "https://github.com/dotnet/roslyn/issues/61517"), Trait(Traits.Feature, Traits.Features.CodeActionsUseUTF8StringLiteral)]
public async Task TestParamArray15()
{
await new VerifyCS.Test
Expand Down Expand Up @@ -1313,7 +1313,7 @@ public B(params byte[] bytes)
}.RunAsync();
}

[Fact, Trait(Traits.Feature, Traits.Features.CodeActionsUseUTF8StringLiteral)]
[Fact(Skip = "https://github.com/dotnet/roslyn/issues/61517"), Trait(Traits.Feature, Traits.Features.CodeActionsUseUTF8StringLiteral)]
public async Task TestParamArray16()
{
await new VerifyCS.Test
Expand Down Expand Up @@ -1343,7 +1343,7 @@ public void M(int[] i, byte[] b)
}.RunAsync();
}

[Fact, Trait(Traits.Feature, Traits.Features.CodeActionsUseUTF8StringLiteral)]
[Fact(Skip = "https://github.com/dotnet/roslyn/issues/61517"), Trait(Traits.Feature, Traits.Features.CodeActionsUseUTF8StringLiteral)]
public async Task TestParamArray17()
{
await new VerifyCS.Test
Expand Down Expand Up @@ -1373,7 +1373,7 @@ public void M(int[] i, params byte[] b)
}.RunAsync();
}

[Fact, Trait(Traits.Feature, Traits.Features.CodeActionsUseUTF8StringLiteral)]
[Fact(Skip = "https://github.com/dotnet/roslyn/issues/61517"), Trait(Traits.Feature, Traits.Features.CodeActionsUseUTF8StringLiteral)]
public async Task TestMultidimensionalArray()
{
await new VerifyCS.Test
Expand Down
2 changes: 2 additions & 0 deletions src/Compilers/CSharp/Portable/Binder/Binder.ValueChecks.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2613,6 +2613,7 @@ internal static uint GetValEscape(BoundExpression expr, uint scopeOfTheContainin
case BoundKind.DefaultExpression:
case BoundKind.Parameter:
case BoundKind.ThisReference:
case BoundKind.UTF8String:
Copy link
Member

@jcouv jcouv May 26, 2022 •
edited by AlekseyTs
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Usually, GetValEscape and GetRefEscape need to be updated in pair. Does GetRefEscape need to be updated too? #Resolved

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does GetRefEscape need to be updated too?

Based on the comment in the method, it is supposed to handle "all the possible cases for anything that is not a strict RValue". I think the literal is a strict RValue.

// always returnable
return Binder.ExternalScope;

Expand Down Expand Up @@ -3018,6 +3019,7 @@ internal static bool CheckValEscape(SyntaxNode node, BoundExpression expr, uint
case BoundKind.DefaultExpression:
case BoundKind.Parameter:
case BoundKind.ThisReference:
case BoundKind.UTF8String:
// always returnable
return true;

Expand Down
2 changes: 1 addition & 1 deletion src/Compilers/CSharp/Portable/Binder/Binder_Expressions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5925,7 +5925,7 @@ private BoundUTF8String BindUTF8StringLiteral(LiteralExpressionSyntax node, Bind
CheckFeatureAvailability(node, MessageID.IDS_FeatureUTF8StringLiterals, diagnostics);

var value = (string)node.Token.Value;
var type = ArrayTypeSymbol.CreateSZArray(Compilation.Assembly, TypeWithAnnotations.Create(GetSpecialType(SpecialType.System_Byte, diagnostics, node)));
var type = GetWellKnownType(WellKnownType.System_ReadOnlySpan_T, diagnostics, node).Construct(GetSpecialType(SpecialType.System_Byte, diagnostics, node));

return new BoundUTF8String(node, value, type);
}
Expand Down
3 changes: 0 additions & 3 deletions src/Compilers/CSharp/Portable/CSharpResources.resx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7061,9 +7061,6 @@ To remove the warning, you can use /reference instead (set the Embed Interop Typ
<data name="IDS_FeatureUTF8StringLiterals" xml:space="preserve">
<value>UTF-8 string literals</value>
</data>
<data name="ERR_ExpressionTreeContainsUTF8StringLiterals" xml:space="preserve">
<value>An expression tree may not contain UTF-8 string conversion or literal.</value>
</data>
<data name="IDS_FeatureUnsignedRightShift" xml:space="preserve">
<value>unsigned right shift</value>
</data>
Expand Down
Loading