-
Notifications
You must be signed in to change notification settings - Fork 62
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
Improved property edges #1642
Merged
Merged
Improved property edges #1642
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
oxisto
force-pushed
the
better-property-edges
branch
from
August 2, 2024 19:37
d1fa626
to
94dcee8
Compare
oxisto
force-pushed
the
better-property-edges
branch
from
August 4, 2024 14:42
f9fdaee
to
b29ff80
Compare
KuechA
reviewed
Aug 5, 2024
cpg-analysis/src/main/kotlin/de/fraunhofer/aisec/cpg/passes/UnreachableEOGPass.kt
Outdated
Show resolved
Hide resolved
cpg-core/src/main/kotlin/de/fraunhofer/aisec/cpg/graph/declarations/FunctionDeclaration.kt
Outdated
Show resolved
Hide resolved
cpg-core/src/main/kotlin/de/fraunhofer/aisec/cpg/graph/edge/flows/ProgramDependence.kt
Outdated
Show resolved
Hide resolved
cpg-core/src/main/kotlin/de/fraunhofer/aisec/cpg/helpers/SubgraphWalker.kt
Outdated
Show resolved
Hide resolved
cpg-core/src/main/kotlin/de/fraunhofer/aisec/cpg/helpers/Util.kt
Outdated
Show resolved
Hide resolved
cpg-core/src/main/kotlin/de/fraunhofer/aisec/cpg/passes/ControlDependenceGraphPass.kt
Outdated
Show resolved
Hide resolved
...ain/kotlin/de/fraunhofer/aisec/cpg/graph/statements/expressions/InitializerListExpression.kt
Outdated
Show resolved
Hide resolved
oxisto
force-pushed
the
better-property-edges
branch
from
August 7, 2024 20:09
83aa531
to
a7b7263
Compare
oxisto
requested review from
peckto,
maximiliankaul and
konradweiss
as code owners
August 7, 2024 20:33
This PR adds major improvements to property edges
oxisto
force-pushed
the
better-property-edges
branch
from
August 9, 2024 07:37
9d732fc
to
37cdb1d
Compare
konradweiss
requested changes
Aug 9, 2024
cpg-core/src/main/kotlin/de/fraunhofer/aisec/cpg/graph/ExpressionBuilder.kt
Outdated
Show resolved
Hide resolved
cpg-core/src/main/kotlin/de/fraunhofer/aisec/cpg/graph/edges/collections/EdgeList.kt
Outdated
Show resolved
Hide resolved
cpg-core/src/main/kotlin/de/fraunhofer/aisec/cpg/graph/edges/flows/EvaluationOrder.kt
Show resolved
Hide resolved
Quality Gate passedIssues Measures |
konradweiss
approved these changes
Aug 9, 2024
This was referenced Sep 1, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
TL;DR
This PR adds major improvements to property edges. We got rid of the
Properties
map and replaced them with individual Kotlin properties. Furthermore lists of edges are now dedicated container classes with magic.Introduction of new specific edge classes
Next to the
Dataflow
edge class, which already exists, we added new edge classes. ThePropertyEdge
class itself is now abstract and thus no "generic" property edges (without any properties) can be created. There were a few uses of these kind of edges, this PR reverts them back to regular node lists (until we decide that this edge actually needs properties).Usage
This edge class denotes the usage of a declaration in a reference.
Invoke
Invocation of a function declaration by a call expression.
AstEdge
Part of the AST.
TemplateArgument
AST edge for template invocation arguments.
EvaluationOrder
Part of the EOG.
ControlDependence
Part of the CDG.
Conversion of properties in
Properties
map to individual propertiesThis is basically the consequence of the above:
INDEX
:index
toEdge
NAME
:name
toEdge
DEPENDENCE
:dependence
toEdge
UNREACHABLE
:unreachable
toEvaluationOrder
BRANCH
:branch
toEvaluationOrder
BRANCH
:branches
toControlDependence
INSTANTIATION
:instantiation
toTemplateArgument
Simplified creation of edges
Previously we had a lot of
addXXX
wrapper functions around property edges that took care of things like "mirroring" prev/next as well as inserting indices. This is all now done automagically by appropriate containers of property edges (and under thePropertyEdgeCollection
interface under the hood.This makes it easy to add new edges to the collection based on "target" nodes and it also supports setting appropriate properties using a builder pattern.
For all "mirrored" properties, this will also add the edge to the corresponding mirror property (next vs. prev). In order to use this feature the mirror property needs to be specified.
node1.nextDFGEdges.add(node2) // node2 will also now have node1 in its prevDFGEdges
Open Issues
PropertyEdge
toEdge
to be more consistent withNode
?unwrapping
(List
vs.MutableList
vsUnwrappedEdgeList
)