Ruby: Eliminate unnecessary recursion through RealNode

Author: hvitved

ruby/ql/lib/codeql/ruby/ast/internal/AST.qll

@@ -167,7 +167,7 @@ private module Cached {
     TLTExpr(Ruby::Binary g) { g instanceof @ruby_binary_langle } or
     TLambda(Ruby::Lambda g) or
     TLeftAssignmentList(Ruby::LeftAssignmentList g) or
-    TLocalVariableAccessReal(Ruby::Identifier g, AST::LocalVariable v) {
+    TLocalVariableAccessReal(Ruby::Identifier g, TLocalVariableReal v) {
       LocalVariableAccess::range(g, v)
     } or
     TLocalVariableAccessSynth(AST::AstNode parent, int i, AST::LocalVariable v) {
@@ -284,13 +284,55 @@ private module Cached {
     TWhileModifierExpr(Ruby::WhileModifier g) or
     TYieldCall(Ruby::Yield g)
 
+  class TAstNodeReal =
+    TAddExprReal or TAliasStmt or TArgumentList or TAssignAddExpr or TAssignBitwiseAndExpr or
+        TAssignBitwiseOrExpr or TAssignBitwiseXorExpr or TAssignDivExpr or TAssignExponentExpr or
+        TAssignExprReal or TAssignLShiftExpr or TAssignLogicalAndExpr or TAssignLogicalOrExpr or
+        TAssignModuloExpr or TAssignMulExpr or TAssignRShiftExpr or TAssignSubExpr or
+        TBareStringLiteral or TBareSymbolLiteral or TBeginBlock or TBeginExpr or
+        TBitwiseAndExprReal or TBitwiseOrExprReal or TBitwiseXorExprReal or TBlockArgument or
+        TBlockParameter or TBraceBlock or TBreakStmt or TCaseEqExpr or TCaseExpr or
+        TCharacterLiteral or TClassDeclaration or TClassVariableAccessReal or TComplementExpr or
+        TComplexLiteral or TDefinedExpr or TDelimitedSymbolLiteral or TDestructuredLeftAssignment or
+        TDivExprReal or TDo or TDoBlock or TElementReference or TElse or TElsif or TEmptyStmt or
+        TEndBlock or TEnsure or TEqExpr or TExponentExprReal or TFalseLiteral or TFloatLiteral or
+        TForExpr or TForIn or TForwardParameter or TForwardArgument or TGEExpr or TGTExpr or
+        TGlobalVariableAccessReal or THashKeySymbolLiteral or THashLiteral or THashSplatExpr or
+        THashSplatParameter or THereDoc or TIdentifierMethodCall or TIf or TIfModifierExpr or
+        TInstanceVariableAccessReal or TIntegerLiteralReal or TKeywordParameter or TLEExpr or
+        TLShiftExprReal or TLTExpr or TLambda or TLeftAssignmentList or TLocalVariableAccessReal or
+        TLogicalAndExprReal or TLogicalOrExprReal or TMethod or TModuleDeclaration or
+        TModuloExprReal or TMulExprReal or TNEExpr or TNextStmt or TNilLiteral or
+        TNoRegExpMatchExpr or TNotExpr or TOptionalParameter or TPair or TParenthesizedExpr or
+        TRShiftExprReal or TRangeLiteralReal or TRationalLiteral or TRedoStmt or TRegExpLiteral or
+        TRegExpMatchExpr or TRegularArrayLiteral or TRegularMethodCall or TRegularStringLiteral or
+        TRegularSuperCall or TRescueClause or TRescueModifierExpr or TRetryStmt or TReturnStmt or
+        TScopeResolutionConstantAccess or TScopeResolutionMethodCall or TSelfReal or
+        TSimpleParameter or TSimpleSymbolLiteral or TSingletonClass or TSingletonMethod or
+        TSpaceshipExpr or TSplatExprReal or TSplatParameter or TStringArrayLiteral or
+        TStringConcatenation or TStringEscapeSequenceComponent or TStringInterpolationComponent or
+        TStringTextComponent or TSubExprReal or TSubshellLiteral or TSymbolArrayLiteral or
+        TTernaryIfExpr or TThen or TTokenConstantAccess or TTokenMethodName or TTokenSuperCall or
+        TToplevel or TTrueLiteral or TTuplePatternParameter or TUnaryMinusExpr or TUnaryPlusExpr or
+        TUndefStmt or TUnlessExpr or TUnlessModifierExpr or TUntilExpr or TUntilModifierExpr or
+        TWhenExpr or TWhileExpr or TWhileModifierExpr or TYieldCall;
+
+  class TAstNodeSynth =
+    TAddExprSynth or TAssignExprSynth or TBitwiseAndExprSynth or TBitwiseOrExprSynth or
+        TBitwiseXorExprSynth or TClassVariableAccessSynth or TConstantReadAccessSynth or
+        TDivExprSynth or TExponentExprSynth or TGlobalVariableAccessSynth or
+        TInstanceVariableAccessSynth or TIntegerLiteralSynth or TLShiftExprSynth or
+        TLocalVariableAccessSynth or TLogicalAndExprSynth or TLogicalOrExprSynth or
+        TMethodCallSynth or TModuloExprSynth or TMulExprSynth or TRShiftExprSynth or
+        TRangeLiteralSynth or TSelfSynth or TSplatExprSynth or TStmtSequenceSynth or TSubExprSynth;
+
   /**
    * Gets the underlying TreeSitter entity for a given AST node. This does not
    * include synthesized AST nodes, because they are not the primary AST node
    * for any given generated node.
    */
   cached
-  Ruby::AstNode toGenerated(AST::AstNode n) {
+  Ruby::AstNode toGenerated(TAstNodeReal n) {
     n = TAddExprReal(result) or
     n = TAliasStmt(result) or
     n = TArgumentList(result) or
@@ -495,7 +537,9 @@ private module Cached {
   predicate synthChild(AST::AstNode parent, int i, AST::AstNode child) {
     child = getSynthChild(parent, i)
     or
-    any(Synthesis s).child(parent, i, RealChild(child))
+    any(Synthesis s).child(parent, i, RealChildRef(child))
+    or
+    any(Synthesis s).child(parent, i, SynthChildRef(child))
   }
 
   /**
@@ -527,7 +571,7 @@ private module Cached {
 
 import Cached
 
-TAstNode fromGenerated(Ruby::AstNode n) { n = toGenerated(result) }
+TAstNodeReal fromGenerated(Ruby::AstNode n) { n = toGenerated(result) }
 
 class TCall = TMethodCall or TYieldCall;
 

ruby/ql/lib/codeql/ruby/ast/internal/Synthesis.qll

@@ -46,7 +46,25 @@ newtype SynthKind =
  */
 newtype Child =
   SynthChild(SynthKind k) or
-  RealChild(AstNode n)
+  RealChildRef(TAstNodeReal n) or
+  SynthChildRef(TAstNodeSynth n)
+
+/**
+ * The purpose of this inlined predicate is to split up child references into
+ * those that are from real AST nodes (for which there will be no recursion
+ * through `RealChildRef`), and those that are synthesized recursively
+ * (for which there will be recursion through `SynthChildRef`).
+ *
+ * This performs much better than having a combined `ChildRef` that includes
+ * both real and synthesized AST nodes, since the recursion happening in
+ * `Synthesis::child/3` is non-linear.
+ */
+pragma[inline]
+private Child childRef(TAstNode n) {
+  result = RealChildRef(n)
+  or
+  result = SynthChildRef(n)
+}
 
 private newtype TSynthesis = MkSynthesis()
 
@@ -125,7 +143,7 @@ private predicate assign(
     child = SynthChild(LocalVariableAccessSynthKind(v))
     or
     i = 1 and
-    child = RealChild(value)
+    child = childRef(value)
   )
 }
 
@@ -218,10 +236,10 @@ private module SetterDesugar {
         exists(AstNode call | call = TMethodCallSynth(seq, 0, _, _, _) |
           parent = call and
           i = 0 and
-          child = RealChild(sae.getReceiver())
+          child = childRef(sae.getReceiver())
           or
           parent = call and
-          child = RealChild(sae.getArgument(i - 1))
+          child = childRef(sae.getArgument(i - 1))
           or
           exists(int valueIndex | valueIndex = sae.getNumberOfArguments() + 1 |
             parent = call and
@@ -234,7 +252,7 @@ private module SetterDesugar {
               child = SynthChild(LocalVariableAccessSynthKind(TLocalVariableSynth(sae, 0)))
               or
               i = 1 and
-              child = RealChild(sae.getRightOperand())
+              child = childRef(sae.getRightOperand())
             )
           )
         )
@@ -357,7 +375,7 @@ private module AssignOperationDesugar {
       exists(AstNode assign | assign = TAssignExprSynth(vao, -1) |
         parent = assign and
         i = 0 and
-        child = RealChild(vao.getLeftOperand())
+        child = childRef(vao.getLeftOperand())
         or
         parent = assign and
         i = 1 and
@@ -369,7 +387,7 @@ private module AssignOperationDesugar {
           child = SynthChild(vao.getVariableAccessKind())
           or
           i = 1 and
-          child = RealChild(vao.getRightOperand())
+          child = childRef(vao.getRightOperand())
         )
       )
     )
@@ -477,7 +495,7 @@ private module AssignOperationDesugar {
               or
               parent = op and
               i = 1 and
-              child = RealChild(sao.getRightOperand())
+              child = childRef(sao.getRightOperand())
             )
           )
           or
@@ -648,7 +666,7 @@ private module CompoundAssignDesugar {
           or
           parent = TSplatExprSynth(assign, 1) and
           i = 0 and
-          child = RealChild(tae.getRightOperand())
+          child = childRef(tae.getRightOperand())
         )
         or
         exists(Pattern p, int j, int restIndex |
@@ -662,7 +680,7 @@ private module CompoundAssignDesugar {
           exists(AstNode assign | assign = TAssignExprSynth(seq, j + 1) |
             parent = assign and
             i = 0 and
-            child = RealChild(p)
+            child = childRef(p)
             or
             parent = assign and
             i = 1 and
@@ -767,7 +785,7 @@ private module ArrayLiteralDesugar {
         child = SynthChild(ConstantReadAccessKind("::Array"))
         or
         parent = mc and
-        child = RealChild(al.getElement(i - 1))
+        child = childRef(al.getElement(i - 1))
       )
     )
   }

ruby/ql/lib/codeql/ruby/ast/internal/Variable.qll

@@ -495,7 +495,7 @@ abstract class VariableAccessImpl extends Expr, TVariableAccess {
 }
 
 module LocalVariableAccess {
-  predicate range(Ruby::Identifier id, LocalVariable v) {
+  predicate range(Ruby::Identifier id, TLocalVariableReal v) {
     access(id, v) and
     (
       explicitWriteAccess(id, _)