Skip to content

Seal NullLogger #117342

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 1 commit into from
Jul 6, 2025
Merged

Seal NullLogger #117342

merged 1 commit into from
Jul 6, 2025

Conversation

stephentoub
Copy link
Member

Sealing it makes type checks against it cheaper. Some libraries special-case ILogger instances that are NullLogger in order to avoid some logging-related overheads.

While normally sealing a previously shipped public type would be a breaking change, NullLogger's only constructor is private, so types can't currently derive from it.

Sealing it makes type checks against it cheaper. Some libraries special-case ILogger instances that are NullLogger in order to avoid some logging-related overheads.

While normally sealing a previously shipped public type would be a breaking change, NullLogger's only constructor is private, so types can't currently derive from it.
@Copilot Copilot AI review requested due to automatic review settings July 6, 2025 02:22
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Seals the NullLogger class to make type checks against it cheaper without breaking compatibility (its constructor is private).

  • Changed NullLogger to a sealed class in the implementation assembly.
  • Updated the reference assembly to mark NullLogger as sealed.

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
src/libraries/Microsoft.Extensions.Logging.Abstractions/src/NullLogger.cs Added sealed modifier to NullLogger
src/libraries/Microsoft.Extensions.Logging.Abstractions/ref/Microsoft.Extensions.Logging.Abstractions.cs Added sealed to the partial NullLogger

Copy link
Contributor

Tagging subscribers to this area: @dotnet/area-extensions-logging
See info in area-owners.md if you want to be subscribed.

Copy link
Member

@MihaZupan MihaZupan left a comment

Choose a reason for hiding this comment

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

Is this just a one-off or did you write a script to look for such types?

@stephentoub
Copy link
Member Author

Is this just a one-off or did you write a script to look for such types?

One off being reminded of this by https://github.com/dotnet/runtime/pull/117334/files

@tarekgh tarekgh added this to the 10.0.0 milestone Jul 6, 2025
@tarekgh tarekgh merged commit cc37cdd into dotnet:main Jul 6, 2025
81 of 88 checks passed
@stephentoub stephentoub deleted the sealnulllogger branch July 7, 2025 14:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants