Python: Fix bad join order in TypeTracker::callStep #4683
Conversation
From a local evaluation against flask DB, after github#4649 was merged we would get: ``` Tuple counts for TypeTracker::callStep#ff/2@a21b71: 9876 ~0% {3} r1 = SCAN DataFlowPrivate::DataFlowCall::getArg_dispred#fff AS I OUTPUT I.<2>, I.<0>, I.<1> 9876 ~2% {3} r2 = JOIN r1 WITH project#DataFlowPrivate::DataFlowCall::getArg_dispred#fff AS R ON FIRST 1 OUTPUT r1.<2>, R.<0>, r1.<1> 72388997 ~0% {4} r3 = JOIN r2 WITH DataFlowPublic::ParameterNode::isParameterOf_dispred#fff_201#join_rhs AS R ON FIRST 1 OUTPUT r2.<2>, R.<2>, r2.<1>, R.<1> 4952 ~0% {2} r4 = JOIN r3 WITH DataFlowPrivate::DataFlowCall::getCallable_dispred#ff AS R ON FIRST 2 OUTPUT r3.<2>, r3.<3> return r4 ```
| } | ||
|
|
||
| /** | ||
| * Gets a callable for the call where `nodeFrom` is used as the `i`'th argument. |
There was a problem hiding this comment.
| * Gets a callable for the call where `nodeFrom` is used as the `i`'th argument. | |
| * Gets the callable for the call where `nodeFrom` is used as the `i`'th argument. |
I hope we only have one callable for each call, otherwise I think we confuse the data-flow library.
There was a problem hiding this comment.
ok, I wasn't sure, and went with the safer option.
There was a problem hiding this comment.
So I actually tried this out, and it turns out that it can have multiple results. See commit ab856d6.
I'm not sure whether this will be confusing for the data-flow library, since we're in TypeTracker land for these predicates 😊
There was a problem hiding this comment.
Ah, of course, I did feel that I had not been thinking long enough about this. This must be ok, since we cannot always know which callable to choose. I think it will also be alleviated by CFG splitting, but in general it will have to be ok.
Co-authored-by: yoff <lerchedahl@gmail.com>
2 participants
Duplicate of #4679, rebased on
09cfb24to make dist-upgrade easier. (but avoiding to change that PR, since we don't want to jeopardise the performance tests that are currently running against that PR).From a local evaluation against flask DB, after #4649 was merged we would get: