Repository: dotnet/roslyn
File: src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_StringConcat.cs (C#)

Code before:
                else if (wellKnownConcatOptimizationMethods.IsCharToString(call, out var charExpression))
                {
                    argument = charExpression;
                }
            }
            // This is `strValue ?? ""`, possibly from a nested binary addition of an interpolated string. We can just directly use the left operand
            else if (argument is BoundNullCoalescingOperator { LeftOperand: { Type.SpecialType: SpecialType.System_String } left, RightOperand: BoundLiteral { ConstantValueOpt: { IsString: true, RopeValue.IsEmpty: true } } })
            {
                argument = left;
            }

            switch (argument.ConstantValueOpt)
            {
                case { IsNull: true } or { IsString: true, RopeValue.IsEmpty: true }:
                    // If this is a null constant or an empty string, then we don't need to include it in the final arguments list
                    return (null, default);

                case { IsString: true } or { IsChar: true }:
                    // See if we can merge this argument with the previous one
                    if (precedingArgument is { ConstantValueOpt: { IsString: true } or { IsChar: true } })
                    {
                        var constantValue = precedingArgument.ConstantValueOpt!;
                        var previous = getRope(constantValue);
                        var current = getRope(argument.ConstantValueOpt!);
                        // We're visiting arguments in reverse order, so we need to prepend this constant value, not append
                        precedingArgument = _factory.StringLiteral(ConstantValue.CreateFromRope(Rope.Concat(current, previous)));
                        return (null, default);
                    }

                    break;
            }

            return (argument, default);

            static Rope getRope(ConstantValue constantValue)
            {
                Debug.Assert(constantValue.IsString || constantValue.IsChar);
                if (constantValue.IsString)
                {
                    return constantValue.RopeValue!;
                }
                else
                {
                    return Rope.ForString(constantValue.CharValue.ToString());
                }
            }
        }

        /// <summary>
        /// Visits the given argument if necessary and adds it to the final arguments list. It is expected that <paramref name="finalArguments"/> is being built in reverse order, due to the left-recursive
        /// nature of the binary tree that we're traversing.

Review comment (suggestion):
>precedingArgument = _factory.StringLiteral(ConstantValue.CreateFromRope(Rope.Concat(current, previous))); [](http://example.com/codeflow?start=24&length=105)

It feels like the code will be much clearer if we name parameter "followingArgument" instead of "precedingArgument", because we expect the following argument and it is up to the consumer to reconcile this requirement with the way it visits things. Also, it looks like "previous" is actually "next". And we can remove the comment above because we will be calling things what they are.  #Closed

Code after:
                else if (wellKnownConcatOptimizationMethods.IsCharToString(call, out var charExpression))
                {
                    argument = charExpression;
                }
            }
            // This is `strValue ?? ""`, possibly from a nested binary addition of an interpolated string. We can just directly use the left operand
            else if (argument is BoundNullCoalescingOperator { LeftOperand: { Type.SpecialType: SpecialType.System_String } left, RightOperand: BoundLiteral { ConstantValueOpt: { IsString: true, RopeValue.IsEmpty: true } } })
            {
                argument = left;
            }

            switch (argument.ConstantValueOpt)
            {
                case { IsNull: true } or { IsString: true, RopeValue.IsEmpty: true }:
                    // If this is a null constant or an empty string, then we don't need to include it in the final arguments list
                    return (null, default);

                case { IsString: true } or { IsChar: true }:
                    // See if we can merge this argument with the next one
                    if (followingArgument is { ConstantValueOpt: { IsString: true } or { IsChar: true } })
                    {
                        var constantValue = followingArgument.ConstantValueOpt;
                        var next = getRope(constantValue);
                        var current = getRope(argument.ConstantValueOpt);
                        followingArgument = _factory.StringLiteral(ConstantValue.CreateFromRope(Rope.Concat(current, next)));
                        return (null, default);
                    }

                    break;
            }

            return (argument, default);

            static Rope getRope(ConstantValue constantValue)
            {
                Debug.Assert(constantValue.IsString || constantValue.IsChar);
                if (constantValue.IsString)
                {
                    return constantValue.RopeValue!;
                }
                else
                {
                    return Rope.ForString(constantValue.CharValue.ToString());
                }
            }
        }

        /// <summary>
        /// Visits the given argument if necessary and adds it to the final arguments list. It is expected that <paramref name="finalArguments"/> is being built in reverse order, due to the left-recursive
        /// nature of the binary tree that we're traversing.
        /// </summary>