Rust: Add (auto-generated) CFG node wrapper classes

[hvitved]

When writing queries that use data flow or control flow, it is tempting to map entities down to AST nodes and back again. For example, if we have a CFG node for an addition expression, in order to get a CFG node for the LHS one might write:

CfgNode getLhs(CfgNode add) {
  result.getAstNode() = add.getAstNode().getLhs()
}

However, the issue above is that we may loose information about control-flow-splitting, meaning we may mix the CFG node for the addition from one set of splits with the CFG node for the LHS for another set of splits.

To this end, it is convenient to have a layer on top of CfgNode, which lifts member predicates such as getLhs, and which is responsible for getting CFG splitting right:

class BinaryExprCfgNode extends ExprCfgNode {
  ExprCfgNode getLhs() { ... }
}

Then query writers that operate on CFG level or above, should never have to map down to the AST layer.

In this PR we add exactly this layer, and since we already auto-generate the AST layer based on a schema, we extend the auto-generation to also construct the CFG layer. However, only a subset of AST nodes makes sense to include in the CFG layer, so one has to explicitly annotate which classes to include (using the cfg = True annotation in annotations.py).

Some manual additions are also needed in the CFG layer, namely those AST classes that are themselves not auto-generated (such as the AssignmentExpr class), and some classes may need to have extra predicates added when their CFG children do not correspond directly to AST children (for example the getArgument predicate is added to CallExprBaseCfgNode).

[aibaars]

It would be great if subtypes would "inherit" the cfg property from their base class. That way you don't have to repeat it for every class (with a risk of forgetting some).

[hvitved]

As long as one could override the annotation, that could work. For example, we don't want ParensExpr in the CFG.

[paldepind]

Looks pretty amazing to me. I've left a few comments and questions.

Do we write "control flow" with or without a "-"? I was under the impression that we write it without the "-" as that what I've seen the most. Some quick grepping also seem to indicate that the variant without the "-" occurs more often in the repo.

[paldepind]

Tweaks the comments a bit. The "no RHS CFG node" makes it sound as if its a node and not an edge that isn't there.

Suggested change

	// In for example `if (a && true) ...` there is no RHS CFG node going into the
	// `[false] a && true` operation
	strictcount(ConditionalSuccessor cs | exists(last.getACfgNode().getASuccessor(cs)) | cs) = 1
	or
	// In for example `x && return`, there is no RHS CFG node going into the `&&` operation
	not c instanceof CfgImpl::NormalCompletion
	// In for example `if (a && true) ...`, there is no edge from the CFG node
	// for `true` into the `[false] a && true` node.
	strictcount(ConditionalSuccessor cs | exists(last.getACfgNode().getASuccessor(cs)) | cs) = 1
	or
	// In for example `x && return`, there is no edge from the node for
	// `return` into the `&&` node.
	not c instanceof CfgImpl::NormalCompletion

[paldepind]

Is there a reason why wee need this predicate for this consistency check specifically? For instance, I've seen dead ends in syntactically incorrect extracted Rust files. Should we instead do it for all consistency checks?

[hvitved]

We may want to do this for all (CFG) consistency checks.

[paldepind]

If one has a CallExprBaseCfgNode and do c.getArgList().getArg(n) one gets a plain Expr. That could be a footgun.

[hvitved]

It could, but I don't think there is an easy/nice way to get rid of this auto-generated predicate. The best solution, I think, is to get rid of the intermediate ArgList node altogether in the extractor.

[paldepind]

🎉

[paldepind]

I just stumbled upon this: CallExprCfgNode has a getExpr that is different from the getExpr on ExprCfgNode. It is a bit confusing that these member predicate names overlap. Perhaps it would make sense to name the former asExpr or asAstExpr or getAstExpr?

[hvitved]

I just stumbled upon this: CallExprCfgNode has a getExpr that is different from the getExpr on ExprCfgNode. It is a bit confusing that these member predicate names overlap. Perhaps it would make sense to name the former asExpr or asAstExpr or getAstExpr?

Both predicates are auto-generated, so that would not be easy. One can always do call.(ExprCfgNode).getExpr() to invoke the base getExpr predicate if needed.