Merge pull request github#3204 from jbj/Expr-location-workaround

C++: Move Expr location workaround to Expr.qll

Author: MathiasVP

cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowUtil.qll

@@ -116,33 +116,12 @@ class ExprNode extends Node, TExprNode {
 
   override string toString() { result = expr.toString() }
 
-  override Location getLocation() {
-    result = getExprLocationOverride(expr)
-    or
-    not exists(getExprLocationOverride(expr)) and
-    result = expr.getLocation()
-  }
+  override Location getLocation() { result = expr.getLocation() }
 
   /** Gets the expression corresponding to this node. */
   Expr getExpr() { result = expr }
 }
 
-/**
- * Gets a location for `e` that's more accurate than `e.getLocation()`, if any.
- */
-private Location getExprLocationOverride(Expr e) {
-  // Base case: the parent has a better location than `e`.
-  e.getLocation() instanceof UnknownExprLocation and
-  result = e.getParent().getLocation() and
-  not result instanceof UnknownLocation
-  or
-  // Recursive case: the parent has a location override that's better than what
-  // `e` has.
-  e.getLocation() instanceof UnknownExprLocation and
-  result = getExprLocationOverride(e.getParent()) and
-  not result instanceof UnknownLocation
-}
-
 abstract class ParameterNode extends Node, TNode {
   /**
    * Holds if this node is the parameter of `c` at the specified (zero-based)

cpp/ql/src/semmle/code/cpp/exprs/Expr.qll

@@ -53,7 +53,32 @@ class Expr extends StmtParent, @expr {
   Element getParent() { exprparents(underlyingElement(this), _, unresolveElement(result)) }
 
   /** Gets the location of this expression. */
-  override Location getLocation() { exprs(underlyingElement(this), _, result) }
+  override Location getLocation() {
+    result = this.getExprLocationOverride()
+    or
+    not exists(this.getExprLocationOverride()) and
+    result = this.getDbLocation()
+  }
+
+  /**
+   * Gets a location for this expression that's more accurate than
+   * `getDbLocation()`, if any.
+   */
+  private Location getExprLocationOverride() {
+    // Base case: the parent has a better location than `this`.
+    this.getDbLocation() instanceof UnknownExprLocation and
+    result = this.getParent().(Expr).getDbLocation() and
+    not result instanceof UnknownLocation
+    or
+    // Recursive case: the parent has a location override that's better than
+    // what `this` has.
+    this.getDbLocation() instanceof UnknownExprLocation and
+    result = this.getParent().(Expr).getExprLocationOverride() and
+    not result instanceof UnknownLocation
+  }
+
+  /** Gets the location of this expressions, raw from the database. */
+  private Location getDbLocation() { exprs(underlyingElement(this), _, result) }
 
   /** Holds if this is an auxiliary expression generated by the compiler. */
   predicate isCompilerGenerated() {

cpp/ql/test/examples/expressions/PrintAST.expected

@@ -430,28 +430,28 @@ DynamicCast.cpp:
 #-----|               Conversion = [BaseClassConversion] base class conversion
 #-----|               Type = [PointerType] Base *
 #-----|               ValueCategory = prvalue
-#-----|             expr: [ThisExpr] this
-#-----|                 Type = [PointerType] Derived *
-#-----|                 ValueCategory = prvalue(load)
+#    4|             expr: [ThisExpr] this
+#    4|                 Type = [PointerType] Derived *
+#    4|                 ValueCategory = prvalue(load)
 #-----|           0: [ReferenceToExpr] (reference to)
 #-----|               Type = [LValueReferenceType] const Base &
 #-----|               ValueCategory = prvalue
-#-----|             expr: [PointerDereferenceExpr] * ...
-#-----|                 Type = [SpecifiedType] const Base
-#-----|                 ValueCategory = lvalue
+#    4|             expr: [PointerDereferenceExpr] * ...
+#    4|                 Type = [SpecifiedType] const Base
+#    4|                 ValueCategory = lvalue
 #-----|               0: [CStyleCast] (const Base *)...
 #-----|                   Conversion = [BaseClassConversion] base class conversion
 #-----|                   Type = [PointerType] const Base *
 #-----|                   ValueCategory = prvalue
-#-----|                 expr: [AddressOfExpr] & ...
-#-----|                     Type = [PointerType] const Derived *
-#-----|                     ValueCategory = prvalue
+#    4|                 expr: [AddressOfExpr] & ...
+#    4|                     Type = [PointerType] const Derived *
+#    4|                     ValueCategory = prvalue
 #-----|                   0: [ReferenceDereferenceExpr] (reference dereference)
 #-----|                       Type = [SpecifiedType] const Derived
 #-----|                       ValueCategory = lvalue
-#-----|                     expr: [VariableAccess] p#0
-#-----|                         Type = [LValueReferenceType] const Derived &
-#-----|                         ValueCategory = prvalue(load)
+#    4|                     expr: [VariableAccess] p#0
+#    4|                         Type = [LValueReferenceType] const Derived &
+#    4|                         ValueCategory = prvalue(load)
 #-----|     1: [ReturnStmt] return ...
 #-----|       0: [ReferenceToExpr] (reference to)
 #-----|           Type = [LValueReferenceType] Derived &
@@ -1248,9 +1248,9 @@ union_etc.cpp:
 #    6|         0: [PointerFieldAccess] x
 #    6|             Type = [IntType] int
 #    6|             ValueCategory = lvalue
-#-----|           -1: [ThisExpr] this
-#-----|               Type = [PointerType] S *
-#-----|               ValueCategory = prvalue(load)
+#    6|           -1: [ThisExpr] this
+#    6|               Type = [PointerType] S *
+#    6|               ValueCategory = prvalue(load)
 #    6|         1: [VariableAccess] val
 #    6|             Type = [IntType] int
 #    6|             ValueCategory = prvalue(load)
@@ -1431,9 +1431,9 @@ union_etc.cpp:
 #   33|         0: [PointerFieldAccess] q
 #   33|             Type = [IntType] int
 #   33|             ValueCategory = lvalue
-#-----|           -1: [ThisExpr] this
-#-----|               Type = [PointerType] T *
-#-----|               ValueCategory = prvalue(load)
+#   33|           -1: [ThisExpr] this
+#   33|               Type = [PointerType] T *
+#   33|               ValueCategory = prvalue(load)
 #   33|         1: [VariableAccess] val
 #   33|             Type = [IntType] int
 #   33|             ValueCategory = prvalue(load)

cpp/ql/test/library-tests/blocks/cpp/exprs.expected

@@ -79,4 +79,4 @@
 | blocks.cpp:57:12:57:32 | call to expression |
 | blocks.cpp:57:14:57:30 | ^ { ... } |
 | blocks.cpp:57:23:57:26 | four |
-| file://:0:0:0:0 | this |
+| blocks.cpp:57:23:57:26 | this |

cpp/ql/test/library-tests/builtins/edg/expr.expected

@@ -1,5 +1,8 @@
 | edg.c:12:14:12:51 | (int)... | 0 | 0 |
 | edg.c:12:14:12:51 | __builtin_offsetof | 1 | 1 |
+| edg.c:12:14:12:51 | mystruct | 0 | 0 |
+| edg.c:12:49:12:50 | 0 | 0 | 0 |
+| edg.c:12:49:12:50 | * ... | 0 | 0 |
 | edg.c:12:49:12:50 | f2 | 0 | 0 |
 | edg.c:13:14:13:45 | 0 | 0 | 0 |
 | edg.c:13:14:13:45 | & ... | 0 | 0 |
@@ -10,6 +13,3 @@
 | edg.c:13:14:13:45 | (size_t)... | 0 | 0 |
 | edg.c:13:14:13:45 | __INTADDR__ | 1 | 1 |
 | edg.c:13:43:13:44 | f2 | 0 | 0 |
-| file://:0:0:0:0 | 0 | 0 | 0 |
-| file://:0:0:0:0 | * ... | 0 | 0 |
-| file://:0:0:0:0 | mystruct | 0 | 0 |