Skip to content
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

Nullness - downcasting and typetests should understand nullness information #17965

Merged
merged 15 commits into from
Nov 11, 2024

Conversation

T-Gro
Copy link
Member

@T-Gro T-Gro commented Nov 6, 2024

Addresses #17532

This revisits what downcasts and typetests do in the context of nullability analysis.
In the end, both unboxing and typetests are a runtime instruction, which is not aware of any nullability in the form | null. It is aware of internal nullability of NullTrueValue F# types, like option and unit.

The following is ensured:

  • Downcasting from a value which can carry null into a WithoutNull one gives a new warning
  • Downcasting from a non-nullable value into nullable gives (existing) warning about type erasure
  • Typetesting againts nullable versions of types gives warning about type erasure (the "isinst" instruction will always return false for null in the source, so typetesting for nullable versions does not make sense - except for type where null is a true value, where the FSharp.Core function understands this and special-cases types like unit and option)

The UnboxGeneric function in F# Core, in its current version, is doing a runtime test against null and throws if the runtime-known information is not a type accepting null. However, this is runtime and reflection, not aware of nullable analysis.
For that reason, downcasting into | null versions of types, including nullable generic type parameters, now avoid this UnboxGeneric call and immediately go to the unbox.any instruction (which turns null in the input into null in the output, instead of throwing).

Copy link
Contributor

github-actions bot commented Nov 6, 2024

❗ Release notes required


✅ Found changes and release notes in following paths:

Change path Release notes path Description
src/Compiler docs/release-notes/.FSharp.Compiler.Service/9.0.200.md

src/Compiler/TypedTree/TypedTreeOps.fs Outdated Show resolved Hide resolved
@T-Gro T-Gro marked this pull request as ready for review November 6, 2024 17:08
@T-Gro T-Gro requested a review from a team as a code owner November 6, 2024 17:08
@T-Gro
Copy link
Member Author

T-Gro commented Nov 6, 2024

/run fantomas

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

github-actions bot commented Nov 6, 2024

@T-Gro T-Gro merged commit f44e2b2 into main Nov 11, 2024
33 checks passed
@T-Gro T-Gro deleted the feature/nullable-downcasting branch November 11, 2024 16:56
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.

3 participants