-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Swift: Extend AST classes and add control-flow library #9268
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
Conversation
swift/ql/lib/codeql/swift/controlflow/internal/ControlFlowGraphImplSpecific.qll
Fixed
Show resolved
Hide resolved
swift/ql/lib/codeql/swift/controlflow/internal/ControlFlowGraphImpl.qll
Fixed
Show resolved
Hide resolved
swift/ql/lib/codeql/swift/controlflow/internal/ControlFlowGraphImpl.qll
Fixed
Show resolved
Hide resolved
swift/ql/lib/codeql/swift/controlflow/internal/ControlFlowGraphImpl.qll
Fixed
Show resolved
Hide resolved
swift/ql/lib/codeql/swift/controlflow/internal/ControlFlowGraphImplShared.qll
Fixed
Show fixed
Hide fixed
| exists(ControlFlowElement pred | this.appliesTo(pred) | this.hasSuccessor(pred, cfe, _)) | ||
| } | ||
|
|
||
| /** The `succ` relation restricted to predecessors `pred` that this split applies to. */ |
Check warning
Code scanning / CodeQL
Predicate QLDoc style.
swift/ql/lib/codeql/swift/controlflow/internal/ControlFlowGraphImplSpecific.qll
Fixed
Show fixed
Hide fixed
cd9e4f1 to
83bcb53
Compare
| @@ -1,5 +1,5 @@ | |||
| | main.swift:3:8:3:13 | ConditionElement | main.swift:3:8:3:13 | BinaryExpr | | |||
| | main.swift:10:17:10:24 | ConditionElement | main.swift:10:17:10:24 | ParenExpr | | |||
| | main.swift:10:17:10:24 | ConditionElement | main.swift:10:18:10:22 | BinaryExpr | | |||
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.
This change happens because of the implementation of convertsFrom in IdentityExpr. So now we skip past the parentheses.
| private import codeql.swift.generated.expr.Expr | ||
|
|
||
| class Expr extends ExprBase { } | ||
| class Expr extends ExprBase { | ||
| final override Expr getResolveStep() { convertsFrom(result) } |
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 don't really understand what's going on with the whole resolve mechanism - is it just the inverse of getConverted or is there more to it?
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.
That's pretty much it, yes. AST classes can override getResolveStep to not be included in the main AST (i.e., when navigating the parent/child nodes in the AST classes). This is done by autogenerating AST classes lik:
class FooBase {
Element getBar() {
exists(Bar bar |
bar_of_foo(this, bar) and
result = bar.resolve()
)
}
}where resolve is then the TC of getResolveStep.
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.
Currently, resolve only does something in the case of IdentityExprs (which in particular include parenthesis expressions), ExplicitCasts and ImplicitConversions. For the Expr classes we provide aliases for result = getResolveStep() as convertsFrom(result) (the result became a parameter because we couldn't think of a good name), and getConversion(x) can then be implemented as "whatever expression is the result that converted something from x" (i.e., Expr getConversion() { result.convertsFrom(this) }).
Does that make sense?
|
There's a bunch of implicit |
Most of them should be fixed in 4ba2984 now. There are a couple of ones left in the shared parts of the library, but I'd prefer that we fix those in a different PR. |
Commit-by-commit review recommended:
Note that all code has already been reviewed during the proof-of-concept phase. A couple of changes were needed to fit the code onto Swift 5.6 (instead of version 5.5 for which the code was developed).