Skip to content

Use 'checked' names for synthesized vb intrinsic operators #63604

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

Merged
CyrusNajmabadi merged 19 commits into from
Aug 29, 2022
Merged

Use 'checked' names for synthesized vb intrinsic operators #63604

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
4f0d321
Use 'checked' names for synthesized vb operators
CyrusNajmabadi Aug 25, 2022
0549a16
Exercise test paths
CyrusNajmabadi Aug 25, 2022
8a7b3c9
Fix
CyrusNajmabadi Aug 25, 2022
b918f3b
Fix
CyrusNajmabadi Aug 25, 2022
06d3616
Is fixed
CyrusNajmabadi Aug 25, 2022
c8c9ce6
Fix
CyrusNajmabadi Aug 25, 2022
89a596d
formatting
CyrusNajmabadi Aug 25, 2022
0e2f25a
Update src/Compilers/Test/Core/Compilation/TestOperationVisitor.cs
CyrusNajmabadi Aug 25, 2022
1b6fff3
Update src/Compilers/Test/Core/Compilation/TestOperationVisitor.cs
CyrusNajmabadi Aug 25, 2022
07f412b
Formatting
CyrusNajmabadi Aug 25, 2022
219696f
Merge branch 'vbCheckedNames' of https://github.com/CyrusNajmabadi/ro…
CyrusNajmabadi Aug 25, 2022
d355c30
Update src/Compilers/VisualBasic/Test/Semantic/Semantics/UnaryOperato…
CyrusNajmabadi Aug 25, 2022
fc0e85f
REvert c# file
CyrusNajmabadi Aug 25, 2022
db1a8c4
Merge branch 'vbCheckedNames' of https://github.com/CyrusNajmabadi/ro…
CyrusNajmabadi Aug 25, 2022
a596329
Determine the checked state from the name, not an additional bool
CyrusNajmabadi Aug 26, 2022
3266423
Fix test
CyrusNajmabadi Aug 26, 2022
7823890
Update tests
CyrusNajmabadi Aug 26, 2022
18d577a
Move to a computed app
CyrusNajmabadi Aug 26, 2022
648f802
Revert
CyrusNajmabadi Aug 26, 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
12 changes: 4 additions & 8 deletions src/Compilers/CSharp/Portable/Compilation/CSharpSemanticModel.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3783,8 +3783,7 @@ private static void GetSymbolsAndResultKind(BoundUnaryOperator unaryOperator, ou
UnaryOperatorKind op = unaryOperator.OperatorKind.Operator();
symbols = ImmutableArray.Create<Symbol>(new SynthesizedIntrinsicOperatorSymbol(unaryOperator.Operand.Type.StrippedType(),
OperatorFacts.UnaryOperatorNameFromOperatorKind(op, isChecked: unaryOperator.OperatorKind.IsChecked()),
unaryOperator.Type.StrippedType(),
unaryOperator.OperatorKind.IsChecked()));
unaryOperator.Type.StrippedType()));
resultKind = unaryOperator.ResultKind;
}
}
Expand All @@ -3807,8 +3806,7 @@ private static void GetSymbolsAndResultKind(BoundIncrementOperator increment, ou
UnaryOperatorKind op = increment.OperatorKind.Operator();
symbols = ImmutableArray.Create<Symbol>(new SynthesizedIntrinsicOperatorSymbol(increment.Operand.Type.StrippedType(),
OperatorFacts.UnaryOperatorNameFromOperatorKind(op, isChecked: increment.OperatorKind.IsChecked()),
increment.Type.StrippedType(),
increment.OperatorKind.IsChecked()));
increment.Type.StrippedType()));
resultKind = increment.ResultKind;
}
}
Expand Down Expand Up @@ -3842,8 +3840,7 @@ private static void GetSymbolsAndResultKind(BoundBinaryOperator binaryOperator,
symbols = ImmutableArray.Create<Symbol>(new SynthesizedIntrinsicOperatorSymbol(objectType,
OperatorFacts.BinaryOperatorNameFromOperatorKind(op, isChecked: binaryOperator.OperatorKind.IsChecked()),
objectType,
binaryOperator.Type,
binaryOperator.OperatorKind.IsChecked()));
binaryOperator.Type));
}
else
{
Expand Down Expand Up @@ -3884,8 +3881,7 @@ private static Symbol GetIntrinsicOperatorSymbol(BinaryOperatorKind op, bool isD
return new SynthesizedIntrinsicOperatorSymbol(leftType,
OperatorFacts.BinaryOperatorNameFromOperatorKind(op, isChecked),
rightType,
returnType,
isChecked);
returnType);
}

private static void GetSymbolsAndResultKind(BoundCompoundAssignmentOperator compoundAssignment, out bool isDynamic, ref LookupResultKind resultKind, ref ImmutableArray<Symbol> symbols)
Expand Down
20 changes: 5 additions & 15 deletions src/Compilers/CSharp/Portable/Symbols/Synthesized/SynthesizedIntrinsicOperatorSymbol.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,8 @@ internal sealed class SynthesizedIntrinsicOperatorSymbol : MethodSymbol
private readonly string _name;
private readonly ImmutableArray<ParameterSymbol> _parameters;
private readonly TypeSymbol _returnType;
private readonly bool _isCheckedBuiltin;

public SynthesizedIntrinsicOperatorSymbol(TypeSymbol leftType, string name, TypeSymbol rightType, TypeSymbol returnType, bool isCheckedBuiltin)
public SynthesizedIntrinsicOperatorSymbol(TypeSymbol leftType, string name, TypeSymbol rightType, TypeSymbol returnType)
{
if (leftType.Equals(rightType, TypeCompareKind.IgnoreCustomModifiersAndArraySizesAndLowerBounds | TypeCompareKind.IgnoreNullableModifiersForReferenceTypes))
{
Expand All @@ -44,18 +43,18 @@ public SynthesizedIntrinsicOperatorSymbol(TypeSymbol leftType, string name, Type

_parameters = ImmutableArray.Create<ParameterSymbol>(new SynthesizedOperatorParameterSymbol(this, leftType, 0, "left"),
new SynthesizedOperatorParameterSymbol(this, rightType, 1, "right"));
_isCheckedBuiltin = isCheckedBuiltin;
}

public SynthesizedIntrinsicOperatorSymbol(TypeSymbol container, string name, TypeSymbol returnType, bool isCheckedBuiltin)
public SynthesizedIntrinsicOperatorSymbol(TypeSymbol container, string name, TypeSymbol returnType)
{
_containingType = container;
_name = name;
_returnType = returnType;
_parameters = ImmutableArray.Create<ParameterSymbol>(new SynthesizedOperatorParameterSymbol(this, container, 0, "value"));
_isCheckedBuiltin = isCheckedBuiltin;
}

public override bool IsCheckedBuiltin => SyntaxFacts.IsCheckedOperator(this.Name);

public override string Name
{
get
Expand All @@ -64,14 +63,6 @@ public override string Name
}
}

public override bool IsCheckedBuiltin
{
get
{
return _isCheckedBuiltin;
}
}

public override MethodKind MethodKind
{
get
Expand Down Expand Up @@ -441,8 +432,7 @@ public override bool Equals(Symbol obj, TypeCompareKind compareKind)
return false;
}

if (_isCheckedBuiltin == other._isCheckedBuiltin &&
_parameters.Length == other._parameters.Length &&
if (_parameters.Length == other._parameters.Length &&
string.Equals(_name, other._name, StringComparison.Ordinal) &&
TypeSymbol.Equals(_containingType, other._containingType, compareKind) &&
TypeSymbol.Equals(_returnType, other._returnType, compareKind))
Expand Down
60 changes: 51 additions & 9 deletions src/Compilers/CSharp/Test/Semantic/Semantics/OperatorTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7636,10 +7636,6 @@ ExpressionSyntax node4
symbol1.ContainingType.EnumUnderlyingTypeOrSelf().SpecialType.IsIntegralType() ||
symbol1.ContainingType.SpecialType == SpecialType.System_Char);
break;

default:
expectChecked = type.IsDynamic();
break;
Copy link
Contributor Author

@CyrusNajmabadi CyrusNajmabadi Aug 26, 2022

Choose a reason for hiding this comment

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

now, no other op-types ever expect to be checked.

}

Assert.Equal(expectChecked, symbol1.IsCheckedBuiltin);
Expand Down Expand Up @@ -8396,7 +8392,7 @@ ExpressionSyntax node8
Assert.Equal(MethodKind.BuiltinOperator, symbol1.MethodKind);
Assert.True(symbol1.IsImplicitlyDeclared);

bool isChecked;
bool isChecked = false;

switch (op)
{
Expand All @@ -8406,10 +8402,6 @@ ExpressionSyntax node8
case BinaryOperatorKind.Division:
isChecked = isDynamic || symbol1.ContainingSymbol.Kind == SymbolKind.PointerType || symbol1.ContainingType.EnumUnderlyingTypeOrSelf().SpecialType.IsIntegralType();
break;

default:
isChecked = isDynamic;
break;
Copy link
Contributor Author

@CyrusNajmabadi CyrusNajmabadi Aug 26, 2022

Choose a reason for hiding this comment

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

now, no other op-types ever expect to be checked.

}

string expectedSymbol = String.Format("{4} {0}.{2}({1} left, {3} right)",
Expand Down Expand Up @@ -8650,6 +8642,56 @@ void Test(dynamic x)
compilation = compilation.WithOptions(TestOptions.ReleaseDll.WithOverflowChecks(true));
semanticModel = compilation.GetSemanticModel(tree);

var symbols2 = (from node2 in nodes select (IMethodSymbol)semanticModel.GetSymbolInfo(node2).Symbol).ToArray();
foreach (var symbol2 in symbols2)
{
Assert.False(symbol2.IsCheckedBuiltin);
Copy link
Contributor Author

@CyrusNajmabadi CyrusNajmabadi Aug 26, 2022

Choose a reason for hiding this comment

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

these equality operator methods are not checked anymore.

Assert.True(((ITypeSymbol)symbol2.ContainingSymbol).IsDynamic());
Assert.Null(symbol2.ContainingType);
}

for (int i = 0; i < symbols1.Length; i++)
{
Assert.Equal(symbols1[i], symbols2[i]);
Copy link
Contributor Author

@CyrusNajmabadi CyrusNajmabadi Aug 26, 2022

Choose a reason for hiding this comment

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

because they're not checked, they are equal to the non-checked versions always.

}
}

[Fact]
public void DynamicBinaryIntrinsicSymbols2()
{
var source =
@"
class Module1
{
void Test(dynamic x)
{
var z1 = x + 0;
var z2 = 0 + x;
Copy link
Contributor Author

@CyrusNajmabadi CyrusNajmabadi Aug 26, 2022

Choose a reason for hiding this comment

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

i added this new test with dynamic and operators to show where 'checked' might actually appear.

}
}";

var compilation = CreateCompilation(source, options: TestOptions.ReleaseDll.WithOverflowChecks(false));

var tree = compilation.SyntaxTrees.Single();
var semanticModel = compilation.GetSemanticModel(tree);

var nodes = (from node in tree.GetRoot().DescendantNodes()
select node as BinaryExpressionSyntax).
Where(node => node is not null).ToArray();

Assert.Equal(2, nodes.Length);

var symbols1 = (from node1 in nodes select (IMethodSymbol)semanticModel.GetSymbolInfo(node1).Symbol).ToArray();
foreach (var symbol1 in symbols1)
{
Assert.False(symbol1.IsCheckedBuiltin);
Assert.True(((ITypeSymbol)symbol1.ContainingSymbol).IsDynamic());
Assert.Null(symbol1.ContainingType);
}

compilation = compilation.WithOptions(TestOptions.ReleaseDll.WithOverflowChecks(true));
semanticModel = compilation.GetSemanticModel(tree);

var symbols2 = (from node2 in nodes select (IMethodSymbol)semanticModel.GetSymbolInfo(node2).Symbol).ToArray();
foreach (var symbol2 in symbols2)
{
Expand Down
30 changes: 17 additions & 13 deletions src/Compilers/VisualBasic/Portable/BoundTree/BoundBinaryOperator.vb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,20 +39,24 @@ Namespace Microsoft.CodeAnalysis.VisualBasic
Public Overrides ReadOnly Property ExpressionSymbol As Symbol
Get
If (OperatorKind And BinaryOperatorKind.Error) = 0 Then
Dim opName As String = OverloadResolution.TryGetOperatorName(OperatorKind)
Dim op As BinaryOperatorKind = (OperatorKind And BinaryOperatorKind.OpMask)
Dim leftType = TryCast(Left.Type.GetNullableUnderlyingTypeOrSelf(), NamedTypeSymbol)

If opName IsNot Nothing Then
Dim op As BinaryOperatorKind = (OperatorKind And BinaryOperatorKind.OpMask)
Dim leftType = DirectCast(Left.Type.GetNullableUnderlyingTypeOrSelf(), NamedTypeSymbol)
Return New SynthesizedIntrinsicOperatorSymbol(leftType,
opName,
Right.Type.GetNullableUnderlyingTypeOrSelf(),
Type.GetNullableUnderlyingTypeOrSelf(),
Checked AndAlso leftType.IsIntegralType() AndAlso
(op = BinaryOperatorKind.Multiply OrElse
op = BinaryOperatorKind.Add OrElse
op = BinaryOperatorKind.Subtract OrElse
op = BinaryOperatorKind.IntegerDivide))
If leftType IsNot Nothing Then
Dim isChecked = Checked AndAlso leftType.IsIntegralType() AndAlso
(op = BinaryOperatorKind.Multiply OrElse
op = BinaryOperatorKind.Add OrElse
op = BinaryOperatorKind.Subtract OrElse
op = BinaryOperatorKind.IntegerDivide)
Dim opName As String = OverloadResolution.TryGetOperatorName(OperatorKind, isChecked)

If opName IsNot Nothing Then
Return New SynthesizedIntrinsicOperatorSymbol(
leftType,
opName,
Right.Type.GetNullableUnderlyingTypeOrSelf(),
Type.GetNullableUnderlyingTypeOrSelf())
End If
End If
End If

Expand Down
23 changes: 13 additions & 10 deletions src/Compilers/VisualBasic/Portable/BoundTree/BoundUnaryOperator.vb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,16 +32,19 @@ Namespace Microsoft.CodeAnalysis.VisualBasic
Public Overrides ReadOnly Property ExpressionSymbol As Symbol
Get
If (OperatorKind And UnaryOperatorKind.Error) = 0 Then
Dim opName As String = OverloadResolution.TryGetOperatorName(OperatorKind)

If opName IsNot Nothing Then
Dim op As UnaryOperatorKind = (OperatorKind And UnaryOperatorKind.OpMask)
Dim operandType = DirectCast(Operand.Type.GetNullableUnderlyingTypeOrSelf(), NamedTypeSymbol)
Return New SynthesizedIntrinsicOperatorSymbol(operandType,
opName,
Type.GetNullableUnderlyingTypeOrSelf(),
Checked AndAlso operandType.IsIntegralType() AndAlso
op = UnaryOperatorKind.Minus)
Dim op As UnaryOperatorKind = (OperatorKind And UnaryOperatorKind.OpMask)
Dim operandType = TryCast(Operand.Type.GetNullableUnderlyingTypeOrSelf(), NamedTypeSymbol)

If operandType IsNot Nothing Then
Dim isChecked = Checked AndAlso operandType.IsIntegralType() AndAlso op = UnaryOperatorKind.Minus
Dim opName As String = OverloadResolution.TryGetOperatorName(OperatorKind, isChecked)

If opName IsNot Nothing Then
Return New SynthesizedIntrinsicOperatorSymbol(
operandType,
opName,
Type.GetNullableUnderlyingTypeOrSelf())
End If
End If
End If

Expand Down
14 changes: 7 additions & 7 deletions src/Compilers/VisualBasic/Portable/Semantics/Operators.vb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -214,11 +214,11 @@ Namespace Microsoft.CodeAnalysis.VisualBasic
End Select
End Function

Friend Shared Function TryGetOperatorName(op As BinaryOperatorKind) As String
Friend Shared Function TryGetOperatorName(op As BinaryOperatorKind, isChecked As Boolean) As String

Select Case (op And BinaryOperatorKind.OpMask)
Case BinaryOperatorKind.Add
Return WellKnownMemberNames.AdditionOperatorName
Return If(isChecked, WellKnownMemberNames.CheckedAdditionOperatorName, WellKnownMemberNames.AdditionOperatorName)
Case BinaryOperatorKind.Concatenate
Return WellKnownMemberNames.ConcatenateOperatorName
Case BinaryOperatorKind.Like
Expand All @@ -236,17 +236,17 @@ Namespace Microsoft.CodeAnalysis.VisualBasic
Case BinaryOperatorKind.GreaterThan
Return WellKnownMemberNames.GreaterThanOperatorName
Case BinaryOperatorKind.Subtract
Return WellKnownMemberNames.SubtractionOperatorName
Return If(isChecked, WellKnownMemberNames.CheckedSubtractionOperatorName, WellKnownMemberNames.SubtractionOperatorName)
Case BinaryOperatorKind.Multiply
Return WellKnownMemberNames.MultiplyOperatorName
Return If(isChecked, WellKnownMemberNames.CheckedMultiplyOperatorName, WellKnownMemberNames.MultiplyOperatorName)
Case BinaryOperatorKind.Power
Return WellKnownMemberNames.ExponentOperatorName
Case BinaryOperatorKind.Divide
Return WellKnownMemberNames.DivisionOperatorName
Case BinaryOperatorKind.Modulo
Return WellKnownMemberNames.ModulusOperatorName
Case BinaryOperatorKind.IntegerDivide
Return WellKnownMemberNames.IntegerDivisionOperatorName
Return If(isChecked, WellKnownMemberNames.CheckedDivisionOperatorName, WellKnownMemberNames.IntegerDivisionOperatorName)
Case BinaryOperatorKind.LeftShift
Return WellKnownMemberNames.LeftShiftOperatorName
Case BinaryOperatorKind.RightShift
Expand All @@ -267,13 +267,13 @@ Namespace Microsoft.CodeAnalysis.VisualBasic
End Select
End Function

Friend Shared Function TryGetOperatorName(op As UnaryOperatorKind) As String
Friend Shared Function TryGetOperatorName(op As UnaryOperatorKind, isChecked As Boolean) As String

Select Case (op And UnaryOperatorKind.OpMask)
Case UnaryOperatorKind.Plus
Return WellKnownMemberNames.UnaryPlusOperatorName
Case UnaryOperatorKind.Minus
Return WellKnownMemberNames.UnaryNegationOperatorName
Return If(isChecked, WellKnownMemberNames.CheckedUnaryNegationOperatorName, WellKnownMemberNames.UnaryNegationOperatorName)
Case UnaryOperatorKind.Not
Return WellKnownMemberNames.OnesComplementOperatorName
Case UnaryOperatorKind.Implicit
Expand Down
Loading