Merge pull request #3387 from geoffw0/tostringperf

C++: Eliminate recursion from toString().

Author: jbj

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

@@ -98,7 +98,12 @@ class Declaration extends Locatable, @declaration {
     this.hasQualifiedName(namespaceQualifier, "", baseName)
   }
 
-  override string toString() { result = this.getName() }
+  /**
+   * Gets a description of this `Declaration` for display purposes.
+   */
+  string getDescription() { result = this.getName() }
+
+  final override string toString() { result = this.getDescription() }
 
   /**
    * Gets the name of this declaration.

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

@@ -79,7 +79,10 @@ class Namespace extends NameQualifyingElement, @namespace {
   /** Gets the metric namespace. */
   MetricNamespace getMetrics() { result = this }
 
-  override string toString() { result = this.getQualifiedName() }
+  /** Gets a version of the `QualifiedName` that is more suitable for display purposes. */
+  string getFriendlyName() { result = this.getQualifiedName() }
+
+  final override string toString() { result = getFriendlyName() }
 
   /** Gets a declaration of (part of) this namespace. */
   NamespaceDeclarationEntry getADeclarationEntry() { result.getNamespace() = this }
@@ -104,7 +107,7 @@ class NamespaceDeclarationEntry extends Locatable, @namespace_decl {
     namespace_decls(underlyingElement(this), unresolveElement(result), _, _)
   }
 
-  override string toString() { result = this.getNamespace().toString() }
+  override string toString() { result = this.getNamespace().getFriendlyName() }
 
   /**
    * Gets the location of the token preceding the namespace declaration
@@ -150,7 +153,7 @@ class UsingDeclarationEntry extends UsingEntry {
    */
   Declaration getDeclaration() { usings(underlyingElement(this), unresolveElement(result), _) }
 
-  override string toString() { result = "using " + this.getDeclaration().toString() }
+  override string toString() { result = "using " + this.getDeclaration().getDescription() }
 }
 
 /**
@@ -169,7 +172,7 @@ class UsingDirectiveEntry extends UsingEntry {
    */
   Namespace getNamespace() { usings(underlyingElement(this), unresolveElement(result), _) }
 
-  override string toString() { result = "using namespace " + this.getNamespace().toString() }
+  override string toString() { result = "using namespace " + this.getNamespace().getFriendlyName() }
 }
 
 /**
@@ -204,7 +207,7 @@ class GlobalNamespace extends Namespace {
    */
   deprecated string getFullName() { result = this.getName() }
 
-  override string toString() { result = "(global namespace)" }
+  override string getFriendlyName() { result = "(global namespace)" }
 }
 
 /**

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

@@ -260,24 +260,33 @@ class ParameterDeclarationEntry extends VariableDeclarationEntry {
    */
   int getIndex() { param_decl_bind(underlyingElement(this), result, _) }
 
+  private string getAnonymousParameterDescription() {
+    not exists(getName()) and
+    exists(string idx |
+      idx =
+        ((getIndex() + 1).toString() + "th")
+            .replaceAll("1th", "1st")
+            .replaceAll("2th", "2nd")
+            .replaceAll("3th", "3rd")
+            .replaceAll("11st", "11th")
+            .replaceAll("12nd", "12th")
+            .replaceAll("13rd", "13th") and
+      if exists(getCanonicalName())
+      then result = "declaration of " + getCanonicalName() + " as anonymous " + idx + " parameter"
+      else result = "declaration of " + idx + " parameter"
+    )
+  }
+
   override string toString() {
-    if exists(getName())
-    then result = super.toString()
-    else
-      exists(string idx |
-        idx =
-          ((getIndex() + 1).toString() + "th")
-              .replaceAll("1th", "1st")
-              .replaceAll("2th", "2nd")
-              .replaceAll("3th", "3rd")
-              .replaceAll("11st", "11th")
-              .replaceAll("12nd", "12th")
-              .replaceAll("13rd", "13th")
-      |
-        if exists(getCanonicalName())
-        then result = "declaration of " + getCanonicalName() + " as anonymous " + idx + " parameter"
-        else result = "declaration of " + idx + " parameter"
-      )
+    isDefinition() and
+    result = "definition of " + getName()
+    or
+    not isDefinition() and
+    if getName() = getCanonicalName()
+    then result = "declaration of " + getName()
+    else result = "declaration of " + getCanonicalName() + " as " + getName()
+    or
+    result = getAnonymousParameterDescription()
   }
 
   /**

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

@@ -116,7 +116,7 @@ class XMLFile extends XMLParent, File {
   XMLFile() { xmlEncoding(this, _) }
 
   /** Gets a printable representation of this XML file. */
-  override string toString() { result = XMLParent.super.toString() }
+  override string toString() { result = getName() }
 
   /** Gets the name of this XML file. */
   override string getName() { result = File.super.getAbsolutePath() }
@@ -236,7 +236,7 @@ class XMLElement extends @xmlelement, XMLParent, XMLLocatable {
   string getAttributeValue(string name) { result = this.getAttribute(name).getValue() }
 
   /** Gets a printable representation of this XML element. */
-  override string toString() { result = XMLParent.super.toString() }
+  override string toString() { result = getName() }
 }
 
 /**

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

@@ -86,7 +86,7 @@ class Closure extends Class {
     result.getName() = "operator()"
   }
 
-  override string toString() { result = "decltype([...](...){...})" }
+  override string getDescription() { result = "decltype([...](...){...})" }
 }
 
 /**
@@ -99,7 +99,7 @@ class Closure extends Class {
  * ```
  */
 class LambdaCapture extends Locatable, @lambdacapture {
-  override string toString() { result = getField().toString() }
+  override string toString() { result = getField().getName() }
 
   override string getCanonicalQLClass() { result = "LambdaCapture" }
 

cpp/ql/test/library-tests/templates/isfromtemplateinstantiation/isfromtemplateinstantiation.ql

@@ -2,7 +2,7 @@ import cpp
 
 class FunctionMonkeyPatch extends Function {
   language[monotonicAggregates]
-  override string toString() {
+  override string getDescription() {
     exists(string name, string templateArgs, string args |
       result = name + templateArgs + args and
       name = this.getQualifiedName() and
@@ -30,7 +30,9 @@ class FunctionMonkeyPatch extends Function {
 }
 
 class ParameterMonkeyPatch extends Parameter {
-  override string toString() { result = super.getType().getName() + " " + super.toString() }
+  override string getDescription() {
+    result = super.getType().getName() + " " + super.getDescription()
+  }
 }
 
 from Element e, Element ti

cpp/ql/test/library-tests/usings/Usings1.expected

@@ -1,7 +1,7 @@
-| templates.cpp:9:5:9:14 | using c |
-| usings.cpp:8:1:8:11 | using nf |
-| usings.cpp:9:1:9:17 | using namespace N |
-| usings.cpp:18:3:18:13 | using bf |
-| usings.cpp:21:5:21:14 | using gf |
-| usings.cpp:34:3:34:20 | using tbf |
-| usings.cpp:42:5:42:22 | using foo |
+| templates.cpp:9:5:9:14 | using c | UsingDeclarationEntry, enclosingElement:std |
+| usings.cpp:8:1:8:11 | using nf | UsingDeclarationEntry, enclosingElement:(global namespace) |
+| usings.cpp:9:1:9:17 | using namespace N | UsingDirectiveEntry, enclosingElement:(global namespace) |
+| usings.cpp:18:3:18:13 | using bf | UsingDeclarationEntry, enclosingElement:D |
+| usings.cpp:21:5:21:14 | using gf | UsingDeclarationEntry, enclosingElement:{ ... } |
+| usings.cpp:34:3:34:20 | using tbf | UsingDeclarationEntry, enclosingElement:TD |
+| usings.cpp:42:5:42:22 | using foo | UsingDeclarationEntry, enclosingElement:nsbar |

cpp/ql/test/library-tests/usings/Usings1.ql

@@ -1,4 +1,14 @@
 import cpp
 
+string describe(UsingEntry ue) {
+  ue instanceof UsingDeclarationEntry and
+  result = "UsingDeclarationEntry"
+  or
+  ue instanceof UsingDirectiveEntry and
+  result = "UsingDirectiveEntry"
+  or
+  result = "enclosingElement:" + ue.getEnclosingElement().toString()
+}
+
 from UsingEntry ue
-select ue
+select ue, concat(describe(ue), ", ")