Fix RULE-0-2-4/A0-1-3 false positives for private functions

Two categories of false positive are addressed:

1. **Private pure virtual functions used via the NVI idiom** —
   `DynamicCallGraph::getTarget()` resolves a virtual call to the
   implementations that may actually *run*. A pure virtual function
   (`= 0`) has no body, so it is never a viable dispatch target and is
   therefore never returned by `getTarget()` — not even when it is
   unambiguously named by a call, as in the non-virtual interface (NVI)
   idiom where a public member calls a private pure virtual. It was
   consequently reported as unused even though a sibling member calls
   it. `functionIsCalled` now also counts the *statically named* callee
   (`FunctionCall.getTarget()`), which recovers exactly that case.

   This is deliberately narrower than excluding `PureVirtualFunction`
   from `LocalFunction` outright: a private pure virtual that is
   genuinely never called and never overridden is still reported.

2. **Private members of never-instantiated class templates** — when a
   class template is never instantiated with a concrete type anywhere
   in the analyzed compilation units, Clang never elaborates a body for
   its member functions, so `Call`/`FunctionCall` targets within that
   pattern's own text cannot be resolved by
   `DynamicCallGraph::getTarget()` or `VirtualDispatch`, even for calls
   between sibling members of the very same class (e.g. a public entry
   point calling a private helper). This is common for generic
   "plumbing" library code (CRTP-style wrappers and the like) that is
   only ever instantiated by downstream consumers outside the analyzed
   codebase. The new `hasNoVisibleInstantiation(fn)` predicate
   conservatively treats such private members as "used" (out of scope
   for this analysis) rather than reporting them as dead code — but
   only when *no* sibling member of the same class has any
   instantiation either, so genuinely dead private helpers in class
   templates that *are* instantiated elsewhere are still correctly
   reported.

The sibling-instantiation check is factored into a `pragma[noinline]`
predicate over the declaring `Class` rather than over the member
`Function`. Inlined into the caller, the join orderer loses the fact
that the class is functionally determined and materialises the full
(member, sibling) cross product per class before projecting it away.
On a large real-world database this cut the peak intermediate relation
for that predicate from 4,831,818 tuples to 7,700 with identical
results.

Fixes #1168

Author: castler

change_notes/2026-08-03-unused-local-function-false-positives.md

@@ -0,0 +1,16 @@
+- `A0-1-3`, `RULE-0-2-4` - `UnusedLocalFunction.ql`:
+  - Fixed false positives for private pure virtual functions used through the non-virtual
+    interface (NVI) idiom. `DynamicCallGraph::getTarget()` resolves a virtual call to the
+    implementations that may actually run; a pure virtual function has no body, so it is
+    never a viable dispatch target and was reported as unused even when a sibling member
+    called it. A call is now also counted when the function is the statically named
+    callee. Pure virtual functions that are genuinely never called and never overridden
+    are still reported.
+  - Excluded private member functions of class templates that are never concretely
+    instantiated anywhere in the database (and where no sibling member of the same
+    class-template pattern is instantiated either). Clang never elaborates a body for the
+    members of such patterns, so calls between sibling members of the same
+    never-instantiated class (e.g. a public entry point calling a private helper) cannot be
+    resolved by the call graph. This is common for generic "plumbing" library code
+    (CRTP-style wrappers, etc.) that is only ever
+    instantiated by downstream consumers outside of the analyzed codebase.

cpp/common/src/codingstandards/cpp/rules/unusedlocalfunction/UnusedLocalFunction.qll

@@ -20,11 +20,84 @@ predicate overloadedFunctionIsCalled(Function unusedFunction) {
   exists(Function f | f = unusedFunction.getAnOverload() and f = getTarget(_))
 }
 
+/**
+ * Holds if `fn` is the target of some call, either statically or according to the
+ * dynamic call graph.
+ *
+ * `DynamicCallGraph::getTarget()` resolves a virtual call to the functions that may
+ * actually run, i.e. the overriding implementations. A pure virtual function has no
+ * body, so it is never a viable dispatch target and is therefore *never* returned by
+ * `getTarget()` -- even when it is unambiguously named by a call, as in the
+ * non-virtual interface (NVI) idiom where a public member calls a private pure
+ * virtual. The additional static `FunctionCall.getTarget()` disjunct recovers exactly
+ * that case: the callee as written in the source.
+ */
+predicate functionIsCalled(Function fn) {
+  fn = getTarget(_)
+  or
+  // The statically named callee, which the dynamic call graph drops for calls that
+  // dispatch to an override (notably pure virtual functions, which have no body).
+  exists(FunctionCall fc | fc.getTarget() = fn)
+}
+
 /** Checks if a Function's address was taken. */
 predicate addressBeenTaken(Function unusedFunction) {
   exists(FunctionAccess fa | fa.getTarget() = unusedFunction)
 }
 
+/**
+ * Holds if some member of the class `c` has at least one concrete instantiation anywhere in the
+ * database.
+ *
+ * If this holds for the declaring type of a member function `fn`, the class template is genuinely
+ * "alive" (used with a concrete type somewhere), and the fact that `fn` itself was never
+ * instantiated is real evidence that it is unused: for a member function to lack a concrete
+ * instantiation while sibling members do have one, it must never have been called from any of
+ * those sibling bodies.
+ *
+ * `pragma[noinline]` keeps this a standalone relation of arity one. Inlined into the caller, the
+ * join orderer loses the fact that `c` is functionally determined and materialises the full
+ * (member, sibling) cross product per class before projecting it away, which is quadratic in the
+ * size of the largest class.
+ */
+pragma[noinline]
+private predicate classHasAnyInstantiatedMember(Class c) {
+  exists(Function sibling, Function siblingInstantiation |
+    sibling.getDeclaringType() = c and
+    siblingInstantiation.isConstructedFrom(sibling)
+  )
+}
+
+/**
+ * Holds if `fn` is a function from an uninstantiated template for which no concrete
+ * instantiation exists anywhere in the database, and no other member of the same
+ * class-template pattern is instantiated either.
+ *
+ * When a class template is never instantiated with a concrete type in the analyzed
+ * compilation units, Clang never elaborates a body for its member functions, so
+ * `Call`/`FunctionCall` targets within that pattern's own text cannot be resolved by
+ * `DynamicCallGraph::getTarget()` or `VirtualDispatch`, even for calls between sibling members
+ * of the very same class (e.g. a constructor calling a private helper). This is common for
+ * generic "plumbing" library code (CRTP-style wrappers, etc.)
+ * that is only ever instantiated by downstream consumers outside of this codebase. In that
+ * situation we have no visibility at all into the call graph, so we conservatively treat the
+ * function as "used" (out of scope for this analysis) rather than report it as dead code.
+ *
+ * We only do this when *no* sibling member of the class pattern has an instantiation either
+ * (see `classHasAnyInstantiatedMember`): if some sibling *is* instantiated, the class is
+ * genuinely used, and `fn` lacking an instantiation is real (not merely missing) evidence that
+ * it is unused.
+ */
+predicate hasNoVisibleInstantiation(Function fn) {
+  // Restricted to class-template members: a standalone function template that is never
+  // instantiated anywhere is genuinely dead code, and detecting that does not suffer from the
+  // "sibling member of the same class" ambiguity this predicate is designed for.
+  fn instanceof MemberFunction and
+  fn.isFromUninstantiatedTemplate(_) and
+  not exists(Function instantiation | instantiation.isConstructedFrom(fn)) and
+  not classHasAnyInstantiatedMember(fn.getDeclaringType())
+}
+
 /** A `Function` nested in an anonymous namespace. */
 class AnonymousNamespaceFunction extends Function {
   AnonymousNamespaceFunction() { getNamespace().getParentNamespace*().isAnonymous() }
@@ -74,7 +147,7 @@ module UnusedLocalFunction<UnusedLocalFunctionConfigSig Config> {
   query predicate problems(LocalFunction unusedLocalFunction, string message) {
     not isExcluded(unusedLocalFunction, Config::getQuery()) and
     // No static or dynamic call target for this function
-    not unusedLocalFunction = getTarget(_) and
+    not functionIsCalled(unusedLocalFunction) and
     // If this is a TemplateFunction or an instantiation of a template, then only report it as unused
     // if all other instantiations of the template are unused
     not exists(
@@ -88,7 +161,7 @@ module UnusedLocalFunction<UnusedLocalFunctionConfigSig Config> {
     |
       // There exists an instantiation which is called
       functionFromInstantiatedTemplate.isConstructedFrom(functionFromUninstantiatedTemplate) and
-      functionFromInstantiatedTemplate = getTarget(_)
+      functionIsCalled(functionFromInstantiatedTemplate)
     ) and
     // A function is defined as "used" if any one of the following holds true:
     // - It's an explicitly deleted functions e.g. =delete
@@ -100,6 +173,9 @@ module UnusedLocalFunction<UnusedLocalFunctionConfigSig Config> {
     not unusedLocalFunction.getAnAttribute().getName() = "maybe_unused" and
     not overloadedFunctionIsCalled(unusedLocalFunction) and
     not addressBeenTaken(unusedLocalFunction) and
+    // We have no visibility into the call graph of a template that is never instantiated
+    // anywhere in the database, so we cannot reliably tell it is unused.
+    not hasNoVisibleInstantiation(unusedLocalFunction) and
     message =
       unusedLocalFunction.getLocalFunctionType() + " function " + unusedLocalFunction.getName() +
         " is not statically called, or is in an unused template."

cpp/common/test/rules/unusedlocalfunction/UnusedLocalFunction.expected

@@ -6,3 +6,5 @@
 | test.cpp:85:6:85:7 | h3 | Anonymous namespace function h3 is not statically called, or is in an unused template. |
 | test.cpp:144:8:144:8 | f | Anonymous namespace class member function f is not statically called, or is in an unused template. |
 | test.cpp:150:8:150:8 | f | Anonymous namespace class member function f is not statically called, or is in an unused template. |
+| test.cpp:214:9:214:18 | deadHelper | Private member function deadHelper is not statically called, or is in an unused template. |
+| test.cpp:236:16:236:28 | neverUsedPure | Private member function neverUsedPure is not statically called, or is in an unused template. |

cpp/common/test/rules/unusedlocalfunction/test.cpp

@@ -157,4 +157,82 @@ class C3 {
 public:
   void f() {} // COMPLIANT - public external linkage
 };
-} // namespace N1
+} // namespace N1
+
+class PureVirtualBase {
+public:
+  void callImpl() { impl(); }
+
+private:
+  virtual void impl() = 0; // COMPLIANT - pure virtual contract.
+};
+
+class PureVirtualDerived : public PureVirtualBase {
+private:
+  void impl() override {}
+};
+
+void test_pure_virtual_private_member() {
+  PureVirtualDerived derived;
+  derived.callImpl();
+}
+
+/**
+ * Class templates that are never instantiated anywhere in the analyzed
+ * compilation units.
+ *
+ * Clang never elaborates a body for the members of such patterns, so calls
+ * between sibling members (even genuine ones, like a public entry point calling
+ * a private helper) cannot be resolved by the call graph. We conservatively
+ * treat all of them as used, rather than risk reporting them as dead code.
+ */
+template <class NeverUsedT> class NeverInstantiatedFactory {
+public:
+  static void Create() { instanceHelper(); }
+
+private:
+  static void instanceHelper() {
+  } // COMPLIANT - class template is never instantiated anywhere in this
+    // translation unit, so the analysis has no visibility into whether
+    // `Create` (also never instantiated) really calls it; conservatively
+    // not reported.
+};
+
+/**
+ * A class template that *is* instantiated (and its caller genuinely used), so
+ * the ordinary per-instantiation call-graph reasoning applies and a
+ * truly-unused private helper is still correctly reported.
+ */
+template <class UsedT> class InstantiatedFactory {
+public:
+  UsedT get() { return makeValue(); }
+
+private:
+  UsedT makeValue() {
+    return UsedT();
+  }                    // COMPLIANT - called by get(), which is instantiated.
+  UsedT deadHelper() { // NON_COMPLIANT - never called, and the class template
+                       // is instantiated, so the analysis does have visibility
+                       // into this member.
+    return UsedT();
+  }
+};
+
+void test_instantiated_factory() {
+  InstantiatedFactory<int> factory;
+  factory.get();
+}
+/**
+ * A private pure virtual that is genuinely dead: it is never called through the
+ * non-virtual interface, and no derived class ever overrides it. Pure virtual
+ * functions are deliberately in scope for this query (see
+ * `UnusedFunctions::UsableFunction`), so this must still be reported.
+ */
+class DeadPureVirtualBase {
+public:
+  void unrelated() {}
+
+private:
+  virtual void neverUsedPure() = 0; // NON_COMPLIANT - never called, never
+                                    // overridden.
+};