Skip to content

Fix ValueTracking for index parameters #57727

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

Merged
merged 5 commits into from
Nov 16, 2021

Conversation

ryzngard
Copy link
Contributor

Index values weren't properly handled previously, and it was assumed all parameters existed in a method. In reality some exist in properties. Who knew!??!

No need to worry about the cast, symbol finder will do the right thing. Code does need to be updated to handle the fact that we want to track potentially two things with index: the argument passed in as the index in callsites, and potentially the item being indexed to. Update the FindReferencesProgress to handle references for index specifically.

Fixes #57100

…all parameters existed in a method. In reality some exist in properties. Who knew!??!

No need to worry about the cast, symbol finder will do the right thing. Code does need to be updated to handle the fact that we want to track potentially two things with index: the argument passed in as the index in callsites, and potentially the item being indexed to. Update the FindReferencesProgress to handle references for index specifically.
@ryzngard ryzngard requested a review from a team as a code owner November 12, 2021 07:18
@ghost ghost added the Area-IDE label Nov 12, 2021
@CyrusNajmabadi
Copy link
Member

Index values weren't properly handled previously, and it was assumed all parameters existed in a method. In reality some exist in properties. Who knew!??!

this can also be true for VB properties, if you haven't, can you test that out?

Also, it woudl be good to test tracking of the standard 'value' value in a C# property if we're not already doing that.

ryzngard and others added 3 commits November 12, 2021 00:53
@ryzngard
Copy link
Contributor Author

Index values weren't properly handled previously, and it was assumed all parameters existed in a method. In reality some exist in properties. Who knew!??!

this can also be true for VB properties, if you haven't, can you test that out?

Also, it woudl be good to test tracking of the standard 'value' value in a C# property if we're not already doing that.

Added tests for this :)

@ryzngard ryzngard merged commit c7521c9 into dotnet:main Nov 16, 2021
@ryzngard ryzngard deleted the issues/57100_value_tracking_index branch November 16, 2021 20:59
@ghost ghost added this to the Next milestone Nov 16, 2021
333fred added a commit to 333fred/roslyn that referenced this pull request Nov 17, 2021
…rations

* upstream/main: (3387 commits)
  Fix ValueTracking for index parameters (dotnet#57727)
  Avoid accessing current assembly identity while reporting an accessibility diagnostics for an inaccessible internal symbol. (dotnet#57783)
  Include a type for NoneOperations in VB, print the type in tests (dotnet#57664)
  Don't throw exceptions for file changes after a project is unloaded
  Check up front for being called to remove more than once
  Fix C# language name in spec (dotnet#57427)
  Add test
  Fix null ref in navbars
  Ensure that getting the checksum for a project cone is resilient to its project references being missing
  Check constraints on lifted operator types (dotnet#57050)
  Adjust tests for Windows 11 changes (dotnet#57678)
  Add comment
  Load SVsShellDebugger before calling IVsSolution.CreateSolution
  Remove extra EnsureEditableDocuments  calls (dotnet#57725)
  Don't show nullable annotation in completion items of static field/property
  Don't analyze local function bodies as though they are top level code (dotnet#57623)
  update error code to fix main break (dotnet#57739)
  Error when ref is used on a parameter or return type of an UnmanagedCallersOnly method (dotnet#57043)
  Simplify code from review
  Fix featureflag name for .net 6 host in UI
  ...
@allisonchou allisonchou modified the milestones: Next, 17.1.P2 Nov 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

InvalidCastException in ValueTracking
4 participants