Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

Nullable: Exception, SystemException, Argument*Exception#23515

Merged
safern merged 2 commits intodotnet:NullableFeaturefrom
safern:NullableException
Mar 28, 2019
Merged

Nullable: Exception, SystemException, Argument*Exception#23515
safern merged 2 commits intodotnet:NullableFeaturefrom
safern:NullableException

Conversation

@safern
Copy link
Copy Markdown
Member

@safern safern commented Mar 28, 2019

No description provided.

Comment thread src/System.Private.CoreLib/src/System/Exception.CoreCLR.cs Outdated
}

public MethodBase TargetSite
public MethodBase? TargetSite
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.

Exception.Message is another one we should add to the "should it be nullable" list (it wasn't changed in the PR so I can't comment on it). @safern, could you set up a meeting to work through it with all the relevant parties? We're going to hit more of these. I've never seen null returned from Message, though (whereas I have from ToString).

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Done! I think it makes sense to at least return an empty string from Exception.Message -- I don't think it makes sense to return null, but that takes us back to the same discussion of ToString. Let's discuss this on the meeting and hopefully we all agree on something in order to keep making progress.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Going to merge as this one seems "clearer" to me that it shouldn't return null. However, we can revisit tomorrow in our meeting and fix if that is the case.

@safern safern merged commit aba605f into dotnet:NullableFeature Mar 28, 2019
@safern safern deleted the NullableException branch March 28, 2019 21:59
picenka21 pushed a commit to picenka21/runtime that referenced this pull request Feb 18, 2022
…clr#23515)

* Nullable: Exception, SystemException, Argument*Exception

* Add Debug.Assert rather than comment


Commit migrated from dotnet/coreclr@aba605f
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants