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
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
1 change: 0 additions & 1 deletion src/Compilers/CSharp/Portable/Binder/Binder_Expressions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5913,7 +5913,6 @@ private BoundUTF8String BindUTF8StringLiteral(LiteralExpressionSyntax node, Bind
var value = (string)node.Token.Value;
var type = ArrayTypeSymbol.CreateSZArray(Compilation.Assembly, TypeWithAnnotations.Create(GetSpecialType(SpecialType.System_Byte, diagnostics, node)));

// PROTOTYPE(UTF8StringLiterals) : The result type is a deviation from the proposal, but I think it simplifies language rules. Will bring this for discussion to LDM.
return new BoundUTF8String(node, value, type);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -649,7 +649,8 @@ private Conversion ClassifyStandardImplicitConversion(BoundExpression sourceExpr
// with the exception of the switch expression conversion.

Conversion conversion = ClassifyImplicitBuiltInConversionFromExpression(sourceExpression, compilation, source, destination, ref useSiteInfo);
if (conversion.Exists)
if (conversion.Exists &&
!conversion.IsUtf8StringLiteral) // UTF-8 string conversion is not a standard conversion.
{
Debug.Assert(IsStandardImplicitConversionFromExpression(conversion.Kind));
return conversion;
Expand Down Expand Up @@ -1107,7 +1108,7 @@ private Conversion ClassifyImplicitUtf8StringLiteralConversion(BoundExpression s
ConstantValue constantValue = sourceExpression.ConstantValue;

TypeSymbol destinationOriginalDefinition = destination.OriginalDefinition;
if (constantValue?.IsString == true && // PROTOTYPE(UTF8StringLiterals) : confirm if we actually want it to work with 'null' constant value.
if (constantValue is ({ IsString: true } or { IsNull: true }) &&
sourceExpression.Type?.SpecialType == SpecialType.System_String &&
(destination is ArrayTypeSymbol { IsSZArray: true, ElementType.SpecialType: SpecialType.System_Byte } || // byte[]
(destinationOriginalDefinition.TypeKind == TypeKind.Struct && destinationOriginalDefinition.IsRefLikeType &&
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1669,19 +1669,6 @@ private BetterResult BetterFunctionMember<TMember>(
Debug.Assert(m2.Result.IsValid);
Debug.Assert(arguments != null);

// Prefer overloads that did not use the UTF8 string literal conversion to convert arguments.
// PROTOTYPE(UTF8StringLiterals) : Confirm this is the behavior we want. Similar to function types, this
// doesn't detect user defined conversions that became applicable due to
// UTF8 string literal conversions. But at the moment we do not allow
// implicit conversions like that and they are never applicable.
switch (RequiredUTF8StringLiteralConversion(m1), RequiredUTF8StringLiteralConversion(m2))
{
case (false, true):
return BetterResult.Left;
case (true, false):
return BetterResult.Right;
}

// Prefer overloads that did not use the inferred type of lambdas or method groups
// to infer generic method type arguments or to convert arguments.
switch (RequiredFunctionType(m1), RequiredFunctionType(m2))
Expand Down Expand Up @@ -2139,23 +2126,6 @@ private static bool RequiredFunctionType<TMember>(MemberResolutionResult<TMember
return conversionsOpt.Any(c => c.Kind == ConversionKind.FunctionType);
}

/// <summary>
/// Returns true if the overload required a UTF8 string literal conversion to parameter types.
/// </summary>
private static bool RequiredUTF8StringLiteralConversion<TMember>(MemberResolutionResult<TMember> m)
where TMember : Symbol
{
Debug.Assert(m.Result.IsValid);

var conversionsOpt = m.Result.ConversionsOpt;
if (conversionsOpt.IsDefault)
{
return false;
}

return conversionsOpt.Any(c => c.Kind == ConversionKind.ImplicitUtf8StringLiteral);
}

private static BetterResult PreferValOverInOrRefInterpolatedHandlerParameters<TMember>(
ArrayBuilder<BoundExpression> arguments,
MemberResolutionResult<TMember> m1,
Expand Down
3 changes: 3 additions & 0 deletions src/Compilers/CSharp/Portable/CSharpResources.resx
Original file line number Diff line number Diff line change
Expand Up @@ -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

</data>
</root>
1 change: 1 addition & 0 deletions src/Compilers/CSharp/Portable/Errors/ErrorCode.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2028,6 +2028,7 @@ internal enum ErrorCode
WRN_NullCheckingOnNullableType = 8995,

ERR_CannotBeConvertedToUTF8 = 9100, // PROTOTYPE(UTF8StringLiterals) : pack numbers
ERR_ExpressionTreeContainsUTF8StringLiterals = 9101,

#endregion

Expand Down
9 changes: 8 additions & 1 deletion src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs
Original file line number Diff line number Diff line change
Expand Up @@ -7723,7 +7723,14 @@ private TypeWithState VisitConversion(
break;

case ConversionKind.ImplicitUtf8StringLiteral:
resultState = NullableFlowState.NotNull; // PROTOTYPE(UTF8StringLiterals) : Adjust if we actually want it to work with 'null' value.
if (targetType.IsReferenceType)
{
resultState = getConversionResultState(operandType);
}
else
{
resultState = NullableFlowState.NotNull;
}
break;

case ConversionKind.ObjectCreation:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -761,6 +761,13 @@ public override BoundNode VisitConversion(BoundConversion node)
}
break;

case ConversionKind.ImplicitUtf8StringLiteral:
if (_inExpressionLambda)
{
Error(ErrorCode.ERR_ExpressionTreeContainsUTF8StringLiterals, node);
}
break;

default:

if (_inExpressionLambda && node.Conversion.Method is MethodSymbol method && method.IsAbstract && method.IsStatic)
Expand All @@ -776,6 +783,16 @@ public override BoundNode VisitConversion(BoundConversion node)
return result;
}

public override BoundNode VisitUTF8String(BoundUTF8String node)
{
if (_inExpressionLambda)
{
Error(ErrorCode.ERR_ExpressionTreeContainsUTF8StringLiterals, node);
}

return null;
}

public override BoundNode VisitDelegateCreationExpression(BoundDelegateCreationExpression node)
{
if (node.Argument.Kind != BoundKind.MethodGroup)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,11 @@ public override BoundNode VisitConversion(BoundConversion node)
private BoundNode RewriteUtf8StringLiteralConversion(BoundConversion node)
{
string? value = node.Operand.ConstantValue?.StringValue;
Debug.Assert(value != null); // PROTOTYPE(UTF8StringLiterals) : Adjust if we actually want it to work with 'null' value.

if (value == null)
{
return new BoundDefaultExpression(node.Syntax, node.Type);
}

ArrayTypeSymbol byteArray;

Expand Down
19 changes: 13 additions & 6 deletions src/Compilers/CSharp/Portable/Parser/Lexer_StringLiteral.cs
Original file line number Diff line number Diff line change
Expand Up @@ -77,11 +77,9 @@ private void ScanStringLiteral(ref TokenInfo info, bool inDirective)
}
else
{
// PROTOTYPE(UTF8StringLiterals) : Should the suffix be case-insensitive?
if (!inDirective && TextWindow.PeekChar() == 'u' && TextWindow.PeekChar(1) == '8')
if (!inDirective && ScanUTF8Suffix())
{
info.Kind = SyntaxKind.UTF8StringLiteralToken;
TextWindow.AdvanceChar(2);
}
else
{
Expand All @@ -101,6 +99,17 @@ private void ScanStringLiteral(ref TokenInfo info, bool inDirective)
}
}

private bool ScanUTF8Suffix()
{
if (TextWindow.PeekChar() is ('u' or 'U') && TextWindow.PeekChar(1) == '8')
{
TextWindow.AdvanceChar(2);
return true;
}

return false;
}

private char ScanEscapeSequence(out char surrogateCharacter)
{
var start = TextWindow.Position;
Expand Down Expand Up @@ -194,11 +203,9 @@ private void ScanVerbatimStringLiteral(ref TokenInfo info)
_builder.Append(ch);
}

// PROTOTYPE(UTF8StringLiterals) : Should the suffix be case-insensitive?
if (TextWindow.PeekChar() == 'u' && TextWindow.PeekChar(1) == '8')
if (ScanUTF8Suffix())
{
info.Kind = SyntaxKind.UTF8StringLiteralToken;
TextWindow.AdvanceChar(2);
}
else
{
Expand Down
5 changes: 5 additions & 0 deletions src/Compilers/CSharp/Portable/xlf/CSharpResources.cs.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions src/Compilers/CSharp/Portable/xlf/CSharpResources.de.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions src/Compilers/CSharp/Portable/xlf/CSharpResources.es.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions src/Compilers/CSharp/Portable/xlf/CSharpResources.fr.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions src/Compilers/CSharp/Portable/xlf/CSharpResources.it.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions src/Compilers/CSharp/Portable/xlf/CSharpResources.ja.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions src/Compilers/CSharp/Portable/xlf/CSharpResources.ko.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions src/Compilers/CSharp/Portable/xlf/CSharpResources.pl.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions src/Compilers/CSharp/Portable/xlf/CSharpResources.pt-BR.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions src/Compilers/CSharp/Portable/xlf/CSharpResources.ru.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions src/Compilers/CSharp/Portable/xlf/CSharpResources.tr.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions src/Compilers/CSharp/Portable/xlf/CSharpResources.zh-Hans.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions src/Compilers/CSharp/Portable/xlf/CSharpResources.zh-Hant.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading