Rewrite method infos earlier in ref safety analysis#79447
Rewrite method infos earlier in ref safety analysis#79447333fred merged 16 commits intodotnet:mainfrom
Conversation
Ref safety analysis did the same thing that nullable analysis did with respect rewriting new extensions into calling form, but did not do it immediately on seeing a call. We now do this as soon as the call is done, giving a more uniform handling of the analysis.
2dfaf21 to
b6ecfc8
Compare
|
@dotnet/roslyn-compiler @AlekseyTs @jcouv for reviews |
This reverts commit a993004.
…rlier * upstream/main: (217 commits) Fix tests Fix tests Fix tests Fix tests Fix tests Fix tests Reduce allocations during CommonCompletionItem.Create (dotnet#79591) Fix tests Fix tests Fix tests Fix tests Add test Fix tests Add work item Fix eol handling on the last token in a file when formatting code actions remove unchecked values from tests [main] Source code updates from dotnet/dotnet (dotnet#79599) Nullable extensions: Add assertion to AsMemberOfType and handle failures (dotnet#79428) Avoid adding dependency on System.Threading.Channels to InteractiveHost (dotnet#79594) Update debugger contracts to 18.0.0-beta.25353.1 (dotnet#79277) ...
This reverts commit d520c3e.
|
We should add a test for the scenario in #79654 |
|
@jjonescz @RikkiGibson @jaredpar please take another look |
It looks like this test was not added but this still seems worthwhile to me. |
|
@RikkiGibson @jjonescz for another look please. |
|
@RikkiGibson after taking another look at that bug, I do think it falls in the scope of this PR, as the fix is "analyze extension setters like they're method calls". I've pushed a change to do just that. |
|
@jjonescz @RikkiGibson please review |
|
@RikkiGibson please review |
| E.set_Property(s, x); | ||
|
|
||
| s.RefReceiverProperty = x; | ||
| E.set_RefReceiverProperty(ref s, x); |
There was a problem hiding this comment.
Can you also add the following test.
s = s.Property; // success
s = s.RefReceiverProperty; // failThere was a problem hiding this comment.
Ah. s.RefReceiverProperty is allowed to return an S that wraps ref s (the extension parameter), because ref-safe-context of s is return-only.
This seems to illustrate that extension authors should prefer scoped ref S s when S is ref struct, to make things resemble the defaults used for ref struct instance methods.
Ref safety analysis did the same thing that nullable analysis did with respect rewriting new extensions into calling form, but did not do it immediately on seeing a call. We now do this as soon as the call is done, giving a more uniform handling of the analysis. Fixes #79654.
Relates to test plan #76130