-
Notifications
You must be signed in to change notification settings - Fork 66
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Handle InitializerListExpression
s as variables of ComprehensionExpression
s in the DFG
#2016
base: main
Are you sure you want to change the base?
Conversation
…ssions in the DFG
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some smaller remarks. In general this looks good, but I wonder whether we should move access
to Expression
.
cpg-core/src/main/kotlin/de/fraunhofer/aisec/cpg/graph/statements/ForEachStatement.kt
Outdated
Show resolved
Hide resolved
...ore/src/main/kotlin/de/fraunhofer/aisec/cpg/graph/statements/expressions/AssignExpression.kt
Outdated
Show resolved
Hide resolved
.../main/kotlin/de/fraunhofer/aisec/cpg/graph/statements/expressions/ComprehensionExpression.kt
Outdated
Show resolved
Hide resolved
* Is this reference used for writing data instead of just reading it? Determines dataflow | ||
* direction | ||
*/ | ||
var access: AccessValues |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On second thought. Wouldn't it make sense to just implements this on all Expression
nodes? The memory overhead is probably negligible and we would remove a lot of ugly as? HasAccess
casts. Basically this should apply to all expressions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I decided against this because many Expression
s cannot receive any data (i.e., AccessValues.WRITE
never makes sense for them and for some, also READ
might be confusing), so the access value is not useful there. Are the casts are a major concern? Any opinions on this @konradweiss ?
Adapts the
ControlFlowSensitiveDFGPass
to handle multiple declarations and references in the variable ofComprehensionExpression
s. To enhance the readability, the logic is moved to an own function.Fixes #1957, #2017
It further introduces a new interface
HasAccess
for Nodes which can be read from or written to. This allows to set the attributeaccess: AccessValues
for more nodes thanReference
and it also allows to implement a propagation logic per node class. Currently, the classesReference
,InitializerListExpression
andSubscriptExpression
implement this interface. It is mainly used to determine the direction of DFG edges.