From c34114e5edbdfbf349581d1cfbb1d3a2899e1aff Mon Sep 17 00:00:00 2001
From: Pavel Avgustinov <pavel@semmle.com>
Date: Fri, 21 Sep 2018 12:21:48 +0100
Subject: [PATCH] Avoid caching `unresolveElement`.

The `resolveClass` part of its definition is not functional, but the
rest is. If we do not cache it but eagerly inline it, there are more
cases where we can use that functionality to generate better code.
---
 cpp/ql/src/definitions.qll                       |  6 ++++--
 cpp/ql/src/semmle/code/cpp/Element.qll           |  4 +++-
 cpp/ql/src/semmle/code/cpp/pointsto/PointsTo.qll | 16 +++++++++++++++-
 3 files changed, 22 insertions(+), 4 deletions(-)

diff --git a/cpp/ql/src/definitions.qll b/cpp/ql/src/definitions.qll
index 9ae818eecb56..b067c22040ce 100644
--- a/cpp/ql/src/definitions.qll
+++ b/cpp/ql/src/definitions.qll
@@ -102,7 +102,8 @@ private predicate constructorCallStartLoc(ConstructorCall cc, File f, int line,
 
 /**
  * Holds if `f`, `line`, `column` indicate the start character
- * of `tm`, which mentions `t`.
+ * of `tm`, which mentions `t`. Type mentions for instantiations
+ * are filtered out.
  */
 private predicate typeMentionStartLoc(TypeMention tm, Type t, File f, int line, int column) {
   exists(Location l |
@@ -111,7 +112,8 @@ private predicate typeMentionStartLoc(TypeMention tm, Type t, File f, int line,
     l.getStartLine() = line and
     l.getStartColumn() = column
   ) and
-  t = tm.getMentionedType()
+  t = tm.getMentionedType() and
+  not t instanceof ClassTemplateInstantiation
 }
 
 /**
diff --git a/cpp/ql/src/semmle/code/cpp/Element.qll b/cpp/ql/src/semmle/code/cpp/Element.qll
index 7231d11aec9d..a418e6c3418b 100644
--- a/cpp/ql/src/semmle/code/cpp/Element.qll
+++ b/cpp/ql/src/semmle/code/cpp/Element.qll
@@ -8,7 +8,8 @@ private import semmle.code.cpp.internal.ResolveClass
  * For example, for an incomplete struct `e` the result may be a
  * complete struct with the same name.
  */
-private cached @element resolveElement(@element e) {
+pragma[inline]
+private @element resolveElement(@element e) {
   if isClass(e)
   then result = resolveClass(e)
   else result = e
@@ -31,6 +32,7 @@ Element mkElement(@element e) {
  * extensional.
  * See `underlyingElement` for when `e` is `this`.
  */
+pragma[inline]
 @element unresolveElement(Element e) {
   resolveElement(result) = e
 }
diff --git a/cpp/ql/src/semmle/code/cpp/pointsto/PointsTo.qll b/cpp/ql/src/semmle/code/cpp/pointsto/PointsTo.qll
index bd4684cdfa42..6f1ab3758262 100644
--- a/cpp/ql/src/semmle/code/cpp/pointsto/PointsTo.qll
+++ b/cpp/ql/src/semmle/code/cpp/pointsto/PointsTo.qll
@@ -633,12 +633,26 @@ class PointsToExpr extends Expr
   pragma[noopt]
   Element pointsTo()
   {
-    this.interesting() and exists(int set, @element thisEntity, @element resultEntity | thisEntity = underlyingElement(this) and pointstosets(set, thisEntity) and setlocations(set, resultEntity) and resultEntity = unresolveElement(result))
+    this.interesting() and
+    exists(int set, @element thisEntity, @element resultEntity |
+      thisEntity = underlyingElement(this) and
+      pointstosets(set, thisEntity) and
+      setlocations(set, resultEntity) and
+      resultEntity = localUnresolveElement(result)
+    )
   }
 
   float confidence() { result = 1.0 / count(this.pointsTo()) }
 }
 
+/*
+ * This is used above in a `pragma[noopt]` context, which prevents its
+ * customary inlining. We materialise it explicitly here.
+ */
+private @element localUnresolveElement(Element e) {
+  result = unresolveElement(e)
+}
+
 /**
  * Holds if anything points to an element, that is, is equivalent to:
  * ```