Skip to content
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

Filter symbol first before creating InheritanceMargin target #61911

Merged
merged 10 commits into from
Jun 21, 2022

Conversation

Cosifne
Copy link
Member

@Cosifne Cosifne commented Jun 15, 2022

Fixes AB#1514769
The reason for the IndexOutOfRange exception is because there is no target for a given InheritanceMarginItem.
And it happens because

Code defects here
At all the similar places, it creates an InheritanceMarginItem when there are base type symbols or derived type symbols.
However, there is another filtering later, which is checking if the symbol could be navigable.
And if this filter gets all the symbols out, then we end up with an InheritanceMarginItem with no targets.
This really happens in cases like

public class B : C
{
}
#line hidden
public class C
{
}

here for B, C is a hidden place, so it would be filtered by this line And for B, it has an empty InheritanceMarginItem.

The fix is:

  1. I did some code refactoring to make sure before InheritanceMarginItem is created, we should check the target items is not empty.
  2. Hidden places should be included.

@Cosifne Cosifne requested a review from a team as a code owner June 15, 2022 06:16
@Cosifne Cosifne marked this pull request as draft June 15, 2022 06:19
@Cosifne Cosifne force-pushed the dev/shech/InheritanceMarginTargetFilter branch from e34b0b0 to d9d9eb0 Compare June 15, 2022 23:17
@Cosifne Cosifne marked this pull request as ready for review June 15, 2022 23:44
@Cosifne Cosifne requested a review from CyrusNajmabadi June 15, 2022 23:44
@CyrusNajmabadi
Copy link
Member

Done with pass.

@Cosifne Cosifne requested a review from CyrusNajmabadi June 16, 2022 20:29
@Cosifne
Copy link
Member Author

Cosifne commented Jun 16, 2022

Tag @CyrusNajmabadi to have a second review

@CyrusNajmabadi
Copy link
Member

Looks great!

@Cosifne Cosifne enabled auto-merge (squash) June 21, 2022 20:34
@Cosifne Cosifne merged commit 69bdfb2 into dotnet:main Jun 21, 2022
@ghost ghost added this to the Next milestone Jun 21, 2022
@RikkiGibson RikkiGibson modified the milestones: Next, 17.3 P3 Jun 28, 2022
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.

3 participants