Skip to content

Commit e4ff20f

Browse files
authored
Bugfix SmartParameter source code generation (#2041)
* Bugfix SmartParameter source code generation SmartParameter codegen must use the target type for casting, not the type of the source value. Basically, this is a follow up to PR #1228 (1678a49), which fixed this problem for SmartArgument. Fixes #2011. * Tests added. * Applying code review suggestions; code clean-up. PR fixes the following issues: fixes #2011, fixes #1812, fixes #1177.
1 parent 96b3764 commit e4ff20f

File tree

4 files changed

+181
-9
lines changed

4 files changed

+181
-9
lines changed

src/BenchmarkDotNet/Code/CodeGenerator.cs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -189,7 +189,8 @@ private static DeclarationsProvider GetDeclarationsProvider(Descriptor descripto
189189
return new NonVoidDeclarationsProvider(descriptor);
190190
}
191191

192-
private static string GetParamsContent(BenchmarkCase benchmarkCase)
192+
// internal for tests
193+
internal static string GetParamsContent(BenchmarkCase benchmarkCase)
193194
=> string.Join(
194195
string.Empty,
195196
benchmarkCase.Parameters.Items

src/BenchmarkDotNet/Parameters/SmartParamBuilder.cs

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ namespace BenchmarkDotNet.Parameters
1414
internal static class SmartParamBuilder
1515
{
1616
[SuppressMessage("ReSharper", "CoVariantArrayConversion")]
17-
internal static object[] CreateForParams(MemberInfo source, object[] values)
17+
internal static object[] CreateForParams(Type parameterType, MemberInfo source, object[] values)
1818
{
1919
// IEnumerable<object>
2020
if (values.IsEmpty() || values.All(SourceCodeHelper.IsCompilationTimeConstant))
@@ -24,7 +24,7 @@ internal static object[] CreateForParams(MemberInfo source, object[] values)
2424
if (values.All(value => value is object[] array && array.Length == 1 && SourceCodeHelper.IsCompilationTimeConstant(array[0])))
2525
return values.Select(x => ((object[])x)[0]).ToArray();
2626

27-
return values.Select((value, index) => new SmartParameter(source, value, index)).ToArray();
27+
return values.Select((value, index) => new SmartParameter(parameterType, source, value, index)).ToArray();
2828
}
2929

3030
internal static ParameterInstances CreateForArguments(MethodInfo benchmark, ParameterDefinition[] parameterDefinitions, (MemberInfo source, object[] values) valuesInfo, int sourceIndex, SummaryStyle summaryStyle)
@@ -106,12 +106,14 @@ public string ToSourceCode()
106106

107107
internal class SmartParameter : IParam
108108
{
109+
private readonly Type parameterType;
109110
private readonly MemberInfo source;
110111
private readonly MethodBase method;
111112
private readonly int index;
112113

113-
public SmartParameter(MemberInfo source, object value, int index)
114+
public SmartParameter(Type parameterType, MemberInfo source, object value, int index)
114115
{
116+
this.parameterType = parameterType;
115117
this.source = source;
116118
method = source is PropertyInfo property ? property.GetMethod : source as MethodInfo;
117119
Value = value;
@@ -124,7 +126,7 @@ public SmartParameter(MemberInfo source, object value, int index)
124126

125127
public string ToSourceCode()
126128
{
127-
string cast = Value == null ? string.Empty : $"({Value.GetType().GetCorrectCSharpTypeName()})";
129+
string cast = $"({parameterType.GetCorrectCSharpTypeName()})"; // it's an object so we need to cast it to the right type
128130

129131
string instancePrefix = method.IsStatic ? source.DeclaringType.GetCorrectCSharpTypeName() : "instance";
130132

src/BenchmarkDotNet/Running/BenchmarkConverter.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -191,10 +191,10 @@ IEnumerable<ParameterDefinition> GetDefinitions<TAttribute>(Func<TAttribute, Typ
191191

192192
var paramsDefinitions = GetDefinitions<ParamsAttribute>((attribute, parameterType) => GetValidValues(attribute.Values, parameterType));
193193

194-
var paramsSourceDefinitions = GetDefinitions<ParamsSourceAttribute>((attribute, _) =>
194+
var paramsSourceDefinitions = GetDefinitions<ParamsSourceAttribute>((attribute, parameterType) =>
195195
{
196196
var paramsValues = GetValidValuesForParamsSource(type, attribute.Name);
197-
return SmartParamBuilder.CreateForParams(paramsValues.source, paramsValues.values);
197+
return SmartParamBuilder.CreateForParams(parameterType, paramsValues.source, paramsValues.values);
198198
});
199199

200200
var paramsAllValuesDefinitions = GetDefinitions<ParamsAllValuesAttribute>((_, parameterType) => GetAllValidValues(parameterType));

tests/BenchmarkDotNet.IntegrationTests/ParamSourceTests.cs

Lines changed: 171 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,30 @@
11
using System;
22
using System.Collections.Generic;
33
using System.Linq;
4-
using System.Text;
5-
using System.Threading.Tasks;
64
using BenchmarkDotNet.Attributes;
5+
using BenchmarkDotNet.Code;
6+
using BenchmarkDotNet.Configs;
7+
using BenchmarkDotNet.Jobs;
8+
using BenchmarkDotNet.Reports;
9+
using BenchmarkDotNet.Running;
10+
using BenchmarkDotNet.Toolchains;
11+
using BenchmarkDotNet.Toolchains.InProcess.Emit;
712
using Xunit;
13+
using Xunit.Abstractions;
814

915
namespace BenchmarkDotNet.IntegrationTests
1016
{
1117
public class ParamSourceTests: BenchmarkTestExecutor
1218
{
19+
public ParamSourceTests(ITestOutputHelper output) : base(output) { }
20+
21+
public static IEnumerable<object[]> GetToolchains()
22+
=> new[]
23+
{
24+
new object[] { Job.Default.GetToolchain() },
25+
new object[] { InProcessEmitToolchain.Instance },
26+
};
27+
1328
[Fact]
1429
public void ParamSourceCanHandleStringWithSurrogates()
1530
{
@@ -35,5 +50,159 @@ public IEnumerable<string> StringValues
3550
[Benchmark]
3651
public void Method() { }
3752
}
53+
54+
private Summary CanExecuteWithExtraInfo(Type type, IToolchain toolchain)
55+
{
56+
IConfig config = CreateSimpleConfig(job: Job.Dry.WithToolchain(toolchain));
57+
if (!toolchain.IsInProcess)
58+
{
59+
// Show the relevant codegen excerpt in test results (the *.notcs is not part of the logs)
60+
Output.WriteLine("// Benchmarks and CodeGenerator.GetParamsContent()");
61+
BenchmarkRunInfo runInfo = BenchmarkConverter.TypeToBenchmarks(type, config);
62+
foreach (BenchmarkCase benchmarkCase in runInfo.BenchmarksCases)
63+
{
64+
Output.WriteLine("// " + benchmarkCase.DisplayInfo);
65+
Output.WriteLine(CodeGenerator.GetParamsContent(benchmarkCase));
66+
}
67+
}
68+
return CanExecute(type, config);
69+
}
70+
71+
public interface ITargetInterface
72+
{
73+
int Data { get; }
74+
}
75+
76+
private class NonPublicSource : ITargetInterface
77+
{
78+
public int Data { get; }
79+
public NonPublicSource(int data) => Data = data;
80+
public override string ToString() => "src " + Data.ToString();
81+
}
82+
83+
public class PrivateClassWithPublicInterface
84+
{
85+
public static IEnumerable<ITargetInterface> GetSource()
86+
{
87+
yield return null;
88+
yield return new NonPublicSource(1);
89+
yield return new NonPublicSource(2);
90+
}
91+
92+
[ParamsSource(nameof(GetSource))]
93+
public ITargetInterface ParamsTarget { get; set; }
94+
95+
[Benchmark]
96+
public int Benchmark() => ParamsTarget?.Data ?? 0;
97+
}
98+
99+
[Theory, MemberData(nameof(GetToolchains))]
100+
public void PrivateClassWithPublicInterface_Succeeds(IToolchain toolchain) => CanExecuteWithExtraInfo(typeof(PrivateClassWithPublicInterface), toolchain);
101+
102+
public class PrivateClassWithPublicInterface_Array
103+
{
104+
public IEnumerable<ITargetInterface[]> GetSource()
105+
{
106+
yield return null;
107+
yield return Array.Empty<NonPublicSource>();
108+
yield return new NonPublicSource[] { null };
109+
yield return new[] { new NonPublicSource(1), new NonPublicSource(2) };
110+
}
111+
112+
[ParamsSource(nameof(GetSource))]
113+
public ITargetInterface[] ParamsTarget { get; set; }
114+
115+
[Benchmark]
116+
public int Benchmark() => ParamsTarget?.Sum(p => p?.Data ?? 0) ?? 0;
117+
}
118+
119+
[Theory, MemberData(nameof(GetToolchains))]
120+
public void PrivateClassWithPublicInterface_Array_Succeeds(IToolchain toolchain) => CanExecuteWithExtraInfo(typeof(PrivateClassWithPublicInterface_Array), toolchain);
121+
122+
public class PrivateClassWithPublicInterface_Enumerable
123+
{
124+
public IEnumerable<IEnumerable<ITargetInterface>> GetSource()
125+
{
126+
static IEnumerable<ITargetInterface> YieldNull() { yield return null; }
127+
yield return null;
128+
yield return Enumerable.Empty<NonPublicSource>();
129+
yield return YieldNull();
130+
yield return PrivateClassWithPublicInterface.GetSource();
131+
}
132+
133+
[ParamsSource(nameof(GetSource))]
134+
public IEnumerable<ITargetInterface> ParamsTarget { get; set; }
135+
136+
[Benchmark]
137+
public int Benchmark() => ParamsTarget?.Sum(p => p?.Data ?? 0) ?? 0;
138+
}
139+
140+
[Theory, MemberData(nameof(GetToolchains))]
141+
public void PrivateClassWithPublicInterface_Enumerable_Succeeds(IToolchain toolchain) => CanExecuteWithExtraInfo(typeof(PrivateClassWithPublicInterface_Enumerable), toolchain);
142+
143+
public class PrivateClassWithPublicInterface_AsObject
144+
{
145+
public static IEnumerable<object> GetSource()
146+
{
147+
yield return null;
148+
yield return new NonPublicSource(1);
149+
yield return new NonPublicSource(2);
150+
}
151+
152+
[ParamsSource(nameof(GetSource))]
153+
public ITargetInterface ParamsTarget { get; set; }
154+
155+
[Benchmark]
156+
public int Benchmark() => ParamsTarget?.Data ?? 0;
157+
}
158+
159+
[Theory, MemberData(nameof(GetToolchains))]
160+
public void PrivateClassWithPublicInterface_AsObject_Succeeds(IToolchain toolchain) => CanExecuteWithExtraInfo(typeof(PrivateClassWithPublicInterface_AsObject), toolchain);
161+
162+
public class PublicSource
163+
{
164+
public int Data { get; }
165+
public PublicSource(int data) => Data = data;
166+
// op_Implicit would be meaningless because codegen wouldn't have to do anything.
167+
public static explicit operator TargetType(PublicSource @this) => @this != null ? new TargetType(@this.Data) : null;
168+
public override string ToString() => "src " + Data.ToString();
169+
}
170+
171+
public class TargetType
172+
{
173+
public int Data { get; }
174+
public TargetType(int data) => Data = data;
175+
public override string ToString() => "target " + Data.ToString();
176+
}
177+
178+
public class SourceWithExplicitCastToTarget
179+
{
180+
public static IEnumerable<PublicSource> GetSource()
181+
{
182+
yield return null;
183+
yield return new PublicSource(1);
184+
yield return new PublicSource(2);
185+
}
186+
187+
[ParamsSource(nameof(GetSource))]
188+
public TargetType ParamsTarget { get; set; }
189+
190+
[Benchmark]
191+
public int Benchmark() => ParamsTarget?.Data ?? 0;
192+
}
193+
194+
[Fact]
195+
public void SourceWithExplicitCastToTarget_DefaultToolchain_Succeeds() => CanExecuteWithExtraInfo(typeof(SourceWithExplicitCastToTarget), Job.Default.GetToolchain());
196+
197+
[Fact]
198+
public void SourceWithExplicitCastToTarget_InProcessToolchain_Throws()
199+
{
200+
// op_Explicit is currently not supported by InProcessEmitToolchain
201+
// See TryChangeType() in Toolchains/InProcess.Emit.Implementation/Runnable/RunnableReflectionHelpers.cs
202+
// If that changes, this test and the one above should be merged into:
203+
// [Theory, MemberData(nameof(GetToolchains))]
204+
// public void SourceWithExplicitCastToTarget_Succeeds(IToolchain toolchain) => CanExecuteWithExtraInfo(typeof(SourceWithExplicitCastToTarget), toolchain);
205+
Assert.ThrowsAny<Exception>(() => CanExecuteWithExtraInfo(typeof(SourceWithExplicitCastToTarget), InProcessEmitToolchain.Instance));
206+
}
38207
}
39208
}

0 commit comments

Comments
 (0)