Skip to content

Conversation

@Erarndt
Copy link
Contributor

@Erarndt Erarndt commented Jun 5, 2025

Fixes #
Helps with
#10961

Context

The existing implementation of ConstructFunction() called Substring() and passed that value to a handful of helper functions. It turns out that we can wrap the string in ReadOnlyMemory<char> and pass that to the helper functions instead to avoid the additional allocation. Some of the functions needed to be updated to accept the new type.

Before:

image

After:

image

Changes Made

Testing

Notes

Copilot AI review requested due to automatic review settings June 5, 2025 22:48
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR refactors ConstructFunction and related helpers in Expander.cs to eliminate intermediate Substring allocations by leveraging ReadOnlySpan<char> and ReadOnlyMemory<char>.

  • Changed ScanForClosingParenthesis and ScanForClosingQuote to operate on spans.
  • Updated ExtractFunctionArguments to accept a ReadOnlyMemory<char> and use span-based slicing.
  • Replaced most Substring calls with span/memory slicing, except for a remaining Substring in ConstructIndexerFunction.
Comments suppressed due to low confidence (3)

src/Build/Evaluation/Expander.cs:708

  • The XML doc comment still refers to string expression, but the parameter is now ReadOnlySpan<char>. Update the summary and <param> tags to match the new type.
private static int ScanForClosingParenthesis(ReadOnlySpan<char> expression, int index, out bool potentialPropertyFunction, out bool potentialRegistryFunction)

src/Build/Evaluation/Expander.cs:3925

  • [nitpick] Mixing string and span/memory APIs can be confusing. Consider changing this method to accept ReadOnlyMemory<char> or ReadOnlySpan<char> for expressionFunction, so slicing logic is uniform and allocations are minimized.
private static void ConstructIndexerFunction(string expressionFunction, IElementLocation elementLocation, object propertyValue, int methodStartIndex, int indexerEndIndex, ref FunctionBuilder<T> functionBuilder)

src/Build/Evaluation/Expander.cs:828

  • The new span-based parsing logic introduces multiple edge cases (nested properties, quotes, commas). Add unit tests covering mismatched parentheses/quotes and empty or complex argument lists to ensure correctness.
private static string[] ExtractFunctionArguments(IElementLocation elementLocation, string expressionFunction, ReadOnlyMemory<char> argumentsMemory)

Copy link
Member

@SimaTian SimaTian left a comment

Choose a reason for hiding this comment

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

This directly helps with
#10961
I like it.

@SimaTian SimaTian merged commit 6e62322 into dotnet:main Jun 10, 2025
10 checks passed
@Erarndt Erarndt deleted the dev/erarndt/ConstructFunctionSubstring branch June 10, 2025 18:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants