C#: Fix compiler warning of possible null de-reference. #14693
Conversation
| @@ -30,6 +31,7 @@ protected CachedEntity(Context context) : base(context) | |||
| /// <typeparam name="TSymbol">The type of the symbol.</typeparam> | |||
| public abstract class CachedEntity<TSymbol> : CachedEntity where TSymbol : notnull | |||
| { | |||
| [NotNull] | |||
There was a problem hiding this comment.
There are a couple of #nullable disable warnings scattered around the code that relate to cached entities, such as in Compilation, Type, VarargsParam. It might be worth checking if these preprocessor directives can be also removed, or if any of the classes benefit from the Symbol not being marked as not null.
There was a problem hiding this comment.
Oh no! Good that you bring this up! Then the change as it is might not work! I will look into it.
There was a problem hiding this comment.
I just took a look at the inheritance hierarchy for CachedEntity.
There are many places where "null" is passed specifically as the Symbol and then most likely all the methods that rely on Symbol are overridden.
If it should really be fixed, then we probably need to distinguish between cached entities and nullable cached entities, which requires a larger re-write.
I think I will re-introduce the null check in the GetHashCode method to make sure that nothing gets broken (but in principle this method should always be overwritten in case Symbol could be null).
None of the disable nullable warnings can be removed as the Symbol can be (or is actually) null.
However, I think it is still a good idea to tag the property as "not null" as all the "correct" usages of the class shouldn't suffer from the hacks where the warning is disabled.
d1777de
to
0cf00eb
Compare
No description provided.