Skip to content

Allow foreach on typed null enumerables #80783

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.

Sign up for GitHub

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
CyrusNajmabadi merged 20 commits into from
Oct 21, 2025
Merged

Allow foreach on typed null enumerables #80783

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
c83025b
Initial plan
Copilot Oct 16, 2025
0b49420
Fix: Check for constant null before reporting ERR_ForEachMissingMembe…
Copilot Oct 16, 2025
570e4da
Fix: Allow foreach on null-typed enumerables
Copilot Oct 16, 2025
987f561
Improve foreach null tests per review feedback
Copilot Oct 16, 2025
2f4f633
Merge WorkItem and Fact attributes per review feedback
Copilot Oct 16, 2025
7f1c97c
Add tests for dynamic and multidimensional array null casts
Copilot Oct 16, 2025
2e8018b
Add tests for type parameter constrained to IEnumerable
Copilot Oct 16, 2025
da68821
Inline ReportConstantNullCollectionExpr method
Copilot Oct 17, 2025
6145ef9
Add tests for foreach on default, new(), and []
Copilot Oct 17, 2025
48965b2
Update tests
CyrusNajmabadi Oct 17, 2025
5ff37ef
Update tests
CyrusNajmabadi Oct 17, 2025
9ba39fe
Remove outdated comment about Dev10 behavior
Copilot Oct 18, 2025
75f4070
Address review feedback from @AlekseyTs
Copilot Oct 18, 2025
ed2f6ee
Change remaining tests to VerifyEmitDiagnostics per review feedback
Copilot Oct 18, 2025
a1afaba
Strengthen null check to require BoundLiteral
Copilot Oct 18, 2025
99001f8
Use TargetFramework.StandardAndCSharp for dynamic tests
Copilot Oct 18, 2025
431b3fb
Fix failing test: Revert ForEach_StringNotIEnumerable to VerifyDiagno…
Copilot Oct 18, 2025
e170275
Add missing types to systemSource and use VerifyEmitDiagnostics
Copilot Oct 18, 2025
c41ee21
Add System.Enum type definition to systemSource
Copilot Oct 18, 2025
74c24ea
Updaate baseline
CyrusNajmabadi Oct 20, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 5 additions & 30 deletions src/Compilers/CSharp/Portable/Binder/ForEachLoopBinder.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -938,7 +938,11 @@ EnumeratorResult getEnumeratorInfo(SyntaxNode syntax, SyntaxNode collectionSynta

if (collectionExprType is null) // There's no way to enumerate something without a type.
{
if (!ReportConstantNullCollectionExpr(collectionExpr, diagnostics))
if (collectionExpr is BoundLiteral && collectionExpr.ConstantValueOpt is { IsNull: true })
{
diagnostics.Add(ErrorCode.ERR_NullNotValid, collectionExpr.Syntax.Location);
}
else
{
// Anything else with a null type is a method group or anonymous function
diagnostics.Add(ErrorCode.ERR_AnonMethGrpInForEach, collectionSyntax.Location, collectionExpr.Display);
Expand All @@ -965,10 +969,6 @@ EnumeratorResult getEnumeratorInfo(SyntaxNode syntax, SyntaxNode collectionSynta
// The spec specifically lists the collection, enumerator, and element types for arrays and dynamic.
if (collectionExprType.Kind == SymbolKind.ArrayType || collectionExprType.Kind == SymbolKind.DynamicType)
{
if (ReportConstantNullCollectionExpr(collectionExpr, diagnostics))
{
return EnumeratorResult.FailedAndReported;
}
builder = GetDefaultEnumeratorInfo(syntax, builder, diagnostics, collectionExprType);
return EnumeratorResult.Succeeded;
}
Expand All @@ -979,10 +979,6 @@ EnumeratorResult getEnumeratorInfo(SyntaxNode syntax, SyntaxNode collectionSynta
if (SatisfiesGetEnumeratorPattern(syntax, collectionSyntax, ref builder, unwrappedCollectionExpr, isAsync, viaExtensionMethod: false, diagnostics))
{
collectionExpr = unwrappedCollectionExpr;
if (ReportConstantNullCollectionExpr(collectionExpr, diagnostics))
{
return EnumeratorResult.FailedAndReported;
}
return createPatternBasedEnumeratorResult(ref builder, unwrappedCollectionExpr, isAsync, viaExtensionMethod: false, diagnostics);
}

Expand Down Expand Up @@ -1014,11 +1010,6 @@ EnumeratorResult getEnumeratorInfo(SyntaxNode syntax, SyntaxNode collectionSynta
// Lowering will not use iterator info with strings, so it is ok.
if (!isAsync && collectionExprType.SpecialType == SpecialType.System_String)
{
if (ReportConstantNullCollectionExpr(collectionExpr, diagnostics))
{
return EnumeratorResult.FailedAndReported;
}

builder = GetDefaultEnumeratorInfo(syntax, builder, diagnostics, collectionExprType);
return EnumeratorResult.Succeeded;
}
Expand Down Expand Up @@ -1078,11 +1069,6 @@ private EnumeratorResult SatisfiesIEnumerableInterfaces(SyntaxNode collectionSyn
return EnumeratorResult.FailedNotReported;
}

if (ReportConstantNullCollectionExpr(collectionExpr, diagnostics))
Copy link
Contributor

@CyrusNajmabadi CyrusNajmabadi Oct 17, 2025 •
edited by jcouv
Loading

Choose a reason for hiding this comment

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

@copilot if there is only one usage of ReportConstantNullCollectionExpr left, can you inline it into that final location please. #Closed

Copy link
Contributor Author

Copilot AI Oct 17, 2025

Choose a reason for hiding this comment

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

Done in commit 2e8018b. Inlined the method since it was only used in one place.

{
return EnumeratorResult.FailedAndReported;
}

SyntaxNode errorLocationSyntax = collectionSyntax;

if (foundMultipleGenericIEnumerableInterfaces)
Expand Down Expand Up @@ -1203,17 +1189,6 @@ private EnumeratorResult SatisfiesIEnumerableInterfaces(SyntaxNode collectionSyn
return EnumeratorResult.Succeeded;
}

private bool ReportConstantNullCollectionExpr(BoundExpression collectionExpr, BindingDiagnosticBag diagnostics)
{
if (collectionExpr.ConstantValueOpt is { IsNull: true })
{
// Spec seems to refer to null literals, but Dev10 reports anything known to be null.
diagnostics.Add(ErrorCode.ERR_NullNotValid, collectionExpr.Syntax.Location);
return true;
}
return false;
}

private void GetDisposalInfoForEnumerator(SyntaxNode syntax, ref ForEachEnumeratorInfo.Builder builder, BoundExpression expr, bool isAsync, BindingDiagnosticBag diagnostics)
{
// NOTE: if IDisposable is not available at all, no diagnostics will be reported - we will just assume that
Expand Down
12 changes: 2 additions & 10 deletions src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenAwaitForeachTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9929,11 +9929,7 @@ public static async Task Main()
}
}";
CreateCompilationWithTasksExtensions(new[] { source, AsyncStreamsTypes }, parseOptions: TestOptions.Regular9)
.VerifyDiagnostics(
// (9,33): error CS0186: Use of null is not valid in this context
// await foreach (var i in (IAsyncEnumerable<int>)null)
Diagnostic(ErrorCode.ERR_NullNotValid, "(IAsyncEnumerable<int>)null").WithLocation(9, 33)
);
.VerifyEmitDiagnostics();
Copy link
Member

@jcouv jcouv Oct 21, 2025

Choose a reason for hiding this comment

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

It would be even better to verify execution (throws NRE). Comment also applies to other tests that no longer have errors

Copy link
Contributor

@CyrusNajmabadi CyrusNajmabadi Oct 21, 2025

Choose a reason for hiding this comment

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

@copilot can you figure out how tests validate that they throw at runtime, and update all the passing tests to then run and demonstrate that they do crash with a NullReferenceException as expected.

Copy link
Contributor

@CyrusNajmabadi CyrusNajmabadi Oct 21, 2025

Choose a reason for hiding this comment

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

will consider in followup.

}

[Fact]
Expand All @@ -9956,11 +9952,7 @@ public static async Task Main()
public IAsyncEnumerator<int> GetAsyncEnumerator() => throw null;
}";
CreateCompilationWithTasksExtensions(new[] { source, AsyncStreamsTypes }, parseOptions: TestOptions.Regular9)
.VerifyDiagnostics(
// (9,33): error CS0186: Use of null is not valid in this context
// await foreach (var i in (C)null)
Diagnostic(ErrorCode.ERR_NullNotValid, "(C)null").WithLocation(9, 33)
);
.VerifyEmitDiagnostics();
}

[Fact]
Expand Down
18 changes: 3 additions & 15 deletions src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenForEachTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2434,11 +2434,7 @@ public static void Main()
}
}";
CreateCompilation(source, parseOptions: TestOptions.Regular9)
.VerifyDiagnostics(
// (8,27): error CS0186: Use of null is not valid in this context
// foreach (var i in (IEnumerable<int>)null)
Diagnostic(ErrorCode.ERR_NullNotValid, "(IEnumerable<int>)null").WithLocation(8, 27)
);
.VerifyEmitDiagnostics();
}

[Fact]
Expand All @@ -2460,11 +2456,7 @@ public static void Main()
public IEnumerator<int> GetEnumerator() => throw null;
}";
CreateCompilation(source, parseOptions: TestOptions.Regular9)
.VerifyDiagnostics(
// (8,27): error CS0186: Use of null is not valid in this context
// foreach (var i in (C)null)
Diagnostic(ErrorCode.ERR_NullNotValid, "(C)null").WithLocation(8, 27)
);
.VerifyEmitDiagnostics();
}

[Fact]
Expand All @@ -2483,11 +2475,7 @@ public static void Main()
}
}";
CreateCompilation(source, parseOptions: TestOptions.Regular9)
.VerifyDiagnostics(
// (7,27): error CS0186: Use of null is not valid in this context
// foreach (var i in (int[])null)
Diagnostic(ErrorCode.ERR_NullNotValid, "(int[])null").WithLocation(7, 27)
);
.VerifyEmitDiagnostics();
}

[Fact]
Expand Down
Loading