KE2: Extract some expr/stmt kinds #17664
Conversation
| is KtLabeledExpression -> { | ||
| // TODO: we could handle this here, or in each child that might have a label | ||
| // We're handling it in the children with the below | ||
| extractExpression(e.baseExpression!!, callable, parent) |
There was a problem hiding this comment.
I guess we'll want to have a helper that logs an error, and returns an @errorexpr, if this is null?
There was a problem hiding this comment.
I think we can check at the end for all instances of !! and add the logging.
| val locId = tw.getLocation(e) | ||
| tw.writeStmts_throwstmt(id, stmtParent.parent, stmtParent.idx, callable) | ||
| tw.writeHasLocation(id, locId) | ||
| val thrown = e.thrownExpression!! |
| } | ||
| for ((catchIdx, catchClause) in e.catchClauses.withIndex()) { | ||
| val catchId = tw.getFreshIdLabel<DbCatchclause>() | ||
| tw.writeStmts_catchclause(catchId, id, catchIdx, callable) |
There was a problem hiding this comment.
It seems a little odd that the dbscheme has a catchclause but not a finallyclause. I wonder if we should change that.
| val parent: Label<out DbStmtparent> | ||
|
|
||
| val label = (loop.parent as? KtLabeledExpression)?.getLabelName() | ||
| if (label != null) { |
There was a problem hiding this comment.
This follows the logic of the old code, but with the PSI representation it would be more natural to me to write this as
val loopParent = loop.parent
if (loopParent is KtLabeledExpression) {
val label = loopParent.getLabelName()
There was a problem hiding this comment.
I think we'll remove this altogether later, and handle KtLabeledExpression explicitly in extractExpression.
There was a problem hiding this comment.
Might be worth adding a TODO comment, so we don't forget about it, then?
There was a problem hiding this comment.
It's already added above: https://github.com/github/codeql/pull/17664/files#diff-65f5da78cea2691a3682e464d58f0ed3f31dec172341765c46b9b2ec6f6aa4deR222, but I'll add an explicit todo here too.
| extractExpressionExpr(loop.condition, callable, id, 0, id) | ||
| } | ||
| } | ||
| val condition = loop.condition!! |
There was a problem hiding this comment.
I don't know OTTOMH if this should be an errorexpr if it's null, or if that might be valid
There was a problem hiding this comment.
condition is marked with IfNotParsed, which has the following description:
Comes along with @nullable to indicate null is only possible if parsing error present
So I think this should be an error expression.
|
|
||
| return id | ||
| val body = loop.body | ||
| if (body != null && bodyIdx != null) { |
There was a problem hiding this comment.
Are there some theoretically impossible situations we should log about here? e.g. non-null-body with nulll-index?
There was a problem hiding this comment.
It looks like the null case was introduced to handle enhanced for loops.
No description provided.