dotnet/roslyn

Simplify logic for deconstruction variables in ExpressionVariableFinder

Open

#15,064 opened on Nov 7, 2016

View on GitHub
 (0 comments) (0 reactions) (0 assignees)C# (4,257 forks)batch import
Area-CompilersConcept-Design DebtFeature - Tupleshelp wanted

Repository metrics

Stars
 (20,414 stars)
PR merge metrics
 (Avg merge 6d 17h) (256 merged PRs in 30d)

Description

Per discussion in PR https://github.com/dotnet/roslyn/pull/14871

  • Maybe we don't need MakeOutVariable and MakeDeconstructionVariable, instead one may suffice, by looking up the syntax tree.
  • Maybe we don't need CollectVariablesFromDeconstruction in ExpressionVariableFinder, instead we just need a VisitVariableDesignation which looks up the syntax tree to determine if it is an out or a deconstruction variable.
  • Then maybe we can re-use ExpressionVariableFinder instead of ForEachLoopBinder.CollectLocalsFromDeconstruction.

Contributor guide

Research direction
Read the linked PR discussion, then refactor ExpressionVariableFinder by merging MakeOutVariable and MakeDeconstructionVariable into a single method using syntax tree lookups, and consider reusing ExpressionVariableFinder for ForEachLoopBinder.CollectLocalsFromDeconstruction.
Tech stack
csharp
Domain
compilers
Issue type
Refactor
DifficultyEstimated implementation difficulty for a new contributor, from 1 for very small changes to 5 for expert-level work.
2
Estimated timeA rough time range for an experienced contributor to investigate, implement, test, and prepare a pull request.
1-3 hours
Activity statusHow available the issue appears right now: fresh, active, stale, blocked, or waiting on maintainer input.
Fresh
ClarityHow clearly the issue explains the expected change, acceptance criteria, and next step.
Clear
Prerequisites
C#RoslynGit
Newbie friendlinessA 1-100 score estimating how approachable this issue is for first-time contributors.
55