Merge pull request github#3831 from MathiasVP/remove-abstract-decl-var-call

C++: Remove abstractness from DeclarationEntry, AccessHolder and Call

Author: jbj

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

@@ -124,7 +124,7 @@ class Declaration extends Locatable, @declaration {
    * To test whether this declaration has a particular name in the global
    * namespace, use `hasGlobalName`.
    */
-  abstract string getName();
+  string getName() { none() } // overridden in subclasses
 
   /** Holds if this declaration has the given name. */
   predicate hasName(string name) { name = this.getName() }
@@ -140,7 +140,7 @@ class Declaration extends Locatable, @declaration {
   }
 
   /** Gets a specifier of this declaration. */
-  abstract Specifier getASpecifier();
+  Specifier getASpecifier() { none() } // overridden in subclasses
 
   /** Holds if this declaration has a specifier with the given name. */
   predicate hasSpecifier(string name) { this.getASpecifier().hasName(name) }
@@ -156,7 +156,7 @@ class Declaration extends Locatable, @declaration {
    * Gets the location of a declaration entry corresponding to this
    * declaration.
    */
-  abstract Location getADeclarationLocation();
+  Location getADeclarationLocation() { none() } // overridden in subclasses
 
   /**
    * Gets the declaration entry corresponding to this declaration that is a
@@ -165,7 +165,7 @@ class Declaration extends Locatable, @declaration {
   DeclarationEntry getDefinition() { none() }
 
   /** Gets the location of the definition, if any. */
-  abstract Location getDefinitionLocation();
+  Location getDefinitionLocation() { none() } // overridden in subclasses
 
   /** Holds if the declaration has a definition. */
   predicate hasDefinition() { exists(this.getDefinition()) }
@@ -289,6 +289,8 @@ class Declaration extends Locatable, @declaration {
   }
 }
 
+private class TDeclarationEntry = @var_decl or @type_decl or @fun_decl;
+
 /**
  * A C/C++ declaration entry. For example the following code contains five
  * declaration entries:
@@ -304,9 +306,9 @@ class Declaration extends Locatable, @declaration {
  * See the comment above `Declaration` for an explanation of the relationship
  * between `Declaration` and `DeclarationEntry`.
  */
-abstract class DeclarationEntry extends Locatable {
+class DeclarationEntry extends Locatable, TDeclarationEntry {
   /** Gets a specifier associated with this declaration entry. */
-  abstract string getASpecifier();
+  string getASpecifier() { none() } // overridden in subclasses
 
   /**
    * Gets the name associated with the corresponding definition (where
@@ -329,10 +331,10 @@ abstract class DeclarationEntry extends Locatable {
    *  `I.getADeclarationEntry()` returns `D`
    *  but `D.getDeclaration()` only returns `C`
    */
-  abstract Declaration getDeclaration();
+  Declaration getDeclaration() { none() } // overridden in subclasses
 
   /** Gets the name associated with this declaration entry, if any. */
-  abstract string getName();
+  string getName() { none() } // overridden in subclasses
 
   /**
    * Gets the type associated with this declaration entry.
@@ -341,7 +343,7 @@ abstract class DeclarationEntry extends Locatable {
    * For function declarations, get the return type of the function.
    * For type declarations, get the type being declared.
    */
-  abstract Type getType();
+  Type getType() { none() } // overridden in subclasses
 
   /**
    * Gets the type associated with this declaration entry after specifiers
@@ -359,7 +361,7 @@ abstract class DeclarationEntry extends Locatable {
   predicate hasSpecifier(string specifier) { getASpecifier() = specifier }
 
   /** Holds if this declaration entry is a definition. */
-  abstract predicate isDefinition();
+  predicate isDefinition() { none() } // overridden in subclasses
 
   override string toString() {
     if isDefinition()
@@ -371,6 +373,8 @@ abstract class DeclarationEntry extends Locatable {
   }
 }
 
+private class TAccessHolder = @function or @usertype;
+
 /**
  * A declaration that can potentially have more C++ access rights than its
  * enclosing element. This comprises `Class` (they have access to their own
@@ -392,7 +396,7 @@ abstract class DeclarationEntry extends Locatable {
  * the informal phrase "_R_ occurs in a member or friend of class C", where
  * `AccessHolder` corresponds to this _R_.
  */
-abstract class AccessHolder extends Declaration {
+class AccessHolder extends Declaration, TAccessHolder {
   /**
    * Holds if `this` can access private members of class `c`.
    *
@@ -410,7 +414,7 @@ abstract class AccessHolder extends Declaration {
   /**
    * Gets the nearest enclosing `AccessHolder`.
    */
-  abstract AccessHolder getEnclosingAccessHolder();
+  AccessHolder getEnclosingAccessHolder() { none() } // overridden in subclasses
 
   /**
    * Holds if a base class `base` of `derived` _is accessible at_ `this` (N4140

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

@@ -325,7 +325,7 @@ class ParameterDeclarationEntry extends VariableDeclarationEntry {
  */
 class LocalScopeVariable extends Variable, @localscopevariable {
   /** Gets the function to which this variable belongs. */
-  /*abstract*/ Function getFunction() { none() }
+  Function getFunction() { none() } // overridden in subclasses
 }
 
 /**

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

@@ -2,12 +2,22 @@ import semmle.code.cpp.exprs.Expr
 import semmle.code.cpp.Function
 private import semmle.code.cpp.dataflow.EscapesTree
 
+private class TCall = @funbindexpr or @callexpr;
+
 /**
  * A C/C++ call.
- *
- * This is the abstract root QL class for all types of calls.
  */
-abstract class Call extends Expr, NameQualifiableElement {
+class Call extends Expr, NameQualifiableElement, TCall {
+  // `@funbindexpr` (which is the dbscheme type for FunctionCall) is a union type that includes
+  // `@routineexpr. This dbscheme type includes accesses to functions that are not necessarily calls to
+  // that function. That's why the charpred for `FunctionCall` requires:
+  // ```
+  // iscall(underlyingElement(this), _)
+  // ```
+  // So for the charpred for `Call` we include the requirement that if this is an instance of
+  // `@funbindexpr` it must be a _call_ to the function.
+  Call() { this instanceof @callexpr or iscall(underlyingElement(this), _) }
+
   /**
    * Gets the number of arguments (actual parameters) of this call. The count
    * does _not_ include the qualifier of the call, if any.
@@ -74,7 +84,7 @@ abstract class Call extends Expr, NameQualifiableElement {
    *   method, and it might not exist.
    * - For a variable call, it never exists.
    */
-  abstract Function getTarget();
+  Function getTarget() { none() } // overridden in subclasses
 
   override int getPrecedence() { result = 17 }