Merge pull request #232 from jbj/resolveElement-member

C++: make `unresolve` a member of ElementBase

Author: adityasharad

cpp/ql/src/semmle/code/cpp/Class.qll

@@ -12,7 +12,11 @@ private import semmle.code.cpp.internal.ResolveClass
  */
 class Class extends UserType {
   Class() {
-    isClass(underlyingElement(this))
+    isClass(this.underlying())
+  }
+
+  override @element unresolve() {
+    resolveClass(result) = this
   }
 
   /** Gets a child declaration of this class. */

cpp/ql/src/semmle/code/cpp/Element.qll

@@ -2,63 +2,75 @@ import semmle.code.cpp.Location
 private import semmle.code.cpp.Enclosing
 private import semmle.code.cpp.internal.ResolveClass
 
-/**
- * Get the `@element` that represents this `@element`.
- * Normally this will simply be `e`, but sometimes it is not.
- * For example, for an incomplete struct `e` the result may be a
- * complete struct with the same name.
- */
-private cached @element resolveElement(@element e) {
-  if isClass(e)
-  then result = resolveClass(e)
-  else result = e
-}
-
 /**
  * Get the `Element` that represents this `@element`.
  * Normally this will simply be a cast of `e`, but sometimes it is not.
  * For example, for an incomplete struct `e` the result may be a
  * complete struct with the same name.
  */
+pragma[inline]
 Element mkElement(@element e) {
-  result = resolveElement(e)
+  result.unresolve() = e
 }
 
 /**
- * Get an `@element` that resolves to the `Element`. This should
+ * INTERNAL: Do not use.
+ *
+ * Gets an `@element` that resolves to the `Element`. This should
  * normally only be called from member predicates, where `e` is not
  * `this` and you need the result for an argument to a database
  * extensional.
  * See `underlyingElement` for when `e` is `this`.
  */
+pragma[inline]
 @element unresolveElement(Element e) {
-  resolveElement(result) = e
+  result = e.unresolve()
 }
 
 /**
- * Get the `@element` that this `Element` extends. This should normally
+ * INTERNAL: Do not use.
+ *
+ * Gets the `@element` that this `Element` extends. This should normally
  * only be called from member predicates, where `e` is `this` and you
  * need the result for an argument to a database extensional.
  * See `unresolveElement` for when `e` is not `this`.
  */
+pragma[inline]
 @element underlyingElement(Element e) {
   result = e
 }
 
 /**
- * A C/C++ element with no member predicates other than `toString`. Not for
+ * A C/C++ element with a minimal set of member predicates. Not for
  * general use. This class does not define a location, so classes wanting to
  * change their location without affecting other classes can extend
  * `ElementBase` instead of `Element` to create a new rootdef for `getURL`,
  * `getLocation`, or `hasLocationInfo`.
  */
 class ElementBase extends @element {
-  ElementBase() {
-    this = resolveElement(_)
-  }
-
   /** Gets a textual representation of this element. */
   string toString() { none() }
+
+  /**
+   * INTERNAL: Do not use.
+   *
+   * Gets the `@element` that this `Element` extends. This should normally only
+   * be called from member predicates on `this` where you need the result for
+   * an argument to a database extensional.
+   * See `unresolve` for when the qualifier is not `this`.
+   */
+  final @element underlying() { result = this }
+
+  /**
+   * INTERNAL: Do not use.
+   *
+   * Gets an `@element` that resolves to the `Element`. This should normally
+   * only be called from member predicates, where the qualifier is not `this`
+   * and you need the result for an argument to a database extensional.
+   * See `underlying` for when the qualifier is `this`.
+   */
+  pragma[inline]
+  @element unresolve() { result = this }
 }
 
 /**

cpp/ql/src/semmle/code/cpp/Specifier.qll

@@ -294,13 +294,13 @@ class AttributeArgument extends Element, @attribute_arg {
   }
 
   override string toString() {
-    if exists (@attribute_arg_empty self | mkElement(self) = this)
+    if exists (@attribute_arg_empty self | self = underlyingElement(this))
       then result = "empty argument"
       else exists (string prefix, string tail
            | (if exists(getName())
                 then prefix = getName() + "="
                 else prefix = "") and
-             (if exists (@attribute_arg_type self | mkElement(self) = this)
+             (if exists (@attribute_arg_type self | self = underlyingElement(this))
                 then tail = getValueType().getName()
                 else tail = getValueText()) and
              result = prefix + tail)

cpp/ql/src/semmle/code/cpp/Type.qll

@@ -7,6 +7,8 @@ private import semmle.code.cpp.internal.ResolveClass
  * A C/C++ type.
  */
 class Type extends Locatable, @type {
+  Type() { isType(this.underlying()) }
+
   /**
    * Gets the name of this type.
    */

cpp/ql/src/semmle/code/cpp/internal/ResolveClass.qll

@@ -54,4 +54,10 @@ cached private module Cached {
     (usertypes(t,_,1) or usertypes(t,_,2) or usertypes(t,_,3) or usertypes(t,_,6)
     or usertypes(t,_,10) or usertypes(t,_,11) or usertypes(t,_,12))
   }
+
+  cached predicate isType(@type t) {
+    not isClass(t)
+    or
+    t = resolveClass(_)
+  }
 }

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 = this.underlying() 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:
  * ```

cpp/ql/src/semmle/code/cpp/security/Overflow.qll

@@ -13,6 +13,7 @@ predicate guardedAbs(Operation e, Expr use) {
 }
 
 /** is the size of this use guarded to be less than something? */
+pragma[nomagic]
 predicate guardedLesser(Operation e, Expr use) {
   exists(IfStmt c, RelationalOperation guard |
     use = guard.getLesserOperand().getAChild*() and
@@ -33,6 +34,7 @@ predicate guardedLesser(Operation e, Expr use) {
 }
 
 /** is the size of this use guarded to be greater than something? */
+pragma[nomagic]
 predicate guardedGreater(Operation e, Expr use) {
   exists(IfStmt c, RelationalOperation guard |
     use = guard.getGreaterOperand().getAChild*() and