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

Code before:
        }

        /// <summary>
        /// Given an unvisited string concat binary operator and potential compound assignment left-hand side read, visits all the arguments for passing to
        /// <see cref="CreateStringConcat(SyntaxNode, ArrayBuilder{BoundExpression})"/> and performs any optimizations on the arguments that can be done. This
        /// includes coalescing consecutive constant strings or chars into a single string constant, and deconstructing nested string.Concat calls.
        /// </summary>
        private void CollectAndVisitConcatArguments(BoundBinaryOperator originalOperator, BoundExpression? visitedCompoundAssignmentLeftRead, out ArrayBuilder<BoundExpression> destinationArguments)
        {
            Debug.Assert(!_inExpressionLambda);
            destinationArguments = ArrayBuilder<BoundExpression>.GetInstance();
            var concatMethods = new WellKnownConcatRelatedMethods(_compilation);
            pushArguments(this, originalOperator, destinationArguments, ref concatMethods);
            if (visitedCompoundAssignmentLeftRead is not null)
            {
                // We don't expect to be able to optimize anything about the compound assignment left read, so we just add it as-is. This assert should be kept in sync
                // with the cases that can be optimized by the VisitAndAddConcatArgumentInReverseOrder method below; if we ever find a case that can be optimized, we may
                // need to consider whether to do so. The visiting logic in the parent function here depends on only one argument being added for a compound assignment
                // left read, so if we ever do introduce optimizations here that result in more than one argument being added to destinationArguments, we'll need to adjust
                // that logic.
#if DEBUG
                var precedingArgument = destinationArguments.Count > 0 ? destinationArguments[^1] : null;
                var (singleConcatArgument, nestedConcatArguments) = SimplifyConcatArgument(visitedCompoundAssignmentLeftRead, ref precedingArgument, ref concatMethods);
                // Simplify should have done no work and just returned the original argument
                Debug.Assert(ReferenceEquals(singleConcatArgument, visitedCompoundAssignmentLeftRead));
                Debug.Assert(precedingArgument is null || ReferenceEquals(precedingArgument, destinationArguments[^1]));
                Debug.Assert(nestedConcatArguments.IsDefault);
#endif
                destinationArguments.Add(visitedCompoundAssignmentLeftRead);
            }
            destinationArguments.ReverseContents();

            // We push these in reverse order to take advantage of the left-recursive nature of the tree and avoid needing a second stack
            static void pushArguments(LocalRewriter self, BoundBinaryOperator binaryOperator, ArrayBuilder<BoundExpression> arguments, ref WellKnownConcatRelatedMethods concatMethods)
            {
                while (true)
                {
                    if (shouldRecurse(binaryOperator.Right, out var right))
                    {
                        pushArguments(self, right, arguments, ref concatMethods);
                    }
                    else
                    {
                        self.VisitAndAddConcatArgumentInReverseOrder(binaryOperator.Right, argumentAlreadyVisited: false, arguments, ref concatMethods);
                    }

                    if (shouldRecurse(binaryOperator.Left, out var left))
                    {
                        binaryOperator = left;
                    }
                    else

Review comment (refactor):
>precedingArgument is null [](http://example.com/codeflow?start=29&length=25)

Is it Ok for this value to become `null` when it was not `null` before the `SimplifyConcatArgument` call? #Closed

Code after:
        }

        /// <summary>
        /// Given an unvisited string concat binary operator and potential compound assignment left-hand side read, visits all the arguments for passing to
        /// <see cref="CreateStringConcat(SyntaxNode, ArrayBuilder{BoundExpression})"/> and performs any optimizations on the arguments that can be done. This
        /// includes coalescing consecutive constant strings or chars into a single string constant, and deconstructing nested string.Concat calls.
        /// </summary>
        private void CollectAndVisitConcatArguments(BoundBinaryOperator originalOperator, BoundExpression? visitedCompoundAssignmentLeftRead, out ArrayBuilder<BoundExpression> destinationArguments)
        {
            Debug.Assert(!_inExpressionLambda);
            destinationArguments = ArrayBuilder<BoundExpression>.GetInstance();
            var concatMethods = new WellKnownConcatRelatedMethods(_compilation);
            pushArguments(this, originalOperator, destinationArguments, ref concatMethods);
            if (visitedCompoundAssignmentLeftRead is not null)
            {
                // We don't expect to be able to optimize anything about the compound assignment left read, so we just add it as-is. This assert should be kept in sync
                // with the cases that can be optimized by the VisitAndAddConcatArgumentInReverseOrder method below; if we ever find a case that can be optimized, we may
                // need to consider whether to do so. The visiting logic in the parent function here depends on only one argument being added for a compound assignment
                // left read, so if we ever do introduce optimizations here that result in more than one argument being added to destinationArguments, we'll need to adjust
                // that logic.
#if DEBUG
                var followingArgument = destinationArguments.Count > 0 ? destinationArguments[^1] : null;
                var (singleConcatArgument, nestedConcatArguments) = SimplifyConcatArgument(visitedCompoundAssignmentLeftRead, ref followingArgument, ref concatMethods);
                // Simplify should have done no work and just returned the original argument
                Debug.Assert(ReferenceEquals(singleConcatArgument, visitedCompoundAssignmentLeftRead));
                Debug.Assert((destinationArguments.Count == 0 && followingArgument is null) || (destinationArguments.Count != 0 && ReferenceEquals(followingArgument, destinationArguments[^1])));
                Debug.Assert(nestedConcatArguments.IsDefault);
#endif
                destinationArguments.Add(visitedCompoundAssignmentLeftRead);
            }
            destinationArguments.ReverseContents();

            // We push these in reverse order to take advantage of the left-recursive nature of the tree and avoid needing a second stack
            static void pushArguments(LocalRewriter self, BoundBinaryOperator binaryOperator, ArrayBuilder<BoundExpression> arguments, ref WellKnownConcatRelatedMethods concatMethods)
            {
                while (true)
                {
                    if (shouldRecurse(binaryOperator.Right, out var right))
                    {
                        pushArguments(self, right, arguments, ref concatMethods);
                    }
                    else
                    {
                        self.VisitAndAddConcatArgumentInReverseOrder(binaryOperator.Right, argumentAlreadyVisited: false, arguments, ref concatMethods);
                    }

                    if (shouldRecurse(binaryOperator.Left, out var left))
                    {
                        binaryOperator = left;
                    }
                    else