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

Confusing NullReferenceException on default ImmutableArray #98300

Closed
thomhurst opened this issue Feb 12, 2024 · 3 comments
Closed

Confusing NullReferenceException on default ImmutableArray #98300

thomhurst opened this issue Feb 12, 2024 · 3 comments

Comments

@thomhurst
Copy link

thomhurst commented Feb 12, 2024

I was recently writing an Analyzer, and I was working with an ImmutableArray, iterating through it using LINQ to grab values.

My analyzer was then crashing when another project was using it, and there was no stacktrace to go off, just the exception type and message, which was this:

Warning AD0001 : Analyzer '' threw an exception of type 'System.NullReferenceException' with message 'Object reference not set to an instance of an object.'

After some digging, I eventually found out that it's because I was being passed a default ImmutableArray, which can't be iterated through.

Now that's fine, but what was so confusing was the NullReferenceException.

Because of the lack of stacktrace, I couldn't see where this was happening. So first thing was to check my code for any Nullability warnings, or double check that I'm not suppressing any somewhere.

I had no null warnings, and no null suppressions.

This made it super difficult to trace down.

Would it not make sense to throw a better exception, such as ImmutableArrayNotInitializedException?

It feels like if any code can throw a NullReferenceException, then the compiler should be able to tell us that via the nullability warnings. This however is an exception.

I know I can check the Is default property, which is what I'm now doing, but this is more to highlight the issue with going against the norm of nullability warnings via the compiler.

@ghost ghost added the untriaged New issue has not been triaged by the area owner label Feb 12, 2024
@dotnet-issue-labeler dotnet-issue-labeler bot added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Feb 12, 2024
@huoyaoyuan huoyaoyuan added area-System.Collections and removed needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners labels Feb 12, 2024
@ghost
Copy link

ghost commented Feb 12, 2024

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

Issue Details

I was recently writing an Analyzer, and I was working with an ImmutableArray, iterating through it using LINQ to grab values.

My analyzer was then crashing when another project was using it, and there was no stacktrace to go off, just the exception type and message, which was this:

Warning AD0001 : Analyzer '' threw an exception of type 'System.NullReferenceException' with message 'Object reference not set to an instance of an object.'

After some digging, I eventually found out that it's because I was being passed a default ImmutableArray, which can't be iterated through.

Now that's fine, but what was so confusing was the NullReferenceException.

Because of the lack of stacktrace, I couldn't see where this was happening. So first thing was to check my code for any Nullability warnings, or double check that I'm not suppressing any somewhere.

I had no null warnings, and no null suppressions.

This made it super difficult to trace down.

Would it not make sense to throw a better exception, such as ImmutableArrayNotInitializedException?

It feels like if any code can throw a NullReferenceException, then the compiler should be able to tell us that via the nullability warnings. This however is an exception.

Author: thomhurst
Assignees: -
Labels:

area-System.Collections, untriaged

Milestone: -

@huoyaoyuan
Copy link
Member

ImmutableArray<T> is an extra-slim wrapper of T[]. You should treat it as a variation of T[] in your mind.
Unfortunately, there's no non-defaultable struct feature to capture uninitialized structs currently. See dotnet/csharplang#146.

Would it not make sense to throw a better exception, such as ImmutableArrayNotInitializedException?

Performance is the key characteristic of ImmutableArray. Thus it can't do any check, but only accessing the underlying array directly. You can see the comments in its source code.

This is indeed some quirk that you need to remember. An analyzer flagging default(ImmutableArray<T>) may help.

@eiriktsarpalis
Copy link
Member

It is a known trade-off in immutable array design. As of C# 12 using collection literals should provide a simpler way to declare valid empty instances.

Closing in favor of dotnet/csharplang#146

@ghost ghost removed the untriaged New issue has not been triaged by the area owner label Feb 12, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Mar 14, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants