Skip to content

fix: Emit string.Concat instead of Expression.Add for string + operator #2

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
koenbeuk merged 2 commits into from
Mar 26, 2026
Merged

fix: Emit string.Concat instead of Expression.Add for string + operator #2

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2bb8eed
fix: Emit string.Concat instead of Expression.Add for string + operator
koenbeuk Mar 26, 2026
e7bda3d
fix: Replace broken fallbacks in EnsureStringConcat methods with throws
koenbeuk Mar 26, 2026
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
78 changes: 67 additions & 11 deletions src/ExpressiveSharp.Generator/Emitter/ExpressionTreeEmitter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -731,6 +731,15 @@ private string EmitBinary(IBinaryOperation binary)
return EmitUnsupported(binary);
}

// String concatenation via + uses string.Concat, not Expression.Add.
// Only matches the built-in compiler intrinsic (OperatorMethod is null).
// User-defined operator+ that returns string will have OperatorMethod set
// and is correctly handled by the existing MakeBinary path below.
if (binary.OperatorKind == BinaryOperatorKind.Add
&& binary.OperatorMethod is null
&& binary.Type?.SpecialType == SpecialType.System_String)
return EmitStringConcatenation(binary);

// Use checked variants when in a checked context
if (binary.IsChecked)
{
Expand Down Expand Up @@ -760,6 +769,26 @@ private string EmitBinary(IBinaryOperation binary)
return resultVar;
}

private string EmitStringConcatenation(IBinaryOperation binary)
{
var resultVar = NextVar();
var leftVar = EmitOperation(binary.LeftOperand);
var rightVar = EmitOperation(binary.RightOperand);

// Choose the correct Concat overload based on operand types.
// If both operands are string, use Concat(string, string).
// Otherwise (e.g. object + string from implicit boxing), use Concat(object, object).
var bothString = binary.LeftOperand.Type?.SpecialType == SpecialType.System_String
&& binary.RightOperand.Type?.SpecialType == SpecialType.System_String;

var concatMethod = bothString
? EnsureStringConcatMethod()
: EnsureStringConcatObjectMethod();

AppendLine($"var {resultVar} = {Expr}.Call({concatMethod}, {leftVar}, {rightVar});");
return resultVar;
}

private static string? MapBinaryOperatorKind(BinaryOperatorKind kind)
{
return kind switch
Expand Down Expand Up @@ -2096,20 +2125,31 @@ private string EnsureStringConcatMethod()
&& m.Parameters[0].Type.SpecialType == SpecialType.System_String
&& m.Parameters[1].Type.SpecialType == SpecialType.System_String);

if (concatMethod is not null)
{
_concatMethodField = _fieldCache.EnsureMethodInfo(concatMethod);
}
else
{
// Fallback: emit inline reflection
_concatMethodField = "_stringConcat";
_fieldCache.GetDeclarations(); // ensure we can add to it
}

_concatMethodField = _fieldCache.EnsureMethodInfo(concatMethod
?? throw new InvalidOperationException("string.Concat(string, string) not found in compilation"));
return _concatMethodField;
}

private string? _concatObjectMethodField;

private string EnsureStringConcatObjectMethod()
{
if (_concatObjectMethodField is not null)
return _concatObjectMethodField;

var stringType = _semanticModel.Compilation.GetSpecialType(SpecialType.System_String);
var concatMethod = stringType.GetMembers("Concat")
.OfType<IMethodSymbol>()
.FirstOrDefault(m => m.IsStatic
&& m.Parameters.Length == 2
&& m.Parameters[0].Type.SpecialType == SpecialType.System_Object
&& m.Parameters[1].Type.SpecialType == SpecialType.System_Object);

_concatObjectMethodField = _fieldCache.EnsureMethodInfo(concatMethod
?? throw new InvalidOperationException("string.Concat(object, object) not found in compilation"));
return _concatObjectMethodField;
}

private INamedTypeSymbol? _enumerableType;

private string? ResolveEnumerableMethod(string methodName, int paramCount, ITypeSymbol elementType)
Expand Down Expand Up @@ -2460,6 +2500,22 @@ private string EmitCompoundAssignment(ICompoundAssignmentOperation compoundAssig
return EmitUnsupported(compoundAssign);
}

// String += uses string.Concat, not Expression.Add
if (compoundAssign.OperatorKind == BinaryOperatorKind.Add
&& compoundAssign.OperatorMethod is null
&& compoundAssign.Type?.SpecialType == SpecialType.System_String)
{
var bothString = compoundAssign.Target.Type?.SpecialType == SpecialType.System_String
&& compoundAssign.Value.Type?.SpecialType == SpecialType.System_String;
var concatMethod = bothString
? EnsureStringConcatMethod()
: EnsureStringConcatObjectMethod();
var concatVar = NextVar();
AppendLine($"var {concatVar} = {Expr}.Call({concatMethod}, {targetVar}, {valueVar});");
AppendLine($"var {resultVar} = {Expr}.Assign({targetVar}, {concatVar});");
return resultVar;
}

if (compoundAssign.IsChecked)
{
exprType = exprType switch
Expand Down
5 changes: 3 additions & 2 deletions ...ConstructorTests.ProjectableConstructor_ReferencingBasePropertyInDerivedBody.verified.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ namespace ExpressiveSharp.Generated
{
private static readonly global::System.Reflection.ConstructorInfo _c0 = typeof(global::Foo.Child).GetConstructor(new global::System.Type[] { });
private static readonly global::System.Reflection.PropertyInfo _p0 = typeof(global::Foo.Base).GetProperty("Code");
private static readonly global::System.Reflection.MethodInfo _m0 = typeof(string).GetMethod("Concat", global::System.Reflection.BindingFlags.Public | global::System.Reflection.BindingFlags.NonPublic | global::System.Reflection.BindingFlags.Static, null, new global::System.Type[] { typeof(string), typeof(string) }, null);
private static readonly global::System.Reflection.PropertyInfo _p1 = typeof(global::Foo.Child).GetProperty("Label");

// [Expressive]
Expand All @@ -24,9 +25,9 @@ namespace ExpressiveSharp.Generated
var expr_3 = global::System.Linq.Expressions.Expression.Constant("[", typeof(string)); // "["
var p___this = global::System.Linq.Expressions.Expression.Parameter(typeof(object), "@this"); // Code
var expr_4 = global::System.Linq.Expressions.Expression.Property(p___this, _p0);
var expr_2 = global::System.Linq.Expressions.Expression.MakeBinary(global::System.Linq.Expressions.ExpressionType.Add, expr_3, expr_4);
var expr_2 = global::System.Linq.Expressions.Expression.Call(_m0, expr_3, expr_4);
var expr_5 = global::System.Linq.Expressions.Expression.Constant("]", typeof(string)); // "]"
var expr_1 = global::System.Linq.Expressions.Expression.MakeBinary(global::System.Linq.Expressions.ExpressionType.Add, expr_2, expr_5);
var expr_1 = global::System.Linq.Expressions.Expression.Call(_m0, expr_2, expr_5);
var expr_6 = global::System.Linq.Expressions.Expression.Bind(_p1, expr_1);
var expr_7 = global::System.Linq.Expressions.Expression.MemberInit(expr_0, expr_6);
return global::System.Linq.Expressions.Expression.Lambda<global::System.Func<string, global::Foo.Child>>(expr_7, p_code);
Expand Down
5 changes: 3 additions & 2 deletions ...onstructorTests.ProjectableConstructor_ReferencingPreviouslyAssignedProperty.verified.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ namespace ExpressiveSharp.Generated
{
private static readonly global::System.Reflection.ConstructorInfo _c0 = typeof(global::Foo.PersonDto).GetConstructor(new global::System.Type[] { });
private static readonly global::System.Reflection.PropertyInfo _p0 = typeof(global::Foo.PersonDto).GetProperty("FirstName");
private static readonly global::System.Reflection.MethodInfo _m0 = typeof(string).GetMethod("Concat", global::System.Reflection.BindingFlags.Public | global::System.Reflection.BindingFlags.NonPublic | global::System.Reflection.BindingFlags.Static, null, new global::System.Type[] { typeof(string), typeof(string) }, null);
private static readonly global::System.Reflection.PropertyInfo _p1 = typeof(global::Foo.PersonDto).GetProperty("LastName");
private static readonly global::System.Reflection.PropertyInfo _p2 = typeof(global::Foo.PersonDto).GetProperty("FullName");

Expand All @@ -28,9 +29,9 @@ namespace ExpressiveSharp.Generated
var p___this = global::System.Linq.Expressions.Expression.Parameter(typeof(object), "@this"); // FirstName
var expr_3 = global::System.Linq.Expressions.Expression.Property(p___this, _p0);
var expr_4 = global::System.Linq.Expressions.Expression.Constant(" ", typeof(string)); // " "
var expr_2 = global::System.Linq.Expressions.Expression.MakeBinary(global::System.Linq.Expressions.ExpressionType.Add, expr_3, expr_4);
var expr_2 = global::System.Linq.Expressions.Expression.Call(_m0, expr_3, expr_4);
var expr_5 = global::System.Linq.Expressions.Expression.Property(p___this, _p1); // LastName
var expr_1 = global::System.Linq.Expressions.Expression.MakeBinary(global::System.Linq.Expressions.ExpressionType.Add, expr_2, expr_5);
var expr_1 = global::System.Linq.Expressions.Expression.Call(_m0, expr_2, expr_5);
var expr_6 = global::System.Linq.Expressions.Expression.Bind(_p0, p_firstName);
var expr_7 = global::System.Linq.Expressions.Expression.Bind(_p1, p_lastName);
var expr_8 = global::System.Linq.Expressions.Expression.Bind(_p2, expr_1);
Expand Down
5 changes: 3 additions & 2 deletions ...nerator/ConstructorTests.ProjectableConstructor_ReferencingStaticConstMember.verified.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ namespace ExpressiveSharp.Generated
{
private static readonly global::System.Reflection.ConstructorInfo _c0 = typeof(global::Foo.PersonDto).GetConstructor(new global::System.Type[] { });
private static readonly global::System.Reflection.FieldInfo _f0 = typeof(global::Foo.PersonDto).GetField("Separator", global::System.Reflection.BindingFlags.Public | global::System.Reflection.BindingFlags.NonPublic | global::System.Reflection.BindingFlags.Static);
private static readonly global::System.Reflection.MethodInfo _m0 = typeof(string).GetMethod("Concat", global::System.Reflection.BindingFlags.Public | global::System.Reflection.BindingFlags.NonPublic | global::System.Reflection.BindingFlags.Static, null, new global::System.Type[] { typeof(string), typeof(string) }, null);
private static readonly global::System.Reflection.PropertyInfo _p0 = typeof(global::Foo.PersonDto).GetProperty("FullName");

// [Expressive]
Expand All @@ -23,8 +24,8 @@ namespace ExpressiveSharp.Generated
var p_last = global::System.Linq.Expressions.Expression.Parameter(typeof(string), "last");
var expr_0 = global::System.Linq.Expressions.Expression.New(_c0);
var expr_3 = global::System.Linq.Expressions.Expression.Field(null, _f0); // Separator
var expr_2 = global::System.Linq.Expressions.Expression.MakeBinary(global::System.Linq.Expressions.ExpressionType.Add, p_first, expr_3);
var expr_1 = global::System.Linq.Expressions.Expression.MakeBinary(global::System.Linq.Expressions.ExpressionType.Add, expr_2, p_last);
var expr_2 = global::System.Linq.Expressions.Expression.Call(_m0, p_first, expr_3);
var expr_1 = global::System.Linq.Expressions.Expression.Call(_m0, expr_2, p_last);
var expr_4 = global::System.Linq.Expressions.Expression.Bind(_p0, expr_1);
var expr_5 = global::System.Linq.Expressions.Expression.MemberInit(expr_0, expr_4);
return global::System.Linq.Expressions.Expression.Lambda<global::System.Func<string, string, global::Foo.PersonDto>>(expr_5, p_first, p_last);
Expand Down
3 changes: 2 additions & 1 deletion ...r/ConstructorTests.ProjectableConstructor_ThisInitializer_ChainedThisAndBase.verified.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ namespace ExpressiveSharp.Generated
{
private static readonly global::System.Reflection.ConstructorInfo _c0 = typeof(global::Foo.Child).GetConstructor(new global::System.Type[] { });
private static readonly global::System.Reflection.PropertyInfo _p0 = typeof(global::Foo.Child).GetProperty("Name");
private static readonly global::System.Reflection.MethodInfo _m0 = typeof(string).GetMethod("Concat", global::System.Reflection.BindingFlags.Public | global::System.Reflection.BindingFlags.NonPublic | global::System.Reflection.BindingFlags.Static, null, new global::System.Type[] { typeof(string), typeof(string) }, null);

// [Expressive]
// public Child(int id, string name, string suffix) : this(id, name)
Expand All @@ -24,7 +25,7 @@ namespace ExpressiveSharp.Generated
var expr_0 = global::System.Linq.Expressions.Expression.New(_c0);
var p___this = global::System.Linq.Expressions.Expression.Parameter(typeof(object), "@this"); // Name
var expr_2 = global::System.Linq.Expressions.Expression.Property(p___this, _p0);
var expr_1 = global::System.Linq.Expressions.Expression.MakeBinary(global::System.Linq.Expressions.ExpressionType.Add, expr_2, p_suffix);
var expr_1 = global::System.Linq.Expressions.Expression.Call(_m0, expr_2, p_suffix);
var expr_3 = global::System.Linq.Expressions.Expression.Bind(_p0, expr_1);
var expr_4 = global::System.Linq.Expressions.Expression.MemberInit(expr_0, expr_3);
return global::System.Linq.Expressions.Expression.Lambda<global::System.Func<int, string, string, global::Foo.Child>>(expr_4, p_id, p_name, p_suffix);
Expand Down
9 changes: 5 additions & 4 deletions ...erator/ConstructorTests.ProjectableConstructor_ThisInitializer_WithBodyAfter.verified.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ namespace ExpressiveSharp.Generated
private static readonly global::System.Reflection.ConstructorInfo _c0 = typeof(global::Foo.PersonDto).GetConstructor(new global::System.Type[] { });
private static readonly global::System.Reflection.MethodInfo _m0 = typeof(string).GetMethod("ToUpper", global::System.Reflection.BindingFlags.Public | global::System.Reflection.BindingFlags.NonPublic | global::System.Reflection.BindingFlags.Instance, null, new global::System.Type[] { }, null);
private static readonly global::System.Reflection.PropertyInfo _p0 = typeof(global::Foo.PersonDto).GetProperty("FirstName");
private static readonly global::System.Reflection.MethodInfo _m1 = typeof(string).GetMethod("Concat", global::System.Reflection.BindingFlags.Public | global::System.Reflection.BindingFlags.NonPublic | global::System.Reflection.BindingFlags.Static, null, new global::System.Type[] { typeof(string), typeof(string) }, null);
private static readonly global::System.Reflection.PropertyInfo _p1 = typeof(global::Foo.PersonDto).GetProperty("LastName");
private static readonly global::System.Reflection.PropertyInfo _p2 = typeof(global::Foo.PersonDto).GetProperty("FullName");

Expand All @@ -28,15 +29,15 @@ namespace ExpressiveSharp.Generated
var p___this = global::System.Linq.Expressions.Expression.Parameter(typeof(object), "@this"); // FirstName
var expr_5 = global::System.Linq.Expressions.Expression.Property(p___this, _p0);
var expr_6 = global::System.Linq.Expressions.Expression.Constant(" ", typeof(string)); // " "
var expr_4 = global::System.Linq.Expressions.Expression.MakeBinary(global::System.Linq.Expressions.ExpressionType.Add, expr_5, expr_6);
var expr_4 = global::System.Linq.Expressions.Expression.Call(_m1, expr_5, expr_6);
var expr_7 = global::System.Linq.Expressions.Expression.Property(p___this, _p1); // LastName
var expr_3 = global::System.Linq.Expressions.Expression.MakeBinary(global::System.Linq.Expressions.ExpressionType.Add, expr_4, expr_7);
var expr_3 = global::System.Linq.Expressions.Expression.Call(_m1, expr_4, expr_7);
var expr_2 = global::System.Linq.Expressions.Expression.Call(expr_3, _m0, global::System.Array.Empty<global::System.Linq.Expressions.Expression>());
var expr_10 = global::System.Linq.Expressions.Expression.Property(p___this, _p0); // FirstName
var expr_11 = global::System.Linq.Expressions.Expression.Constant(" ", typeof(string)); // " "
var expr_9 = global::System.Linq.Expressions.Expression.MakeBinary(global::System.Linq.Expressions.ExpressionType.Add, expr_10, expr_11);
var expr_9 = global::System.Linq.Expressions.Expression.Call(_m1, expr_10, expr_11);
var expr_12 = global::System.Linq.Expressions.Expression.Property(p___this, _p1); // LastName
var expr_8 = global::System.Linq.Expressions.Expression.MakeBinary(global::System.Linq.Expressions.ExpressionType.Add, expr_9, expr_12);
var expr_8 = global::System.Linq.Expressions.Expression.Call(_m1, expr_9, expr_12);
var expr_1 = global::System.Linq.Expressions.Expression.Condition(p_upper, expr_2, expr_8, typeof(string));
var expr_13 = global::System.Linq.Expressions.Expression.Bind(_p2, expr_1);
var expr_14 = global::System.Linq.Expressions.Expression.MemberInit(expr_0, expr_13);
Expand Down
3 changes: 2 additions & 1 deletion ...onstructorTests.ProjectableConstructor_ThisInitializer_WithIfElseInDelegated.verified.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ namespace ExpressiveSharp.Generated
{
private static readonly global::System.Reflection.ConstructorInfo _c0 = typeof(global::Foo.PersonDto).GetConstructor(new global::System.Type[] { });
private static readonly global::System.Reflection.PropertyInfo _p0 = typeof(global::Foo.PersonDto).GetProperty("Label");
private static readonly global::System.Reflection.MethodInfo _m0 = typeof(string).GetMethod("Concat", global::System.Reflection.BindingFlags.Public | global::System.Reflection.BindingFlags.NonPublic | global::System.Reflection.BindingFlags.Static, null, new global::System.Type[] { typeof(string), typeof(string) }, null);

// [Expressive]
// public PersonDto(int score, string prefix) : this(score)
Expand All @@ -23,7 +24,7 @@ namespace ExpressiveSharp.Generated
var expr_0 = global::System.Linq.Expressions.Expression.New(_c0);
var p___this = global::System.Linq.Expressions.Expression.Parameter(typeof(object), "@this"); // Label
var expr_2 = global::System.Linq.Expressions.Expression.Property(p___this, _p0);
var expr_1 = global::System.Linq.Expressions.Expression.MakeBinary(global::System.Linq.Expressions.ExpressionType.Add, p_prefix, expr_2);
var expr_1 = global::System.Linq.Expressions.Expression.Call(_m0, p_prefix, expr_2);
var expr_3 = global::System.Linq.Expressions.Expression.Bind(_p0, expr_1);
var expr_4 = global::System.Linq.Expressions.Expression.MemberInit(expr_0, expr_3);
return global::System.Linq.Expressions.Expression.Lambda<global::System.Func<int, string, global::Foo.PersonDto>>(expr_4, p_score, p_prefix);
Expand Down
Loading
Loading