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

Standardize list pattern lowering on Index constructor. #58055

Merged
merged 4 commits into from
Dec 4, 2021
Merged

Standardize list pattern lowering on Index constructor. #58055

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
38fe736
Standardize list pattern lowering on Index constructor.
AlekseyTs Dec 1, 2021
aa7d253
Fixup
AlekseyTs Dec 1, 2021
1b81fa3
Fixup
AlekseyTs Dec 1, 2021
aa095b0
Add a test scenario with initializer.
AlekseyTs Dec 2, 2021
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
6 changes: 1 addition & 5 deletions src/Compilers/CSharp/Portable/Binder/Binder_Patterns.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -374,11 +374,7 @@ private bool BindLengthAndIndexerForListPattern(SyntaxNode node, TypeSymbol inpu

if (!systemIndexType.HasUseSiteError)
{
// Check required well-known member. They may not be needed
// during lowering, but it's simpler to always require them to prevent
// the user from getting surprising errors when optimizations fail
_ = GetWellKnownTypeMember(WellKnownMember.System_Index__op_Implicit_FromInt32, diagnostics, syntax: node);

// Check required well-known member.
_ = GetWellKnownTypeMember(WellKnownMember.System_Index__ctor, diagnostics, syntax: node);
}

Expand Down
5 changes: 3 additions & 2 deletions src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter.PatternLocalRewriter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -303,14 +303,15 @@ void addArg(RefKind refKind, BoundExpression expression)
BoundExpression makeUnloweredIndexArgument(int index)
{
// LocalRewriter.MakePatternIndexOffsetExpression understands this format
var ctor = (MethodSymbol)_factory.WellKnownMember(WellKnownMember.System_Index__ctor);

if (index < 0)
{
var ctor = (MethodSymbol)_factory.WellKnownMember(WellKnownMember.System_Index__ctor);
return new BoundFromEndIndexExpression(_factory.Syntax, _factory.Literal(-index),
methodOpt: ctor, _factory.WellKnownType(WellKnownType.System_Index));
}

return _factory.Convert(_factory.WellKnownType(WellKnownType.System_Index), _factory.Literal(index));
return _factory.New(ctor, _factory.Literal(index), _factory.Literal(false));
}

BoundExpression makeUnloweredRangeArgument(BoundDagSliceEvaluation e)
Expand Down
18 changes: 18 additions & 0 deletions src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_IndexerAccess.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -432,6 +432,24 @@ private BoundExpression DetermineMakePatternIndexOffsetExpressionStrategy(
strategy = PatternIndexOffsetLoweringStrategy.UseAsIs;
return VisitExpression(operand);
}
else if (unloweredExpr is BoundObjectCreationExpression { Constructor: MethodSymbol constructor, Arguments: { Length: 2 } arguments, ArgsToParamsOpt: { IsDefaultOrEmpty: true }, InitializerExpressionOpt: null } &&
Copy link
Member

@jcouv jcouv Dec 1, 2021 •
edited by AlekseyTs
Loading

Choose a reason for hiding this comment

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

InitializerExpressionOpt: null

nit: do we have a indexing test with an Index initialization with an initializer? #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.

do we have a indexing test with an Index initialization with an initializer?

No, the type doesn't have any member that can be initialized, but I decided to have the check for completeness.

(object)constructor == _compilation.GetWellKnownTypeMember(WellKnownMember.System_Index__ctor) &&
arguments[0] is { Type.SpecialType: SpecialType.System_Int32, ConstantValue.Value: int _ and >= 0 } index &&
Copy link
Member

Choose a reason for hiding this comment

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

_

nit: could remove the discard designation

arguments[1] is { Type.SpecialType: SpecialType.System_Boolean, ConstantValue.Value: bool fromEnd })
{
if (fromEnd)
{
// We can replace the `argument.GetOffset(length)` call with `length - index`
strategy = PatternIndexOffsetLoweringStrategy.SubtractFromLength;
}
else
{
// We can return the int directly
strategy = PatternIndexOffsetLoweringStrategy.UseAsIs;
}

return VisitExpression(index);
}
else
{
// `argument.GetOffset(length)`
Expand Down
185 changes: 131 additions & 54 deletions src/Compilers/CSharp/Test/Emit/CodeGen/IndexAndRangeTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1632,44 +1632,117 @@ class C
public static void Main()
{
var s = ""abcdef"";
Console.WriteLine(s[new Index(1, false)]);
int i = 1;
Console.WriteLine(s[new Index(i, false)]);
Console.WriteLine(s[(Index)2]);
Console.WriteLine(s[^1]);
Console.WriteLine(s[new Index(3, false)]);
Console.WriteLine(s[new Index(2, true)]);
i = 6;
Console.WriteLine(s[new Index(i, true)]);
Console.WriteLine(s[new Index(value: 2, fromEnd: false)]);
Console.WriteLine(s[new Index(fromEnd: true, value: 3)]);
Console.WriteLine(s[new Index(3, false) {}]);
}
}", expectedOutput: @"b
c
f");
f
d
e
a
c
d
d");
verifier.VerifyIL("C.Main", @"
{
// Code size 70 (0x46)
// Code size 219 (0xdb)
.maxstack 4
.locals init (string V_0,
System.Index V_1)
.locals init (int V_0, //i
string V_1,
System.Index V_2)
IL_0000: ldstr ""abcdef""
IL_0005: dup
IL_0005: ldc.i4.1
IL_0006: stloc.0
IL_0007: ldloc.0
IL_0008: ldc.i4.1
IL_0009: ldc.i4.0
IL_000a: newobj ""System.Index..ctor(int, bool)""
IL_000f: stloc.1
IL_0010: ldloca.s V_1
IL_0012: ldloc.0
IL_0013: callvirt ""int string.Length.get""
IL_0018: call ""int System.Index.GetOffset(int)""
IL_001d: callvirt ""char string.this[int].get""
IL_0022: call ""void System.Console.WriteLine(char)""
IL_0027: dup
IL_0028: ldc.i4.2
IL_0029: callvirt ""char string.this[int].get""
IL_002e: call ""void System.Console.WriteLine(char)""
IL_0033: dup
IL_0034: callvirt ""int string.Length.get""
IL_0039: ldc.i4.1
IL_003a: sub
IL_003b: callvirt ""char string.this[int].get""
IL_0040: call ""void System.Console.WriteLine(char)""
IL_0045: ret
IL_0007: dup
IL_0008: stloc.1
IL_0009: ldloc.1
IL_000a: ldloc.0
IL_000b: ldc.i4.0
IL_000c: newobj ""System.Index..ctor(int, bool)""
IL_0011: stloc.2
IL_0012: ldloca.s V_2
IL_0014: ldloc.1
IL_0015: callvirt ""int string.Length.get""
IL_001a: call ""int System.Index.GetOffset(int)""
IL_001f: callvirt ""char string.this[int].get""
IL_0024: call ""void System.Console.WriteLine(char)""
IL_0029: dup
IL_002a: ldc.i4.2
IL_002b: callvirt ""char string.this[int].get""
IL_0030: call ""void System.Console.WriteLine(char)""
IL_0035: dup
IL_0036: dup
IL_0037: callvirt ""int string.Length.get""
IL_003c: ldc.i4.1
IL_003d: sub
IL_003e: callvirt ""char string.this[int].get""
IL_0043: call ""void System.Console.WriteLine(char)""
IL_0048: dup
IL_0049: ldc.i4.3
IL_004a: callvirt ""char string.this[int].get""
IL_004f: call ""void System.Console.WriteLine(char)""
IL_0054: dup
IL_0055: dup
IL_0056: callvirt ""int string.Length.get""
IL_005b: ldc.i4.2
IL_005c: sub
IL_005d: callvirt ""char string.this[int].get""
IL_0062: call ""void System.Console.WriteLine(char)""
IL_0067: ldc.i4.6
IL_0068: stloc.0
IL_0069: dup
IL_006a: stloc.1
IL_006b: ldloc.1
IL_006c: ldloc.0
IL_006d: ldc.i4.1
IL_006e: newobj ""System.Index..ctor(int, bool)""
IL_0073: stloc.2
IL_0074: ldloca.s V_2
IL_0076: ldloc.1
IL_0077: callvirt ""int string.Length.get""
IL_007c: call ""int System.Index.GetOffset(int)""
IL_0081: callvirt ""char string.this[int].get""
IL_0086: call ""void System.Console.WriteLine(char)""
IL_008b: dup
IL_008c: ldc.i4.2
IL_008d: callvirt ""char string.this[int].get""
IL_0092: call ""void System.Console.WriteLine(char)""
IL_0097: dup
IL_0098: stloc.1
IL_0099: ldloc.1
IL_009a: ldc.i4.3
IL_009b: ldc.i4.1
IL_009c: newobj ""System.Index..ctor(int, bool)""
IL_00a1: stloc.2
IL_00a2: ldloca.s V_2
IL_00a4: ldloc.1
IL_00a5: callvirt ""int string.Length.get""
IL_00aa: call ""int System.Index.GetOffset(int)""
IL_00af: callvirt ""char string.this[int].get""
IL_00b4: call ""void System.Console.WriteLine(char)""
IL_00b9: stloc.1
IL_00ba: ldloc.1
IL_00bb: ldc.i4.3
IL_00bc: ldc.i4.0
IL_00bd: newobj ""System.Index..ctor(int, bool)""
IL_00c2: stloc.2
IL_00c3: ldloca.s V_2
IL_00c5: ldloc.1
IL_00c6: callvirt ""int string.Length.get""
IL_00cb: call ""int System.Index.GetOffset(int)""
IL_00d0: callvirt ""char string.this[int].get""
IL_00d5: call ""void System.Console.WriteLine(char)""
IL_00da: ret
}
");
}
Expand Down Expand Up @@ -1819,41 +1892,45 @@ public static void Main()

public static void M(int[] array)
{
Console.WriteLine(array[new Index(1, false)]);
bool fromEnd = false;
Console.WriteLine(array[new Index(1, fromEnd)]);
Console.WriteLine(array[^1]);
}
}", TestOptions.ReleaseExe);
var verifier = CompileAndVerify(comp, expectedOutput: @"2
11");
verifier.VerifyIL("C.M", @"
{
// Code size 40 (0x28)
// Code size 42 (0x2a)
.maxstack 3
.locals init (int[] V_0,
System.Index V_1)
IL_0000: ldarg.0
.locals init (bool V_0, //fromEnd
int[] V_1,
System.Index V_2)
IL_0000: ldc.i4.0
IL_0001: stloc.0
IL_0002: ldloc.0
IL_0003: ldc.i4.1
IL_0004: ldc.i4.0
IL_0005: newobj ""System.Index..ctor(int, bool)""
IL_000a: stloc.1
IL_000b: ldloca.s V_1
IL_000d: ldloc.0
IL_000e: ldlen
IL_000f: conv.i4
IL_0010: call ""int System.Index.GetOffset(int)""
IL_0015: ldelem.i4
IL_0016: call ""void System.Console.WriteLine(int)""
IL_001b: ldarg.0
IL_001c: dup
IL_001d: ldlen
IL_001e: conv.i4
IL_001f: ldc.i4.1
IL_0020: sub
IL_0021: ldelem.i4
IL_0022: call ""void System.Console.WriteLine(int)""
IL_0027: ret
IL_0002: ldarg.0
IL_0003: stloc.1
IL_0004: ldloc.1
IL_0005: ldc.i4.1
IL_0006: ldloc.0
IL_0007: newobj ""System.Index..ctor(int, bool)""
IL_000c: stloc.2
IL_000d: ldloca.s V_2
IL_000f: ldloc.1
IL_0010: ldlen
IL_0011: conv.i4
IL_0012: call ""int System.Index.GetOffset(int)""
IL_0017: ldelem.i4
IL_0018: call ""void System.Console.WriteLine(int)""
IL_001d: ldarg.0
IL_001e: dup
IL_001f: ldlen
IL_0020: conv.i4
IL_0021: ldc.i4.1
IL_0022: sub
IL_0023: ldelem.i4
IL_0024: call ""void System.Console.WriteLine(int)""
IL_0029: ret
}");
}

Expand Down
Loading