compilers: Use a dataclass instead of tuple for cached results
There's been a number of bugs where the code does something like:
def foo(...) -> T.Tuple[bool, bool]: ...
if foo():
...
Which will always resolve to True since a non-empty Tuple is truthy. To avoid that I've moved to a simple struct that implements a bool dunder, so that doing if result: does the expected thing.
@eli-schwartz, here is one option, as we discussed. I don't think it makes sense to wrap this in another level of abstraction just to drop the the cached argument.
simple struct that implements a bool dunder, so that doing
if result:does the expected thing.
TBH I've always found overridden __bool__() feels counterintuitive and don't like relying on it. I was expecting to always use the .result member...
Also it's somewhat difficult to eyeball the changes due to the large number of docstrings added, which really should go in a separate commit.
I've split the docstring and comment changes into a separate patch. I'm concerned about the __bool__ thing, since the real value here is that the class implements an explicit bool dunder instead of being truthy, which would mean that there is no benefit to using the struct and we should just leave it, since if CompileCheckResult(...) will always resolve to True, no matter what value is used.