search

dotnet / roslyn

The Roslyn .NET compiler provides C# and Visual Basic languages with rich code analysis APIs.
https://docs.microsoft.com/dotnet/csharp/roslyn-sdk/
MIT License
19.1k stars 4.04k forks source link

Avoid reusing temps whose refs might be captured #76009

Open jjonescz opened 4 days ago

jjonescz commented 4 days ago

Fixes https://github.com/dotnet/roslyn/issues/67435.

The idea is to have a "heuristic" to detect whether a method might capture the references passed to it. If such method call is detected, and a temporary reference is being emitted, we lift the temp to live for the whole block instead of just the expression.

The block lifetime is enough - ref safety analysis already checks refs to rvalues cannot escape blocks.

The heuristic is implemented by CodeGenerator.MightEscapeTemporaryRefs. It runs on the lowered nodes (because it's the emit layer which decides to emit a temporary). It might have false positives (some calls like M(rvalue, out _) might be marked by the heuristic as dangerous but they are not), but it shouldn't have false negatives.

Without a heuristic, we would need to avoid reusing many more temps, which would be a regression (at least in IL size). But perhaps that's negligible and it would be better to avoid this complexity? I'm not sure.

AlekseyTs commented 3 days ago

Done with review pass (commit 1), tests are not looked at.