Skip to content

List patterns: use the same binding path as range indexers#53784

Closed
alrz wants to merge 10 commits intodotnet:features/list-patternsfrom
alrz:list-patterns-03
Closed

List patterns: use the same binding path as range indexers#53784
alrz wants to merge 10 commits intodotnet:features/list-patternsfrom
alrz:list-patterns-03

Conversation

@alrz
Copy link
Copy Markdown
Member

@alrz alrz commented May 30, 2021

Test plan: #51289

@alrz

This comment has been minimized.

@alrz alrz marked this pull request as ready for review June 1, 2021 18:49
@alrz alrz requested review from a team as code owners June 1, 2021 18:49
@jcouv jcouv self-assigned this Jun 1, 2021
@alrz alrz force-pushed the list-patterns-03 branch from 5867adc to 265917f Compare June 3, 2021 17:29
@alrz alrz requested review from 333fred and jcouv June 3, 2021 17:43
@jcouv jcouv added this to the C# 10 milestone Jun 3, 2021
BindingDiagnosticBag diagnostics,
ref CompoundUseSiteInfo<AssemblySymbol> useSiteInfo,
bool forPattern = false)
ref CompoundUseSiteInfo<AssemblySymbol> useSiteInfo)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not for this PR: I was looking at this code some more (to merge conflicts from main branch) and wondered why we have both diagnostics and use-site info in this signature and some relates ones.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jcouv Should we create a DiagnosticBag off of useSiteInfo where we need one? I don't know if that's possible though.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@alrz I can't look at the full code at the moment, so this is a general thought:
If this method selects a property to use, then a diagnostic bag should be enough (no need to delay the "use").
If the method only returns a candidate, then it should only produce use-site information but not use a diagnostic bag. The caller which decides to use the candidate is responsible for recording the use (ie transferring use-site information into diagnostic bag).

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I think we should use useSiteInfo for everything here e.g. obsolete errors and others.

I was looking for AddDiagnostics(UseSiteInfo<TAssemblySymbol> info) I think that's the one we should use.

alrz added 2 commits June 10, 2021 14:05
…t-patterns-03

# Conflicts:
#	src/Compilers/CSharp/Portable/Binder/Binder_Expressions.cs
@alrz alrz marked this pull request as draft June 22, 2021 12:54
@alrz
Copy link
Copy Markdown
Member Author

alrz commented Jun 24, 2021

Merged to #54335

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants