Track enclosing symbol in data flow analysis, which might be a field or property. #28252
Conversation
…or property. Code used to assume it was (and typed it as) a method. Fixes dotnet#19845 Fixes dotnet#27969
|
|
||
| static object Create(object name, Func<string, bool> f) => throw null; | ||
| } | ||
| "); |
There was a problem hiding this comment.
Is this test distinct from 01? #Resolved
There was a problem hiding this comment.
| /// </summary> | ||
| protected MethodSymbol currentMethodOrLambda { get; private set; } | ||
| protected Symbol currentMember { get; private set; } |
There was a problem hiding this comment.
Is it worth it to just call this CurrentSymbol? It's not necessarily a member (possibly a lambda or local function) and now it may not even be a function. This name just seems confusing.
Also, I believe our style guidelines require protected properties to be capitalized. #Closed
There was a problem hiding this comment.
We are more consistent in this area using lowercase spelling (e.g. topLevelMethod, nextVariableSlot). If we're going to make a change, I'd rather we not do it piecemeal.
In reply to: 199903748 [](ancestors = 199903748)
| @@ -283,7 +282,7 @@ protected override ImmutableArray<PendingBranch> RemoveReturns() | |||
|
|
|||
| protected virtual void ReportUnassignedOutParameter(ParameterSymbol parameter, SyntaxNode node, Location location) | |||
| { | |||
| if (!_requireOutParamsAssigned && topLevelMethod == currentMethodOrLambda) | |||
| if (!_requireOutParamsAssigned && topLevelMethod == (object)currentMember) | |||
There was a problem hiding this comment.
ReferenceEquals seems clearer here. I thought our (object) pattern was only for comparing to null. #Closed
There was a problem hiding this comment.
The cast to object can be used whenever needed to avoid the use of the declared operator ==, but I'm happy to use ReferenceEquals here.
In reply to: 199904132 [](ancestors = 199904132)
| @@ -439,7 +438,7 @@ private static bool IsCaptured(Symbol variable, MethodSymbol containingMethodOrL | |||
| currentFunction != null; | |||
There was a problem hiding this comment.
May want to add an object cast here too. #Resolved
|
@agocke I think the changes I've just pushed address your comments. |
| } | ||
| "); | ||
| var dataFlowAnalysisResults = analysisResults; | ||
| Assert.Equal("x", GetSymbolNamesJoined(dataFlowAnalysisResults.VariablesDeclared)); |
There was a problem hiding this comment.
Assert.Equal [](start = 12, length = 12)
Not related to this PR: consider adding a helper method in this test file to do the assert plus the GetSymbolNamesJoined, so that GetSymbolNamesJoined doesn't have to be repeated.
It could look like: ValidateSymbolNames("x", dataFlowAnalysisResults.VariablesDeclared)
| @@ -1443,7 +1442,7 @@ private void EnterParameters(ImmutableArray<ParameterSymbol> parameters) | |||
|
|
|||
| protected virtual void EnterParameter(ParameterSymbol parameter) | |||
| { | |||
| if (parameter.RefKind == RefKind.Out && !this.currentMethodOrLambda.IsAsync) // out parameters not allowed in async | |||
| if (parameter.RefKind == RefKind.Out && !(this.currentSymbol is MethodSymbol currentMethod && currentMethod.IsAsync)) // out parameters not allowed in async | |||
There was a problem hiding this comment.
is [](start = 73, length = 2)
recursive pattern? ;-) #Resolved
|
@jaredpar for ask-mode approval. Thanks |
|
approved |
…atures/compiler * dotnet/features/compiler: (137 commits) Actually fix tabs. Added back breaking changes doc. Addressed PR feedback. Track enclosing symbol in data flow analysis, which might be a field or property. (dotnet#28252) Change text from 'Spell check' to 'Fix typo'. Implement FAR for GetAwaiter methods (dotnet#28230) Fix typo Fix typo Fix import. Address PR feedback and fix failing test. Lower expressions of in arguments to correct temp locals (dotnet#28181) Move to 2.0.7 runtime Produce errors on ref-assignment to non-ref parameters Fix spelling. Preserve left-to-right evaluation order for dynamic compound addition and subtraction. inline. Provide a spellchecking suggestion when someone mispells a constructor. Typo (dotnet#28177) PR Comments Fix regression in bitness of Interactive Window host (dotnet#28006) ...
Code used to assume it was (and typed it as) a method.
Fixes #19845
Fixes #27969
@dotnet/roslyn-compiler May I please have a couple of reviews?
Customer scenario
Code containing a reference to a property in an initializer undergoes region analysis. The compiler crashes.
Bugs this fixes
Fixes #19845
Fixes #27969
Workarounds, if any
None known.
Risk
Low. This is a localized correction that widens the type of a field so that we can track the enclosing symbol when it is not a method.
Performance impact
None expected.
Is this a regression from a previous update?
Probably.
Root cause analysis
How was the bug found?
Customer reported.
Test documentation updated?
N/A