Skip to content
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.

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

Add interpolated string handler overloads for Assert methods #4273

Open
Youssef1313 opened this issue Dec 8, 2024 · 5 comments
Open

Add interpolated string handler overloads for Assert methods #4273

Youssef1313 opened this issue Dec 8, 2024 · 5 comments

Comments

@Youssef1313
Copy link
Member

Youssef1313 commented Dec 8, 2024

Summary

It's common to have Assert.SomeMethod(arguments, message: $"... {something} ... etc");.
The message is only needed when the assert actually fails, which hopefully isn't the common case when running tests :)
Currently, we are paying the cost to construct the string while it ends up not being used. That is exactly what interpolated string handlers tries to solve.

Background and Motivation

Introducing new interpolated string handlers overloads will solve the performance costs for this scenario.

See https://learn.microsoft.com/dotnet/csharp/advanced-topics/performance/interpolated-string-handler for info.

@Evangelink
Copy link
Member

We can probably just copy the class from Roslyn (I think I saw a similar PR at some point).

@Youssef1313
Copy link
Member Author

(I think I saw a similar PR at some point).

This one? dotnet/roslyn#58173 😄

@Evangelink
Copy link
Member

Also this dotnet/roslyn-analyzers#7416, might be worth to turn on the analyzer for us.

@Youssef1313
Copy link
Member Author

That analyzer is specific to Debug.Assert which is costly on older versions of .NET where there is no available overload that has interpolated string handler parameter which is a bit different than what I was suggesting in the original issue. Still, enabling that analyzer may bring benefits for us.

@Evangelink
Copy link
Member

So sorry! I read the ticket in diagonal and thought you were actually talking about our usages of Debug.Assert.

For Assertions, yes let's do that!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants