0

I stumbled across a specific 'is always false' warning from ReSharper (in Rider) that I first thought was incorrect, but from what I gathered from others usually if that happens ReSharper is smarter than the programmer, so I'm trying to understand and learn, maybe someone can help me.

I've recreated the situation in an isolated empty class:

private void Method(object obj)
{
    if (obj == null)
        return;
    var action = new Action(() =>
    {
        if (obj == null) // this condition is marked as always false
            return;
        // do stuff
    });
    // save reference to action somewhere
}

My thought process here was that I don't know when action might be called and while obj cannot be null when action is created, it very well could be when it is invoked. ReSharper seems to think differently.

Language version is C# 9.0, Nullable reference types are disabled.

Ardor
  • 154
  • 11
  • Where else are you altering `obj`? If nowhere - there is your answer. – mjwills Jun 03 '21 at 08:46
  • `if (obj == null) return;` - so whenever obj is null, _none of the consecutive code in the method will ever be executed_. and apparently, there's no code anywhere that set obj to null. however, you're not wrong to check for null. a warning doesn't necessarily say "you're wrong", but "watch out, make sure you know what you're doing" – Franz Gleichmann Jun 03 '21 at 08:47
  • 2
    I assume that respaher says that this expression is always false, because you already check if this obj equals null in previous if statement, above your action. It means that there is no any path when your obj can be null inside your action. – Stanislav Balia Jun 03 '21 at 08:48
  • @FranzGleichmann I know, but I save a reference to that action I create, I could call my method, then delete the object and then invoke the created action, then obj would be null. – Ardor Jun 03 '21 at 08:53
  • @mjwills you think resharper detects that the method is never called, therefore no object exists that could be deleted before the action is invoked? – Ardor Jun 03 '21 at 08:55
  • I think I understand the error of my ways. I assumed obj could be deleted at some point, but since the action I create itself has a reference to obj, GC will never collect it as long as this action exists. Is that correct? – Ardor Jun 03 '21 at 08:59
  • @Ardor The only way the code _could_ be meaningful was if _some other code_ _in this method_ set obj to another value. That is the only scenario where your code inside the action makes any sense whatsoever. Because if _nothing else assigns a value to obj_ then **by definition** `obj` will remain `null`. – mjwills Jun 03 '21 at 08:59
  • GC is completely irrelevant @Ardor. It is as simple as this - Resharper realises that `obj` cannot be `null` (since you checked it earlier **and never assigned it a different value**). So the later check is pointless. – mjwills Jun 03 '21 at 09:01
  • The earlier comment `It means that there is no any path when your obj can be null inside your action.` is overly simplistic - since it fails to acknowledge the path where `obj` _can_ be `null` - and that path is if you assign a value of `null` to `obj` _inside_ the method. Hence my first comment. But, if you _never_ assign a value to `obj` inside the method, well it remains `null` - so Resharper is being quite helpful. – mjwills Jun 03 '21 at 09:03
  • `then delete the object` Try it (and by `delete` I assume you mean `assign null to obj` - you will notice when you do that **the warning goes away**. – mjwills Jun 03 '21 at 09:05
  • @mjwills I think we are on the same page. I falsely thought the object itself could be deleted somehow, but I realize that is not possible. I actually meant 'delete the object', not 'assign null to obj'. That was the wrong assumption, that that could happen. That's what I meant when talking about GC, only the GC deletes the object, which it wont here, since my delegate itself has a reference to the object. But thank you very much, I understand now where I went wrong and it made me aware again of the fact that delegates can be an easy source for memory leaks. – Ardor Jun 03 '21 at 09:21
  • In this case the Action is not likely to cause a memory leak, since the scope of the Action is within the method. But yes, if the scope was wider it would effectively hold a reference to `obj` and stop it being eligible for GC. Either way - try assigning `null` to `obj` and see the warning disappear. Then you'll realise the kind of check it is doing. – mjwills Jun 03 '21 at 09:24

0 Answers0