Skip to content

Remove ActivityDisposable conditional directives and replace with System.IDisposable | null - #18707

Merged
T-Gro merged 3 commits into
mainfrom
copilot/fix-18706
Jun 24, 2025
Merged

Remove ActivityDisposable conditional directives and replace with System.IDisposable | null#18707
T-Gro merged 3 commits into
mainfrom
copilot/fix-18706

Conversation

CopilotAI commented Jun 23, 2025•
edited
Loading

Copy link
Copy Markdown
Contributor

This PR removes the conditional compilation directives for ActivityDisposable that were needed during the nullness bootstrapping phase when the "last known good" compiler had bugs around 'use' and nullable disposable handling.

Changes Made

  1. Removed conditional compilation block from NullnessShims.fs:

    // REMOVED:#if BUILDING_WITH_LKG
typeActivityDisposable= System.IDisposable
#elsetypeActivityDisposable= System.IDisposable |null#endif

  2. Updated function signatures in Activity.fs and Activity.fsi to directly use System.IDisposable | null:

    • start: name: string -> tags: (string * string) seq -> System.IDisposable | null
    • startNoTags: name: string -> System.IDisposable | null
    • startAndMeasureEnvironmentStats: name: string -> System.IDisposable | null

Impact

  • Simplifies the codebase by removing conditional compilation complexity
  • All ActivityDisposable references are now directly replaced with System.IDisposable | null
  • No functional changes to behavior - the type alias was already resolving to System.IDisposable | null in the non-LKG case
  • Maintains nullness safety while removing the bootstrapping workaround

The changes are minimal and surgical, affecting only the necessary files to complete the cleanup of the nullness bootstrapping infrastructure.

Fixes#18706.

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:

  • 1k9vsblobprodcus379.vsblob.vsassets.io
    • Triggering command: dotnet restore FSharp.Compiler.Service.sln (dns block)
  • 4vyvsblobprodcus361.vsblob.vsassets.io
    • Triggering command: dotnet restore FSharp.Compiler.Service.sln (dns block)

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

💬 Share your feedback on Copilot coding agent for the chance to win a $200 gift card! Click here to start the survey.

CopilotAIand others added 2 commits June 23, 2025 15:06
@T-Gro
Remove ActivityDisposable conditional directives and replace with Sys…
dc45d01
…tem.IDisposable | null
Co-authored-by: T-Gro <46543583+T-Gro@users.noreply.github.com>
@T-Gro
Apply code formatting with Fantomas
1113632
Co-authored-by: T-Gro <46543583+T-Gro@users.noreply.github.com>
CopilotAI changed the title [WIP] Conditional directives and ActivitityDisposable are no longer neededRemove ActivityDisposable conditional directives and replace with System.IDisposable | nullJun 23, 2025
CopilotAI requested a review from T-GroJune 23, 2025 15:12
@T-Gro
T-Gro marked this pull request as ready for review June 23, 2025 15:16
@T-Gro
T-Gro requested a review from a team as a code ownerJune 23, 2025 15:16
@T-Gro
T-Gro requested a review from abonieJune 23, 2025 15:17
@github-actions

github-actionsBot commented Jun 24, 2025•
edited
Loading

Copy link
Copy Markdown
Contributor

⚠️ 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

@T-Gro
T-Gro enabled auto-merge (squash) June 24, 2025 08:36
@T-GroT-Gro added the NO_RELEASE_NOTES Label for pull requests which signals, that user opted-out of providing release notes label Jun 24, 2025
@T-Gro
T-Gro merged commit dd55484 into mainJun 24, 2025
@github-project-automationgithub-project-automationBot moved this from New to In Progress in F# Compiler and ToolingJun 24, 2025
@github-project-automationgithub-project-automationBot moved this from In Progress to Done in F# Compiler and ToolingJun 24, 2025
T-Gro added a commit that referenced this pull request Jun 24, 2025
@T-Gro@edgarfgp
@dotnet-maestro@Smaug123@abonie
[automated] Merge branch 'main' => 'release/dev18.0' (#18711)
44a24dc
* More test for error recovery (#18669)
* [main] Update dependencies from dnceng/internal/dotnet-optimization (#18683)
* Update dependencies from https://dev.azure.com/dnceng/internal/_git/dotnet-optimization build 20250610.1
optimization.linux-arm64.MIBC.Runtime , optimization.linux-x64.MIBC.Runtime , optimization.windows_nt-arm64.MIBC.Runtime , optimization.windows_nt-x64.MIBC.Runtime , optimization.windows_nt-x86.MIBC.Runtime
From Version 1.0.0-prerelease.25309.1 -> To Version 1.0.0-prerelease.25310.1
* Update dependencies from https://dev.azure.com/dnceng/internal/_git/dotnet-optimization build 20250611.1
optimization.linux-arm64.MIBC.Runtime , optimization.linux-x64.MIBC.Runtime , optimization.windows_nt-arm64.MIBC.Runtime , optimization.windows_nt-x64.MIBC.Runtime , optimization.windows_nt-x86.MIBC.Runtime
From Version 1.0.0-prerelease.25309.1 -> To Version 1.0.0-prerelease.25311.1
* Update dependencies from https://dev.azure.com/dnceng/internal/_git/dotnet-optimization build 20250616.1
optimization.linux-arm64.MIBC.Runtime , optimization.linux-x64.MIBC.Runtime , optimization.windows_nt-arm64.MIBC.Runtime , optimization.windows_nt-x64.MIBC.Runtime , optimization.windows_nt-x86.MIBC.Runtime
From Version 1.0.0-prerelease.25309.1 -> To Version 1.0.0-prerelease.25316.1
* Update dependencies from https://dev.azure.com/dnceng/internal/_git/dotnet-optimization build 20250617.1
optimization.linux-arm64.MIBC.Runtime , optimization.linux-x64.MIBC.Runtime , optimization.windows_nt-arm64.MIBC.Runtime , optimization.windows_nt-x64.MIBC.Runtime , optimization.windows_nt-x86.MIBC.Runtime
From Version 1.0.0-prerelease.25309.1 -> To Version 1.0.0-prerelease.25317.1
---------
Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com>
Co-authored-by: Tomas Grosup <tomasgrosup@microsoft.com>
* [main] Update dependencies from dotnet/arcade (#18643)
* Expand docstring on SynModuleDecl.Types (#18698)
* Update dependencies from https://github.com/dotnet/arcade build 20250620.5 (#18703)
Microsoft.DotNet.Arcade.Sdk
From Version 10.0.0-beta.25316.2 -> To Version 10.0.0-beta.25320.5
Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com>
* Remove feature/lsp merge from branch-merge.yml (#18705)
Cleaning up, sicne we've merged feature/lsp to main some time ago
* Update dependencies from https://dev.azure.com/dnceng/internal/_git/dotnet-optimization build 20250623.1 (#18710)
optimization.linux-arm64.MIBC.Runtime , optimization.linux-x64.MIBC.Runtime , optimization.windows_nt-arm64.MIBC.Runtime , optimization.windows_nt-x64.MIBC.Runtime , optimization.windows_nt-x86.MIBC.Runtime
From Version 1.0.0-prerelease.25317.1 -> To Version 1.0.0-prerelease.25323.1
Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com>
* Update dependencies from https://github.com/dotnet/arcade build 20250623.3 (#18709)
Microsoft.DotNet.Arcade.Sdk
From Version 10.0.0-beta.25320.5 -> To Version 10.0.0-beta.25323.3
Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com>
* Remove ActivityDisposable conditional directives and replace with System.IDisposable | null (#18707)
---------
Co-authored-by: Edgar Gonzalez <egonzalez@totallymoney.com>
Co-authored-by: dotnet-maestro[bot] <42748379+dotnet-maestro[bot]@users.noreply.github.com>
Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com>
Co-authored-by: Tomas Grosup <tomasgrosup@microsoft.com>
Co-authored-by: Patrick Stevens <3138005+Smaug123@users.noreply.github.com>
Co-authored-by: Adam Boniecki <20281641+abonie@users.noreply.github.com>
Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com>
@dotnet-botdotnet-bot mentioned this pull request Jul 26, 2025
Closed
@dotnet-botdotnet-bot mentioned this pull request Aug 22, 2025
Closed
brianrourkeboll pushed a commit to brianrourkeboll/fsharp that referenced this pull request Sep 11, 2025
@brianrourkeboll
Remove ActivityDisposable conditional directives and replace with Sys…
6d3145f
…tem.IDisposable | null (dotnet#18707)
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@abonieabonieabonie approved these changes
+1 more reviewer
@T-GroT-GroT-Gro approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@T-GroT-Gro

Copilot code reviewCopilot

Labels

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

Projects

F# Compiler and Tooling
Archived in project

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Conditional directives and ActivitityDisposable are no longer needed

3 participants

@abonie@T-Gro