IOperation Node Porting: Non-Leaf Non-Hierarchy Nodes Pt3

[333fred]

This is the next set of non-leaf, non-hierarchy IOperation nodes. With this PR, we're ~55% of the way through the types defined in OperationInterfaces.xml. Once again, commit-by-commit is recommended. You might be able to get away with doing a few commits at a time, but reviewing this all at once is likely to be very hard to do (given that there are 28 ported nodes in this PR).

[333fred]

@dotnet/roslyn-compiler @jcouv @jaredpar this is ready for review.

[333fred]

@dotnet/roslyn-ide 456829a (#48892) marks IArgumentOperation.ParameterSymbol as nullable, because it can be if the argument is __arglist. However, this causes issues with the InlineMethodRefactoring, which assumes that IArgumentOperation.ParameterSymbol will never return null. The fix here isn't readily apparent to me: is there a simply way to block __arglist in this refactoring so we can assume it's never nullable?

[333fred]

@dotnet/roslyn-ide 456829a (#48892) marks IArgumentOperation.ParameterSymbol as nullable, because it can be if the argument is __arglist. However, this causes issues with the InlineMethodRefactoring, which assumes that IArgumentOperation.ParameterSymbol will never return null. The fix here isn't readily apparent to me: is there a simply way to block __arglist in this refactoring so we can assume it's never nullable?

@CyrusNajmabadi or @jasonmalinowski any pointers?

[jasonmalinowski]

Somewhere in

roslyn/src/Features/Core/Portable/InlineMethod/AbstractInlineMethodRefactoringProvider.cs

Lines 93 to 111 in 7da3c0f

	if (calleeMethodSymbol == null)
	{
	return;
	}
	if (calleeMethodSymbol.PartialImplementationPart != null)
	{
	calleeMethodSymbol = calleeMethodSymbol.PartialImplementationPart;
	}
	if (!calleeMethodSymbol.IsOrdinaryMethod() && !calleeMethodSymbol.IsExtensionMethod)
	{
	return;
	}
	if (calleeMethodSymbol.DeclaredAccessibility != Accessibility.Private)
	{
	return;
	}

maybe?

[jcouv]

Compiler portion LGTM Thanks (iteration 26)

[333fred]

@dotnet/roslyn-ide can I get a review on the IDE portions of this change? For the IDE sections, it's probably easiest to not review commit by commit, and instead just review all of it at once as there are not that many.

[333fred]

This is checked here: https://github.com/dotnet/roslyn/blob/master/src/Features/Core/Portable/ConvertIfToSwitch/AbstractConvertIfToSwitchCodeRefactoringProvider.cs#L113

[CyrusNajmabadi]

IDE side lgtm.

[cston]

                if (Reference is not null) builder.Add(Reference);

Could use builder.AddIfNotNull(Reference).

---

Refers to: src/Compilers/Core/Portable/Generated/Operations.Generated.cs:5798 in 2942c0f. [](commit_id = 2942c0f, deletion_comment = False)

[cston]

                if (!ElementValues.IsEmpty) builder.AddRange(ElementValues);

Consider omitting the check. It's not clear that it saves any cycles.

---

Refers to: src/Compilers/Core/Portable/Generated/Operations.Generated.cs:6235 in 2942c0f. [](commit_id = 2942c0f, deletion_comment = False)

[cston]

                Interlocked.CompareExchange(ref _lazyChildren, builder.ToImmutableAndFree(), null);

Since _lazyChildren is an IEnumerable<IOperation>, we're allocating an extra instance for the value, even for the empty case, because of boxing. Consider using builder.ToArrayAndFree() instead.

---

Refers to: src/Compilers/Core/Portable/Generated/Operations.Generated.cs:5799 in 2942c0f. [](commit_id = 2942c0f, deletion_comment = False)

[cston]

A few comments, none blocking.