Skip to content

Fix IEquatable<T> nullness contravariance to match C# behavior #18770

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

Merged
merged 10 commits into from
Jul 25, 2025

Conversation

Copilot
Copy link
Contributor

@Copilot Copilot AI commented Jul 21, 2025

Problem

The F# compiler was emitting nullness warnings when casting non-nullable values to IEquatable<T>, while C# treats IEquatable<T> as contravariant for nullness purposes and suppresses such warnings. This inconsistency created friction when working with F# code that needed to match C# nullness behavior.

open System
let x = ""
let y = x :> IEquatable<string> // Was producing FS3261 nullness warning

Solution

Extended the constraint solver to treat IEquatable<T> type parameters as contravariant for nullness purposes, even though they're not formally marked as contravariant in IL. This matches the behavior implemented in Roslyn for C#.

Changes Made

  1. ConstraintSolver.fs: Modified SolveTypeEqualsTypeWithContravarianceEqns to:

    • Accept an optional TyconRef parameter to identify the type constructor
    • Added isIEquatableContravariantForNullness helper function that checks for IEquatable<T>
    • Extended contravariance logic to treat IEquatable<T> parameter 0 as contravariant for nullness
    • Added comprehensive documentation referencing the GitHub issues
  2. NullableReferenceTypesTests.fs: Added test case No nullness warning when casting non-nullable to IEquatable to validate the fix

Validation

  • ✅ New test passes: No nullness warnings for IEquatable<T> contravariance
  • ✅ All 167 existing NullableReferenceTypes tests continue to pass
  • ✅ Manual validation confirms compilation without spurious nullness warnings

References

The fix is minimal and surgical, only affecting the specific IEquatable<T> nullness contravariance case while preserving all other nullness checking behavior.

Warning

Firewall rules blocked me from connecting to one or more addresses

I tried to connect to the following addresses, but was blocked by firewall rules:

  • 1p7vsblobprodcus324.vsblob.vsassets.io
    • Triggering command: /usr/share/dotnet/dotnet msbuild -v:quiet -restore /home/REDACTED/.packagemanagement/nuget/Projects/112289--a67d0eeb-92ff-4473-9f1c-a88ee35e0deb/Project.fsproj /nologo /t:InteractivePackageManagement (dns block)
  • m8dvsblobprodcus37.vsblob.vsassets.io
    • Triggering command: /usr/share/dotnet/dotnet msbuild -v:quiet -restore /home/REDACTED/.packagemanagement/nuget/Projects/112289--a67d0eeb-92ff-4473-9f1c-a88ee35e0deb/Project.fsproj /nologo /t:InteractivePackageManagement (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

This pull request was created as a result of the following prompt from Copilot chat.

Problem

The F# compiler emits a nullness warning when casting a non-nullable value (like "") to IEquatable<string>. C# treats IEquatable<T> as contravariant for nullness, suppressing such warnings. This PR introduces a special case for IEquatable<T> in the F# type checker, so nullness warnings are not emitted for contravariant usage, matching C# behavior.

Implementation

  1. ConstraintSolver.fs

    • Extend the logic for contravariant type parameters in SolveTypeEqualsTypeWithContravarianceEqns to also treat the type parameter of IEquatable<T> as contravariant for nullness purposes, even if not marked as such by IL.
    • Add extensive comments explaining why this special case is needed, referencing issue #18759 and the analogous Roslyn issue (#37187).
  2. New Test for Nullness

    • Add a test based on the repro in issue #18759 to the Nullness test suite. The test should confirm that no warning is produced when casting a non-nullable value to IEquatable<string>.

References

Test Code Example

open System
let x = ""
let y = x :> IEquatable<string> // Should not warn about nullness

Notes

  • Extensive commenting should be included in the code to explain the special-casing.
  • The new test must fail if any nullness warning is produced for this scenario.

image1


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Copilot AI and others added 2 commits July 21, 2025 15:20
Co-authored-by: T-Gro <46543583+T-Gro@users.noreply.github.com>
Co-authored-by: T-Gro <46543583+T-Gro@users.noreply.github.com>
@Copilot Copilot AI changed the title [WIP] Special-case IEquatable<T> for nullness contravariance and add test for issue #18759 Fix IEquatable<T> nullness contravariance to match C# behavior Jul 21, 2025
@Copilot Copilot AI requested a review from T-Gro July 21, 2025 15:27
Copilot finished work on behalf of T-Gro July 21, 2025 15:27
Copy link
Contributor

github-actions bot commented Jul 22, 2025

❗ Release notes required


✅ Found changes and release notes in following paths:

Warning

No PR link found in some release notes, please consider adding it.

Change path Release notes path Description
src/Compiler docs/release-notes/.FSharp.Compiler.Service/10.0.100.md No current pull request URL (#18770) found, please consider adding it

Co-authored-by: T-Gro <46543583+T-Gro@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Jul 22, 2025

Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details.

Warning

Firewall rules blocked me from connecting to one or more addresses

I tried to connect to the following addresses, but was blocked by firewall rules:

  • 1p7vsblobprodcus324.vsblob.vsassets.io
    • Triggering command: /usr/share/dotnet/dotnet msbuild -v:quiet -restore /home/REDACTED/.packagemanagement/nuget/Projects/172297--5b6a182c-f6a1-4014-a333-5b2c4f96dc72/Project.fsproj /nologo /t:InteractivePackageManagement (dns block)
  • m8dvsblobprodcus37.vsblob.vsassets.io
    • Triggering command: /usr/share/dotnet/dotnet msbuild -v:quiet -restore /home/REDACTED/.packagemanagement/nuget/Projects/172297--5b6a182c-f6a1-4014-a333-5b2c4f96dc72/Project.fsproj /nologo /t:InteractivePackageManagement (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

@Copilot Copilot AI requested a review from T-Gro July 22, 2025 07:34
Copilot finished work on behalf of T-Gro July 22, 2025 07:34
Copilot finished work on behalf of T-Gro July 22, 2025 07:52
… .gitignore

Co-authored-by: T-Gro <46543583+T-Gro@users.noreply.github.com>
@Copilot Copilot AI requested a review from T-Gro July 22, 2025 08:30
Copilot finished work on behalf of T-Gro July 22, 2025 08:30
Copy link
Member

@T-Gro T-Gro left a comment

Choose a reason for hiding this comment

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

Good.
Just add release notes instructions for this in the usual format in docs release notes.
Treat this as "fixed" bug for upcasts to equatable.

@T-Gro T-Gro marked this pull request as ready for review July 22, 2025 10:27
@T-Gro T-Gro requested a review from a team as a code owner July 22, 2025 10:27
Co-authored-by: T-Gro <46543583+T-Gro@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Jul 22, 2025

Good.
Just add release notes instructions for this in the usual format in docs release notes.
Treat this as "fixed" bug for upcasts to equatable.

Added release notes entry for the IEquatable nullness fix in the preview.md file, treating it as a fixed bug for upcasts to equatable. Changes committed in 49de3f3.

@Copilot Copilot AI requested a review from T-Gro July 22, 2025 10:39
Copilot finished work on behalf of T-Gro July 22, 2025 10:39
@github-project-automation github-project-automation bot moved this from New to In Progress in F# Compiler and Tooling Jul 24, 2025
@Copilot Copilot AI requested a review from T-Gro July 25, 2025 09:32
Copilot finished work on behalf of T-Gro July 25, 2025 09:32
@T-Gro T-Gro enabled auto-merge (squash) July 25, 2025 09:46
@T-Gro T-Gro merged commit 53929f2 into main Jul 25, 2025
39 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Nullness bug when casting a string to an interface
3 participants