Dataflow: Support side-effects for callbacks in summaries. #6767
Conversation
Two questions. Otherwise LGTM.
| private predicate isContentOfArgument(SummaryComponentStack s) { | ||
| s.head() = TContentSummaryComponent(_) and isContentOfArgument(s.tail()) | ||
| or | ||
| s = TSingletonSummaryComponentStack(TArgumentSummaryComponent(_)) |
Should this rather be
s.head() = TContentSummaryComponent(_) and
s.tail() = TSingletonSummaryComponentStack(TArgumentSummaryComponent(_))?
No, I also meant to include, say, Element of Element of Argument[...].
It's basically mirroring the way that the shared dataflow lib is converting A --read--> B to post(B) --store--> post(A).
What I meant was whether the second disjunct above should be replaced with my suggesting. I.e. whether to disallow simply Argument[...].
Ah, right. No, I most definitely want to include just Argument[...]. That's likely the most important case. I guess the predicate name could be better - I'm open to suggestions.
| isContentOfArgument(output) | ||
| or | ||
| // flow from the receiver of a callback into the instance-parameter | ||
| exists(SummaryComponentStack s, SummaryComponentStack callbackRef | |
I am not convinced that this should go into the shared library; perhaps initially we should just have it in the Java library (and get rid of the thisParam predicate)?
I considered this to be a general OO thing. Consider a call x.apply(y), then the x plays two roles: it is used to determine the runtime dispatch target and it is being passed into the call as the instance argument. And since most languages have the concept of this being a parameter of member functions, then this feature felt pretty universal.
This could even be relevant for languages with lambdas that have no explicit way to reference themselves through a this parameter, since in most such cases lambdas support closures, which means that captured variables are actually stored in a closure object at the lambda construction point, which will likely need to be modelled as implicit instance fields on the implicit this of the lambda in order to get accurate flow.
So actually not just an OO thing. This is relevant for purely functional programming as well, since it's needed for proper support of closures.
OK, let's leave it in then.
|
Happy to approve once the failing test has been fixed. |
SummarizedCallables (only when the input spec isisContentOfArgument).input -> Parameter[0] of Argument[0]andParameter[0] of Argument[0] -> outputimplies a third implicitly added summaryinput -> output.The text was updated successfully, but these errors were encountered: