Merge main into features/list-patterns by alrz · Pull Request #55072 · dotnet/roslyn

alrz · 2021-07-23T16:14:58Z

# Conflicts:
#	src/Compilers/CSharp/Portable/Binder/Binder_Expressions.cs
#	src/Compilers/CSharp/Portable/Binder/Binder_Patterns.cs
#	src/Compilers/CSharp/Portable/Errors/MessageID.cs
#	src/Compilers/CSharp/Portable/FlowAnalysis/DefiniteAssignment.cs
#	src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter.cs
#	src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_AssignmentOperator.cs
#	src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_Call.cs
#	src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_FunctionPointerInvocation.cs
#	src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_IndexerAccess.cs
#	src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_ObjectOrCollectionInitializerExpression.cs
#	src/Compilers/CSharp/Portable/PublicAPI.Unshipped.txt
#	src/Compilers/CSharp/Portable/xlf/CSharpResources.cs.xlf
#	src/Compilers/CSharp/Portable/xlf/CSharpResources.de.xlf
#	src/Compilers/CSharp/Portable/xlf/CSharpResources.es.xlf
#	src/Compilers/CSharp/Portable/xlf/CSharpResources.fr.xlf
#	src/Compilers/CSharp/Portable/xlf/CSharpResources.it.xlf
#	src/Compilers/CSharp/Portable/xlf/CSharpResources.ja.xlf
#	src/Compilers/CSharp/Portable/xlf/CSharpResources.ko.xlf
#	src/Compilers/CSharp/Portable/xlf/CSharpResources.pl.xlf
#	src/Compilers/CSharp/Portable/xlf/CSharpResources.pt-BR.xlf
#	src/Compilers/CSharp/Portable/xlf/CSharpResources.ru.xlf
#	src/Compilers/CSharp/Portable/xlf/CSharpResources.tr.xlf
#	src/Compilers/CSharp/Portable/xlf/CSharpResources.zh-Hans.xlf
#	src/Compilers/CSharp/Portable/xlf/CSharpResources.zh-Hant.xlf
#	src/Compilers/Test/Utilities/CSharp/TestOptions.cs

Revert "Ensure we don't try and build an empty table when modifying entries"

…elease/dev17.0-to-main

…ith no values (dotnet#54639)

…elease/dev17.0-to-release/dev17.0-vs-deps

…-release/dev17.0-vs-deps Merge release/dev17.0 to release/dev17.0-vs-deps

… into merges/main-to-features/FileScopedNamespaces

alrz · 2021-07-23T17:39:55Z

@jcouv I've had already started attempting a merge. Feel free to close if you're half way through.

jcouv · 2021-07-23T17:59:10Z

Cool! There were more merge conflicts than I expected, so I'm glad you took a stab :-)

alrz · 2021-07-23T18:01:35Z

Most of it is a removed unused arg which I regret doing it in this branch.
I listed conflicted files if you wanted to review.

jcouv · 2021-07-23T18:04:09Z

The only trouble is that we can't really review this PR as-is, as the merge conflicts are lost into the merge commit.
For situations where the merge isn't trivial (simple additive merges) we prefer to resolve all the conflicts in a separate commit which can be reviewed.

I think it's possible to re-do this with miminal effot:

re-do a merge without resolving conflicts (ie. commit the conflict markers)
use the last commit of this PR to update the files in your workspace to resolve conflicts again, commit that as the commit for resolving conflicts

jcouv · 2021-07-23T18:04:38Z

I listed conflicted files if you wanted to review.

Okay. I think that works too. I'll take a look that way

jcouv · 2021-07-23T18:05:54Z

I listed conflicted files if you wanted to review.

Actually that doesn't work :-/ GitHub won't let me jump to a specific file, because too many files changed...

alrz · 2021-07-23T18:13:48Z

I did a clean merge with "theirs" option to see the diff: https://github.com/dotnet/roslyn/compare/1cfbd6dc..81d9c767

Does that help?

jcouv · 2021-07-23T18:25:23Z

Does that help?

I'm not sure.
What are the two commits in https://github.com/dotnet/roslyn/compare/1cfbd6dc..81d9c767 ? Neither appears in this PR.

I'd still recommend the approach I described earlier. It is tried and tested (one merge commit keeping conflicts, one commit to resolve conflicts).

alrz · 2021-07-23T18:28:12Z

What are the two commits in https://github.com/dotnet/roslyn/compare/1cfbd6dc..81d9c767 ? Neither appears in this PR.

81d9c76 is the merge with manually resolved conflicts (I do see this in the PR)
1cfbd6d is the clean merge from main with all conflicts resolved to main (by -Xtheirs)

The diff basically gives you all manual changes.

# Conflicts: # src/Compilers/CSharp/Portable/Binder/Binder_Expressions.cs # src/Compilers/CSharp/Portable/Binder/Binder_Patterns.cs # src/Compilers/CSharp/Portable/Errors/MessageID.cs # src/Compilers/CSharp/Portable/FlowAnalysis/DefiniteAssignment.cs # src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter.cs # src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_AssignmentOperator.cs # src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_Call.cs # src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_FunctionPointerInvocation.cs # src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_IndexerAccess.cs # src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_ObjectOrCollectionInitializerExpression.cs # src/Compilers/CSharp/Portable/PublicAPI.Unshipped.txt # src/Compilers/CSharp/Portable/xlf/CSharpResources.cs.xlf # src/Compilers/CSharp/Portable/xlf/CSharpResources.de.xlf # src/Compilers/CSharp/Portable/xlf/CSharpResources.es.xlf # src/Compilers/CSharp/Portable/xlf/CSharpResources.fr.xlf # src/Compilers/CSharp/Portable/xlf/CSharpResources.it.xlf # src/Compilers/CSharp/Portable/xlf/CSharpResources.ja.xlf # src/Compilers/CSharp/Portable/xlf/CSharpResources.ko.xlf # src/Compilers/CSharp/Portable/xlf/CSharpResources.pl.xlf # src/Compilers/CSharp/Portable/xlf/CSharpResources.pt-BR.xlf # src/Compilers/CSharp/Portable/xlf/CSharpResources.ru.xlf # src/Compilers/CSharp/Portable/xlf/CSharpResources.tr.xlf # src/Compilers/CSharp/Portable/xlf/CSharpResources.zh-Hans.xlf # src/Compilers/CSharp/Portable/xlf/CSharpResources.zh-Hant.xlf # src/Compilers/Test/Utilities/CSharp/TestOptions.cs

alrz · 2021-07-23T18:42:19Z

I'd still recommend the approach I described earlier. It is tried and tested (one merge commit keeping conflicts, one commit to resolve conflicts).

Done.

jcouv · 2021-07-23T23:49:05Z

src/Compilers/CSharp/Portable/Binder/Binder_Expressions.cs

                        ? ErrorCode.ERR_ImplicitIndexIndexerWithName
                        : ErrorCode.ERR_ImplicitRangeIndexerWithName,
-                    arguments.Names[0].GetLocation());
+                    arguments.Names[0].GetValueOrDefault().Location);


Is a null location what we want for the OrDefault case, as opposed to Location.None?

This is a move. Changed in:

https://github.com/dotnet/roslyn/pull/52422/files#diff-94aa95e94cfdcc70e3ae5562a1fa1da77438bdb4c5499304caac8dc8e0133500R8015

src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_AssignmentOperator.cs

jcouv · 2021-07-24T00:00:43Z

src/Compilers/Test/Utilities/CSharp/TestOptions.cs

-=======
        public static readonly CSharpParseOptions RegularWithExtendedPropertyPatterns = RegularPreview;
->>>>>>> origin/main
+        public static readonly CSharpParseOptions RegularWithListPatterns = RegularPreview;


nit: this shouldn't be necessary anymore. The default version for all tests has been changed to preview.
Only LangVer tests will have to do something explicit (using RegularN for old and TemporaryPreview LangVer for new). TemporaryPreview will be replaced by RegularN+1 when the new language version is introduced.

The default version for all tests has been changed to preview.

Good to know. I can remove in some other PR (existing or upcoming) if that's ok.

If you'd like. This is just something to know moving forward. Hopefully it should be a simplification.

jcouv

Thanks much for restructuring the PR. It was much easier to review that way.
Looks good, just a question on possible null Location.

CyrusNajmabadi and others added 30 commits July 8, 2021 01:24

remove

f65d930

remove

5f6aaa2

remove

5f59bf9

remove

4e7b632

remove

de9b472

remove

387adac

remove

7a448f6

remove

346c24e

remove

f9cf008

Merge pull request dotnet#54664 from dotnet/revert-54647-17.0p2qb

7b00e21

Revert "Ensure we don't try and build an empty table when modifying entries"

Merge remote-tracking branch 'upstream/release/dev17.0' into merges/r…

5b7a57a

…elease/dev17.0-to-main

Ensure we don't try and build an empty table when modifying entries w…

5c29ded

…ith no values (dotnet#54639)

Merge remote-tracking branch 'upstream/release/dev17.0' into merges/r…

973cb56

…elease/dev17.0-to-release/dev17.0-vs-deps

Merge pull request dotnet#54683 from dotnet/merges/release/dev17.0-to…

0bc8176

…-release/dev17.0-vs-deps Merge release/dev17.0 to release/dev17.0-vs-deps

Switch progression search over to navto search by default.

146a002

Add missing warning to the non-nullable warnings list.

cf9f752

Merge remote-tracking branch 'upstream/features/FileScopedNamespaces'…

77c3300

… into merges/main-to-features/FileScopedNamespaces

Always map symbols

e8ac981

Always report symbols

abcc9da

remove

f1427ab

update docs

a5b9fbd

Update docs

16206a8

Update docs

86f0d4e

Update docs

c659fb1

cleanup

945019e

docs

3d8dd9e

docs

961bf6a

Update

c452e45

Update

c1a6d5a

Update

1308b85

alrz closed this Jul 23, 2021

alrz reopened this Jul 23, 2021

alrz force-pushed the list-merge branch from 1da0af9 to 4b37712 Compare July 23, 2021 17:37

alrz added 4 commits July 23, 2021 23:08

Resolve conflicts

1097878

Simplify

d05daef

Fixup tests

74d4e58

alrz force-pushed the list-merge branch from 19d6585 to 74d4e58 Compare July 23, 2021 18:41

alrz marked this pull request as ready for review July 23, 2021 19:17

alrz requested review from a team as code owners July 23, 2021 19:17

alrz requested a review from a team July 23, 2021 19:17

jcouv reviewed Jul 23, 2021

View reviewed changes

src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_AssignmentOperator.cs Show resolved Hide resolved

jcouv reviewed Jul 24, 2021

View reviewed changes

jcouv approved these changes Jul 24, 2021

View reviewed changes

jcouv merged commit 2f1f839 into dotnet:features/list-patterns Jul 24, 2021

Conversation

alrz commented Jul 23, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

alrz commented Jul 23, 2021

Uh oh!

jcouv commented Jul 23, 2021

Uh oh!

alrz commented Jul 23, 2021

Uh oh!

jcouv commented Jul 23, 2021

Uh oh!

jcouv commented Jul 23, 2021

Uh oh!

jcouv commented Jul 23, 2021

Uh oh!

alrz commented Jul 23, 2021

Uh oh!

jcouv commented Jul 23, 2021

Uh oh!

alrz commented Jul 23, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

alrz commented Jul 23, 2021

Uh oh!

jcouv Jul 23, 2021

Choose a reason for hiding this comment

Uh oh!

alrz Jul 24, 2021

Choose a reason for hiding this comment

Uh oh!

jcouv Jul 24, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jcouv Jul 24, 2021

Choose a reason for hiding this comment

Uh oh!

alrz Jul 24, 2021

Choose a reason for hiding this comment

Uh oh!

jcouv Jul 24, 2021

Choose a reason for hiding this comment

Uh oh!

jcouv left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

19 participants

alrz commented Jul 23, 2021 •

edited

Loading

alrz commented Jul 23, 2021 •

edited

Loading