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

Conversion expression rewrite #21040

Merged
Merged
Show file tree
Hide file tree
Changes from 18 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
bc55977
Adds initial IConverisonExpression API rewrite.
333fred Jul 21, 2017
f2c328d
Updated documentation, added ThrowsExceptionOnFailure field
333fred Jul 21, 2017
0158878
Moved fromm an interface IConversion to a CommonConversion struct.
333fred Jul 26, 2017
c850505
Added IsChecked bool property
333fred Jul 26, 2017
9815e73
Use TryCast for cleaner code
333fred Jul 26, 2017
eb9fdb7
Remove period from error message
333fred Jul 26, 2017
fcae24e
Addressed minor feedback items.
333fred Jul 27, 2017
ac70ad0
Removed virtual from MethodName property, made Conversion a synthesiz…
333fred Aug 1, 2017
2be4107
Move Language to IOperation root.
333fred Aug 1, 2017
62c84ab
Move ThrowsExceptionOnFailure to IsTryCast, invert logic for this.
333fred Aug 1, 2017
1094d2b
Updated test infrastructure code.
333fred Aug 2, 2017
08b5bc3
Updated expected tests output for all tests involving IConversionExpr…
333fred Aug 4, 2017
28ac13f
Merge remote-tracking branch 'dotnet/features/ioperation' into conver…
333fred Aug 4, 2017
90d781e
Moved OperationCloner to semantic model, fixed non-Compilers.sln comp…
333fred Aug 4, 2017
4d1573a
Revert resources change
333fred Aug 4, 2017
f00bd83
More test updates. Added checked tests and fixed checked bug.
333fred Aug 4, 2017
34a897e
Moved GetCSharp/VBConversion to GetConversion, into CSharp/VBExtensio…
333fred Aug 4, 2017
c1b0ca3
Removed LanguageName from ConversionExpression
333fred Aug 4, 2017
aed4959
Revert case changes in VBOperationFactory, make VBOperationCloner Rea…
333fred Aug 4, 2017
9c0a391
Move CloneOperation to not be generic calls all the way down.
333fred Aug 4, 2017
65f5f24
Removed boxing HasFlag usage from CommonConversion.
333fred Aug 5, 2017
5d2ffcb
Fixed IConversionExpression comment.
333fred Aug 5, 2017
ee30c32
Fixed failing unit tests, sorted and removed unnecessary usings in Co…
333fred Aug 6, 2017
8a61410
Merge remote-tracking branch 'dotnet/features/ioperation' into conver…
333fred Aug 7, 2017
901b072
Merge remote-tracking branch 'dotnet/features/ioperation' into conver…
333fred Aug 7, 2017
d4d8834
Merge remote-tracking branch 'dotnet/features/ioperation' into conver…
333fred Aug 8, 2017
f2d8739
Tagged new unit tests with IOperation tag.
333fred Aug 8, 2017
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
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
using System;
using System.Collections.Immutable;
using System.Diagnostics;
using Microsoft.CodeAnalysis.Semantics;

namespace Microsoft.CodeAnalysis.CSharp
{
Expand Down Expand Up @@ -843,6 +844,21 @@ internal UserDefinedConversionAnalysis BestUserDefinedConversionAnalysis
}
}

/// <summary>
/// Creates a <seealso cref="CommonConversion"/> from this C# conversion.
/// </summary>
/// <returns>The <see cref="CommonConversion"/> that represents this conversion.</returns>
/// <remarks>
/// This is a lossy conversion; it is not possible to recover the original <see cref="Conversion"/>
/// from the <see cref="CommonConversion"/> struct.
/// </remarks>
public CommonConversion ToCommonConversion()
{
// The MethodSymbol of CommonConversion only refers to UserDefined conversions, not method groups
var methodSymbol = IsUserDefined ? MethodSymbol : null;
return new CommonConversion(Exists, IsIdentity, IsNumeric, IsReference, methodSymbol);
}

/// <summary>
/// Returns a string that represents the <see cref="Kind"/> of the conversion.
/// </summary>
Expand Down
24 changes: 22 additions & 2 deletions src/Compilers/CSharp/Portable/CSharpExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,8 @@
using Microsoft.CodeAnalysis.CSharp;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.CodeAnalysis.CSharp.Symbols;
using Roslyn.Utilities;
using System.ComponentModel;
using Microsoft.CodeAnalysis.Syntax;
using Microsoft.CodeAnalysis.Semantics;

namespace Microsoft.CodeAnalysis
{
Expand Down Expand Up @@ -686,6 +685,27 @@ public static TypeInfo GetSpeculativeTypeInfo(this SemanticModel semanticModel,
}
}

/// <summary>
/// Gets the underlying <see cref="Conversion"/> information from this <see cref="IConversionExpression"/>. This
/// <see cref="IConversionExpression"/> must have been created from CSharp code.
/// </summary>
/// <param name="conversionExpression">The conversion expression to get original info from.</param>
/// <returns>The underlying <see cref="Conversion"/>.</returns>
/// <exception cref="InvalidCastException">If the <see cref="IConversionExpression"/> was not created from CSharp code.</exception>
public static Conversion GetConversion(this IConversionExpression conversionExpression)
{
if (conversionExpression is BaseCSharpConversionExpression csharpConversionExpression)
{
return csharpConversionExpression.ConversionInternal;
}
else
{
throw new ArgumentException(string.Format(CSharpResources.IConversionExpressionIsNotCSharpConversion,
nameof(IConversionExpression)),
nameof(conversionExpression));
}
}

public static Conversion GetSpeculativeConversion(this SemanticModel semanticModel, int position, ExpressionSyntax expression, SpeculativeBindingOption bindingOption)
{
var csmodel = semanticModel as CSharpSemanticModel;
Expand Down
9 changes: 9 additions & 0 deletions src/Compilers/CSharp/Portable/CSharpResources.Designer.cs

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

3 changes: 3 additions & 0 deletions src/Compilers/CSharp/Portable/CSharpResources.resx
Original file line number Diff line number Diff line change
Expand Up @@ -5108,4 +5108,7 @@ To remove the warning, you can use /reference instead (set the Embed Interop Typ
<data name="ERR_InvalidDebugInfo" xml:space="preserve">
<value>Unable to read debug information of method '{0}' (token 0x{1:X8}) from assembly '{2}'</value>
</data>
<data name="IConversionExpressionIsNotCSharpConversion" xml:space="preserve">
<value>{0} is not a valid C# conversion expression</value>
</data>
</root>
Original file line number Diff line number Diff line change
Expand Up @@ -485,6 +485,11 @@ internal virtual IOperation GetOperationWorker(CSharpSyntaxNode node, GetOperati
return null;
}

internal override T CloneOperation<T>(T operation)
{
return CSharpOperationCloner.Instance.Visit(operation);
}

#region GetSymbolInfo

/// <summary>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
// Copyright (c) Microsoft. All Rights Reserved. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System;
using Microsoft.CodeAnalysis.Semantics;

namespace Microsoft.CodeAnalysis.CSharp
{
internal abstract class BaseCSharpConversionExpression : BaseConversionExpression
{
protected BaseCSharpConversionExpression(Conversion conversion, bool isExplicitInCode, bool isTryCast, bool isChecked, SemanticModel semanticModel, SyntaxNode syntax, ITypeSymbol type, Optional<object> constantValue) :
base(isExplicitInCode, isTryCast, isChecked, semanticModel, syntax, type, constantValue)
{
ConversionInternal = conversion;
}

internal Conversion ConversionInternal { get; }
Copy link
Member Author

Choose a reason for hiding this comment

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

@cston style question: is the preferred strategy to have both the CommonConversion and Conversion struct around, or to perform the conversion when requested? ie public CommonConversion Conversion => (CommonConversion)ConversionInternal;

Copy link
Member

Choose a reason for hiding this comment

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

If creating the struct is sufficiently lightweight, then it's reasonable to create it when requested. But either way, please use an explicit method rather than a conversion operator.

Copy link
Member Author

Choose a reason for hiding this comment

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

I left it as is for now. If you'd prefer to have it created on demand, let me know and I'll change it.


public override CommonConversion Conversion => ConversionInternal.ToCommonConversion();
}

internal sealed partial class CSharpConversionExpression : BaseCSharpConversionExpression
{
public CSharpConversionExpression(IOperation operand, Conversion conversion, bool isExplicit, bool isTryCast, bool isChecked, SemanticModel semanticModel, SyntaxNode syntax, ITypeSymbol type, Optional<object> constantValue) :
base(conversion, isExplicit, isTryCast, isChecked, semanticModel, syntax, type, constantValue)
{
OperandImpl = operand;
}

public override IOperation OperandImpl { get; }
}

internal sealed partial class LazyCSharpConversionExpression : BaseCSharpConversionExpression
{
private readonly Lazy<IOperation> _operand;
public LazyCSharpConversionExpression(Lazy<IOperation> operand, Conversion conversion, bool isExplicit, bool isTryCast, bool isChecked, SemanticModel semanticModel,SyntaxNode syntax, ITypeSymbol type, Optional<object> constantValue) :
base(conversion, isExplicit, isTryCast, isChecked, semanticModel, syntax, type, constantValue)
{
_operand = operand;
}

public override IOperation OperandImpl => _operand.Value;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
// Copyright (c) Microsoft. All Rights Reserved. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using Microsoft.CodeAnalysis.Semantics;

namespace Microsoft.CodeAnalysis.CSharp
{
internal class CSharpOperationCloner : OperationCloner
{
public static OperationCloner Instance { get; } = new CSharpOperationCloner();

public override IOperation VisitConversionExpression(IConversionExpression operation, object argument)
{
return new CSharpConversionExpression(Visit(operation.Operand), operation.GetConversion(), operation.IsExplicitInCode, operation.IsTryCast, operation.IsChecked, ((Operation)operation).SemanticModel, operation.Syntax, operation.Type, operation.ConstantValue);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -640,28 +640,29 @@ private IOperation CreateBoundConversionOperation(BoundConversion boundConversio
else
{
Lazy<IOperation> operand = new Lazy<IOperation>(() => Create(boundConversion.Operand));

bool isExplicit = boundConversion.ExplicitCastInCode;
bool usesOperatorMethod = boundConversion.ConversionKind == CSharp.ConversionKind.ExplicitUserDefined || boundConversion.ConversionKind == CSharp.ConversionKind.ImplicitUserDefined;
IMethodSymbol operatorMethod = boundConversion.SymbolOpt;
SyntaxNode syntax = boundConversion.Syntax;
Conversion conversion = _semanticModel.GetConversion(syntax);
bool isExplicit = boundConversion.ExplicitCastInCode;
bool isTryCast = false;
// Checked conversions only matter if the conversion is a Numeric conversion. Don't have true unless the conversion is actually numeric.
bool isChecked = conversion.IsNumeric && boundConversion.Checked;
ITypeSymbol type = boundConversion.Type;
Optional<object> constantValue = ConvertToOptional(boundConversion.ConstantValue);
return new LazyConversionExpression(operand, conversionKind, isExplicit, usesOperatorMethod, operatorMethod, _semanticModel, syntax, type, constantValue);
return new LazyCSharpConversionExpression(operand, conversion, isExplicit, isTryCast, isChecked, _semanticModel, syntax, type, constantValue);
}
}

private IConversionExpression CreateBoundAsOperatorOperation(BoundAsOperator boundAsOperator)
{
Lazy<IOperation> operand = new Lazy<IOperation>(() => Create(boundAsOperator.Operand));
ConversionKind conversionKind = Semantics.ConversionKind.TryCast;
bool isExplicit = true;
bool usesOperatorMethod = false;
IMethodSymbol operatorMethod = null;
SyntaxNode syntax = boundAsOperator.Syntax;
Conversion conversion = _semanticModel.GetConversion(syntax);
bool isExplicit = true;
bool isTryCast = true;
bool isChecked = false;
ITypeSymbol type = boundAsOperator.Type;
Optional<object> constantValue = ConvertToOptional(boundAsOperator.ConstantValue);
return new LazyConversionExpression(operand, conversionKind, isExplicit, usesOperatorMethod, operatorMethod, _semanticModel, syntax, type, constantValue);
return new LazyCSharpConversionExpression(operand, conversion, isExplicit, isTryCast, isChecked, _semanticModel, syntax, type, constantValue);
}

private IIsTypeExpression CreateBoundIsOperatorOperation(BoundIsOperator boundIsOperator)
Expand Down
2 changes: 2 additions & 0 deletions src/Compilers/CSharp/Portable/PublicAPI.Unshipped.txt
Original file line number Diff line number Diff line change
@@ -1 +1,3 @@
Microsoft.CodeAnalysis.CSharp.Conversion.ToCommonConversion() -> Microsoft.CodeAnalysis.Semantics.CommonConversion
Microsoft.CodeAnalysis.CSharp.LanguageVersion.CSharp7_2 = 702 -> Microsoft.CodeAnalysis.CSharp.LanguageVersion
static Microsoft.CodeAnalysis.CSharp.CSharpExtensions.GetConversion(this Microsoft.CodeAnalysis.Semantics.IConversionExpression conversionExpression) -> Microsoft.CodeAnalysis.CSharp.Conversion
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,8 @@ static void Test2(int y, params int[] x)
Instance Receiver: null
Arguments(1):
IArgument (ArgumentKind.Explicit, Matching Parameter: x) (OperationKind.Argument) (Syntax: 'null')
IConversionExpression (ConversionKind.Cast, Implicit) (OperationKind.ConversionExpression, Type: System.Int32[], Constant: null) (Syntax: 'null')
IConversionExpression (Implicit, TryCast: False, Unchecked) (OperationKind.ConversionExpression, Type: System.Int32[], Constant: null) (Syntax: 'null')
Conversion: CommonConversion (Exists: True, IsIdentity: False, IsNumeric: False, IsReference: True, IsUserDefined: False) (MethodSymbol: null)
Operand: ILiteralExpression (Text: null) (OperationKind.LiteralExpression, Type: null, Constant: null) (Syntax: 'null')
InConversion: null
OutConversion: null");
Expand Down
Loading