Skip to content

Chore :: Remove #IF NULLABLE, MaybeNull and other leftovers from bootstrap times of Nullable feature#19235

Merged
T-Gro merged 12 commits intomainfrom
tgro-cleanup-nullness-shims
Jan 23, 2026
Merged

Chore :: Remove #IF NULLABLE, MaybeNull and other leftovers from bootstrap times of Nullable feature#19235
T-Gro merged 12 commits intomainfrom
tgro-cleanup-nullness-shims

Conversation

@T-Gro
Copy link
Member

@T-Gro T-Gro commented Jan 22, 2026

No description provided.

T-Gro added 7 commits January 22, 2026 09:48
Remove all #if BUILDING_WITH_LKG || BUILD_FROM_SOURCE conditionals from
prim-types.fsi (7 blocks), prim-types.fs (2 blocks), event.fsi (2 blocks),
and event.fs (2 blocks).

For each block, kept the #else branch which includes:
- IsError=true in CompilerMessage attributes (prim-types.fsi)
- 'not null' type constraints on delegate types (all files)

These conditionals were only needed during F# 9 development when the
shipped .NET SDK didn't yet understand NRT. Now that NRT is stable,
this code can be unconditionally included.
- Update NullnessShims.fs: nullSafeEquality and NonEmptyString now use 'T | null
- Update lib.fs/fsi: dispose and DisposablesTracker.Register use 'T | null
- Update illib.fs: reportTime and DelayInitArrayMap use nullable types
- Update FileSystem.fs: isInvalidPath/Filename/Directory use pattern matching
- Update Activity.fs: escapeStringForCsv and appendWithLeadingComma use nullable
- Update Cancellable.fs: Using member uses _ | null
- Update LruCache.fs: removeCollected uses nullable LinkedListNode
- Minor formatting fix in event.fs (fantomas)

Note: Cancellable.fsi retains MaybeNull in signature for backwards compatibility
with consuming code (changing it causes FS3261 nullness errors downstream).
The MaybeNull type alias is kept in NullnessShims.fs for this purpose.
Replace all MaybeNull<'T> usages with the canonical 'T | null syntax in
FSharp.Build implementation files:
- Fsc.fs: 33 replacements
- Fsi.fs: 11 replacements
- FSharpCommandLineBuilder.fs: 2 method signature replacements
- WriteCodeFragment.fs: 2 replacements

The MaybeNull type definition is retained for backward compatibility.
Type annotations added to match expressions returning null to ensure
proper type inference with nullable types.
- Remove unused MaybeNull type alias definition from FSharpCommandLineBuilder.fs
- Remove unused (^) null-propagation operator from NullnessShims.fs
- Inline (^) usage in fsi.fs with direct null check pattern
- All FSharp.Build files now use 'T | null syntax directly
@T-Gro T-Gro marked this pull request as ready for review January 22, 2026 15:15
@T-Gro T-Gro requested a review from a team as a code owner January 22, 2026 15:15
@github-actions
Copy link
Contributor

github-actions bot commented Jan 22, 2026

⚠️ Release notes required, but author opted out

Warning

Author opted out of release notes, check is disabled for this pull request.
cc @dotnet/fsharp-team-msft

@github-project-automation github-project-automation bot moved this from New to In Progress in F# Compiler and Tooling Jan 22, 2026
@T-Gro T-Gro added the NO_RELEASE_NOTES Label for pull requests which signals, that user opted-out of providing release notes label Jan 22, 2026
@T-Gro T-Gro enabled auto-merge (squash) January 22, 2026 15:51
@T-Gro
Copy link
Member Author

T-Gro commented Jan 22, 2026

/run fantomas

@github-actions
Copy link
Contributor

🔧 CLI Command Report

  • Command: /run fantomas
  • Outcome: success

✅ Patch applied:
- Files changed: 2
- Lines changed: 54

bbatsov added a commit to bbatsov/fsharp that referenced this pull request Feb 18, 2026
The BUILDING_WITH_LKG constant was left behind after dotnet#19235 removed all
the #if conditionals that referenced it. It's now defined but never used
anywhere in the source. Remove it from both props files.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

NO_RELEASE_NOTES Label for pull requests which signals, that user opted-out of providing release notes

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

3 participants