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.

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

Follow up on LDM decisions made 2022-01-26 #59229

Merged

Conversation

AlekseyTs
Copy link
Contributor

@AlekseyTs AlekseyTs commented Feb 3, 2022

@RikkiGibson
Copy link
Contributor

It looks like the change may have broken the bootstrap build.

@AlekseyTs
Copy link
Contributor Author

@cston, @dotnet/roslyn-compiler For the second review.

@AlekseyTs
Copy link
Contributor Author

Comment on lines 956 to 970
// PROTOTYPE(UTF8StringLiterals) : An explicit cast is added to work aroung the break -
// error CS0121: The call is ambiguous between the following methods or properties: 'ObjectWriter.WriteValue(object ?)' and 'ObjectWriter.WriteValue(ReadOnlySpan<byte>)'
writer.WriteValue((object)"YYY");

// PROTOTYPE(UTF8StringLiterals) : An explicit cast is added to work aroung the break -
// error CS0121: The call is ambiguous between the following methods or properties: 'ObjectWriter.WriteValue(object ?)' and 'ObjectWriter.WriteValue(ReadOnlySpan<byte>)'
writer.WriteValue((object)"\uD800\uDC00"); // valid surrogate pair

// PROTOTYPE(UTF8StringLiterals) : An explicit cast is added to work aroung the break -
// error CS0121: The call is ambiguous between the following methods or properties: 'ObjectWriter.WriteValue(object ?)' and 'ObjectWriter.WriteValue(ReadOnlySpan<byte>)'
writer.WriteValue((object)"\uDC00\uD800"); // invalid surrogate pair

// PROTOTYPE(UTF8StringLiterals) : An explicit cast is added to work aroung the break -
// error CS0121: The call is ambiguous between the following methods or properties: 'ObjectWriter.WriteValue(object ?)' and 'ObjectWriter.WriteValue(ReadOnlySpan<byte>)'
writer.WriteValue((object)"\uD800"); // incomplete surrogate pair
Copy link
Member

@cston cston Feb 4, 2022

Choose a reason for hiding this comment

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

Suggested change
// PROTOTYPE(UTF8StringLiterals) : An explicit cast is added to work aroung the break -
// error CS0121: The call is ambiguous between the following methods or properties: 'ObjectWriter.WriteValue(object ?)' and 'ObjectWriter.WriteValue(ReadOnlySpan<byte>)'
writer.WriteValue((object)"YYY");
// PROTOTYPE(UTF8StringLiterals) : An explicit cast is added to work aroung the break -
// error CS0121: The call is ambiguous between the following methods or properties: 'ObjectWriter.WriteValue(object ?)' and 'ObjectWriter.WriteValue(ReadOnlySpan<byte>)'
writer.WriteValue((object)"\uD800\uDC00"); // valid surrogate pair
// PROTOTYPE(UTF8StringLiterals) : An explicit cast is added to work aroung the break -
// error CS0121: The call is ambiguous between the following methods or properties: 'ObjectWriter.WriteValue(object ?)' and 'ObjectWriter.WriteValue(ReadOnlySpan<byte>)'
writer.WriteValue((object)"\uDC00\uD800"); // invalid surrogate pair
// PROTOTYPE(UTF8StringLiterals) : An explicit cast is added to work aroung the break -
// error CS0121: The call is ambiguous between the following methods or properties: 'ObjectWriter.WriteValue(object ?)' and 'ObjectWriter.WriteValue(ReadOnlySpan<byte>)'
writer.WriteValue((object)"\uD800"); // incomplete surrogate pair
// PROTOTYPE(UTF8StringLiterals) : An explicit cast is added to work around the break -
// error CS0121: The call is ambiguous between the following methods or properties: 'ObjectWriter.WriteValue(object ?)' and 'ObjectWriter.WriteValue(ReadOnlySpan<byte>)'
writer.WriteValue((object)"YYY");
// PROTOTYPE(UTF8StringLiterals) : An explicit cast is added to work around the break -
// error CS0121: The call is ambiguous between the following methods or properties: 'ObjectWriter.WriteValue(object ?)' and 'ObjectWriter.WriteValue(ReadOnlySpan<byte>)'
writer.WriteValue((object)"\uD800\uDC00"); // valid surrogate pair
// PROTOTYPE(UTF8StringLiterals) : An explicit cast is added to work around the break -
// error CS0121: The call is ambiguous between the following methods or properties: 'ObjectWriter.WriteValue(object ?)' and 'ObjectWriter.WriteValue(ReadOnlySpan<byte>)'
writer.WriteValue((object)"\uDC00\uD800"); // invalid surrogate pair
// PROTOTYPE(UTF8StringLiterals) : An explicit cast is added to work around the break -
// error CS0121: The call is ambiguous between the following methods or properties: 'ObjectWriter.WriteValue(object ?)' and 'ObjectWriter.WriteValue(ReadOnlySpan<byte>)'
writer.WriteValue((object)"\uD800"); // incomplete surrogate pair
``` #Closed

@@ -6947,4 +6947,7 @@ To remove the warning, you can use /reference instead (set the Embed Interop Typ
<data name="IDS_FeatureUTF8StringLiterals" xml:space="preserve">
<value>Utf8 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>
Copy link
Member

@cston cston Feb 4, 2022

Choose a reason for hiding this comment

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

UTF-8

Consider using consistent form for these three messages - perhaps "UTF8". #Closed

);
}

[ConditionalFact(typeof(CoreClrOnly))]
Copy link
Member

@cston cston Feb 5, 2022

Choose a reason for hiding this comment

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

[ConditionalFact(typeof(CoreClrOnly))]

[Fact] #Closed

@@ -1829,7 +1953,8 @@ static void Main()
";
var comp = CreateCompilation(source, targetFramework: TargetFramework.NetCoreApp, options: TestOptions.DebugExe);

CompileAndVerify(comp, expectedOutput: @"ReadOnlySpan").VerifyDiagnostics();
// PROTOTYPE(UTF8StringLiterals) : Add an entry in "docs/compilers/CSharp/Compiler Breaking Changes - DotNet 7.md"?
CompileAndVerify(comp, expectedOutput: @"array").VerifyDiagnostics();
Copy link
Member

@cston cston Feb 5, 2022

Choose a reason for hiding this comment

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

@"array"

Since this is a change in behavior, please include a corresponding compilation using TestOptions.Regular10. (Will it report "feature is in preview"?) #Closed

var comp = CreateCompilation(source, targetFramework: TargetFramework.NetCoreApp, options: TestOptions.DebugExe);

// PROTOTYPE(UTF8StringLiterals) : Add an entry in "docs/compilers/CSharp/Compiler Breaking Changes - DotNet 7.md"?
CompileAndVerify(comp, expectedOutput: @"array").VerifyDiagnostics();
Copy link
Member

@cston cston Feb 5, 2022

Choose a reason for hiding this comment

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

CompileAndVerify(comp, expectedOutput: @"array").VerifyDiagnostics();

Since this is a change in behavior, please include a corresponding compilation using TestOptions.Regular10. #Closed

@@ -216,7 +272,7 @@ public void Errors_07()
}

[Fact]
public void Errors_08()
public void Errors_07()
{
// The behavior is consistent with how type suffixes are handled on numeric literals, see Errors_07.
Copy link
Member

@cston cston Feb 5, 2022

Choose a reason for hiding this comment

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

Suggested change
// The behavior is consistent with how type suffixes are handled on numeric literals, see Errors_07.
// The behavior is consistent with how type suffixes are handled on numeric literals, see Errors_06.
``` #Closed

}

[Fact]
public void Errors_05()
{
// The behavior is consistent with how type suffixes are handled on numeric literals, see Errors_06.
Copy link
Member

@cston cston Feb 5, 2022

Choose a reason for hiding this comment

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

Suggested change
// The behavior is consistent with how type suffixes are handled on numeric literals, see Errors_06.
// The behavior is consistent with how type suffixes are handled on numeric literals, see Errors_05.
``` #Closed

@cston
Copy link
Member

cston commented Feb 5, 2022

        // The behavior is consistent with how type suffixes are handled on numeric literals, see Errors_07.

Errors_06


In reply to: 1030458751


In reply to: 1030458751


Refers to: src/Compilers/CSharp/Test/Syntax/Parsing/UTF8StringLiteralsParsingTests.cs:179 in 5a41f5a. [](commit_id = 5a41f5a, deletion_comment = False)

}

[Fact]
public void Errors_16()
{
// The behavior is consistent with how type suffixes are handled on numeric literals, see Errors_06.
Copy link
Member

@cston cston Feb 5, 2022

Choose a reason for hiding this comment

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

Suggested change
// The behavior is consistent with how type suffixes are handled on numeric literals, see Errors_06.
// The behavior is consistent with how type suffixes are handled on numeric literals, see Errors_05.
``` #Closed

@AlekseyTs
Copy link
Contributor Author

@cston I think I addressed your feedback. Please review.

@AlekseyTs AlekseyTs enabled auto-merge (squash) February 8, 2022 17:06
@AlekseyTs AlekseyTs merged commit 94bfc22 into dotnet:features/Utf8StringLiterals Feb 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants