Rule 8.7: Improve performance

This rule was identified as containing one of the slowest predicates in
our C Coding Standards query suites, and this commit improves the
performance.

Performance is improved with the following changes:
 (a) Factoring out a predicate for the repeated calls to getTarget(), to
     avoid duplication and to create a semantically meaningful predicate
     which gets the target for a reference to an external identifier.
 (b) Use the factored out predicate to refine the reference class to
     only be references to external identifiers. This reduces the size
     of the class.
 (c) Create a predicate for computing a table of external identifiers,
     references to those identifiers and the translation units those
     exist in. We can then compute this table once, and use it in both
     the "find me a reference to this external identifier" case and in
     the "where the external identifier is not referenced in any other
     translation unit" case.

Part (c) is the critical change. Without that, the optimizer was
creating an expensive join order in the negation case, where it
was effectively creating a cross product of all references to each
external identifier, before later excluding on the negation. The use
of our predicate in the negation case means we can first create a table
of external identifiers and translation units, then apply that in the
negation case without cross producting the references.

Author: lcartey

c/misra/src/rules/RULE-8-7/ShouldNotBeDefinedWithExternalLinkage.ql

@@ -20,27 +20,36 @@ import codingstandards.c.misra
 import codingstandards.cpp.Identifiers
 import codingstandards.cpp.Scope
 
+ExternalIdentifiers getExternalIdentifierTarget(NameQualifiableElement nqe) {
+  result = nqe.(Access).getTarget()
+  or
+  result = nqe.(FunctionCall).getTarget()
+}
+
 /**
- * Re-introduce function calls into access description as
- * "any reference"
+ * A reference to an external identifier, either as an `Access` or a `FunctionCall`.
  */
-class Reference extends NameQualifiableElement {
-  Reference() {
-    this instanceof Access or
-    this instanceof FunctionCall
-  }
+class ExternalIdentifierReference extends NameQualifiableElement {
+  ExternalIdentifierReference() { exists(getExternalIdentifierTarget(this)) }
+
+  ExternalIdentifiers getExternalIdentifierTarget() { result = getExternalIdentifierTarget(this) }
+}
+
+predicate isReferencedInTranslationUnit(
+  ExternalIdentifiers e, ExternalIdentifierReference r, TranslationUnit t
+) {
+  r.getExternalIdentifierTarget() = e and
+  r.getFile() = t
 }
 
-from ExternalIdentifiers e, Reference a1, TranslationUnit t1
+from ExternalIdentifiers e, ExternalIdentifierReference a1, TranslationUnit t1
 where
   not isExcluded(e, Declarations6Package::shouldNotBeDefinedWithExternalLinkageQuery()) and
-  (a1.(Access).getTarget() = e or a1.(FunctionCall).getTarget() = e) and
-  a1.getFile() = t1 and
-  //not accessed in any other translation unit
-  not exists(TranslationUnit t2, Reference a2 |
-    not t1 = t2 and
-    (a2.(Access).getTarget() = e or a2.(FunctionCall).getTarget() = e) and
-    a2.getFile() = t2
+  isReferencedInTranslationUnit(e, a1, t1) and
+  // Not referenced in any other translation unit
+  not exists(TranslationUnit t2 |
+    isReferencedInTranslationUnit(e, _, t2) and
+    not t1 = t2
   )
 select e, "Declaration with external linkage is accessed in only one translation unit $@.", a1,
   a1.toString()