Skip to content

Support NotNullIfNotNullAttribute#19977

Open
kerams wants to merge 5 commits into
dotnet:mainfrom
kerams:nn
Open

Support NotNullIfNotNullAttribute#19977
kerams wants to merge 5 commits into
dotnet:mainfrom
kerams:nn

Conversation

@kerams

@kerams kerams commented Jun 21, 2026

Copy link
Copy Markdown
Contributor

Only supported on C# and F# methods. Functions use an entirely different pathway. Also only one instance of the attribute is supported because at that point we can't evaluate nullness of relevant arguments to link the right one (but maybe we could introduce a new type of nullness linkage). It's also exceedingly rare...

Checklist

  • Test cases added
  • Performance benchmarks added in case of performance changes
  • Release notes entry updated

@github-actions

github-actions Bot commented Jun 21, 2026

Copy link
Copy Markdown
Contributor

❗ Release notes required

You can open this PR in browser to add release notes: open in github.dev


✅ Found changes and release notes in following paths:

Change path Release notes path Description
src/Compiler docs/release-notes/.FSharp.Compiler.Service/11.0.100.md
LanguageFeatures.fsi docs/release-notes/.Language/preview.md

@github-actions github-actions Bot added the ⚠️ Affects-Compiler-Output Tooling check: PR touches IL emission or codegen label Jun 21, 2026
@github-actions

Copy link
Copy Markdown
Contributor

🔍 Tooling Safety Check — Affects-Compiler-Output
Affects-Compiler-Output: adds nullability attribute handling to IL/checker/method-call paths

Generated by PR Tooling Safety Check · opus46 4.5M ·

open System.IO

let maybeNull : string | null = "file.txt"
let ext : string = Path.GetExtension(maybeNull)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I remember especially the Path APIs being heavy users of that - can we eliminate some "useless" null checking now? ( I guess too much hassle to do it via ifdefs now?)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Other than the 2 usages I had to adjust, it seems the return values are either used in a way where nullness doesn't matter (passing to String.Equals) or Unchecked.nonNull is used. I'd probably just strip the latter later once the new SDK has flowed back into the repo.

#nullable enable
using System.Diagnostics.CodeAnalysis;
namespace NotNullLib {
public class C {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Does C# support it also on generic arguments? (possibly also non-restricted to reference types, I remember some craziness with T? working )

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes,

[return: NotNullIfNotNull(nameof(x))]
public static T? Echo<T>(T? x) => x;


// The result is non-null when the SECOND parameter is non-null.
[return: NotNullIfNotNull("second")]
public static string? DependsOnSecond(string? first, string? second) => second;

@T-Gro T-Gro Jun 22, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Could you pls also do a test for type inference of an F# function argument ? And assert what is being inferred.

let f x =
     C.DependsOnSecond(x,x)

open System.IO

let maybeNull : string | null = "file.txt"
let ext : string = Path.GetExtension(maybeNull)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can you pls also do one where the null literal is passed directly as a method argument? Both when inlined into the method application, or as vanilla let x = null above.

There have been some regressions due to how the literal is type inferred initially.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

⚠️ Affects-Compiler-Output Tooling check: PR touches IL emission or codegen

Projects

Status: New

Development

Successfully merging this pull request may close these issues.

2 participants