Fix None assignments with fine-grained cache

[christianbundy]

Description

Problem: When using the fine-grained cache, we raise an assertion in
some situations when an attribute is defined as None.

Solution: Remove truthiness check to ensure that None is an allowed
type.

Fixes: #8682
Fixes: #11456

Test Plan

I've added a test that crashes in master and passes when this patch is applied.

cc: @sobolevn

[sobolevn]

Related: #11561

Similar problem.

[sobolevn]

I'm not sure how to encode this as an automated test.

Take a look at test-data/unit/fine-grained.test file 🙂

[christianbundy]

Thanks! I was able to add a test, repro the crash in master, and confirm that the test no longer fails in my branch.

I think this is good to go. ✅

[hauntsaninja]

Thanks, awesome work narrowing this down.

I don't know all the cases when lvalue.node.type is None and I'm not really familiar with the fine grained stuff. That is, I'm not sure I understand exactly what we should return here, e.g. returning UninhabitedType() instead of NoneType() also fixes the crash

[christianbundy]

UninhabitedType() instead of NoneType()

Is this preferable? I don't have an opinion either way, so whichever is more likely to be merged works for me.

[hauntsaninja]

Reading through the code, I think it doesn't make a practical difference, since TypeTriggersVisitor does the same thing for NoneType and UninhabitedType.
However, I think I'd still prefer using UninhabitedType, since it matches what we do in the else branch and earlier in the function, so I made that change :-)

Thanks again, tricky issue to narrow down!

[christianbundy]

Sounds good to me, thank you!