C++: Change exclusion to not be only operator=

Author: jbj

cpp/ql/src/Likely Bugs/Likely Typos/ExprHasNoEffect.ql

@@ -91,16 +91,27 @@ predicate definedInIfDef(Function f) {
   )
 }
 
+/**
+ * Holds if `call` has the form `B::f()` or `q.B::f()`, where `B` is a base
+ * class of the class containing `call`.
+ *
+ * This is most often used for calling base-class functions from within
+ * overrides. Those functions may have no side effect in the current
+ * implementation, but we should not advise callers to rely on this. That would
+ * break encapsulation.
+ */
+predicate baseCall(FunctionCall call) {
+  call.getNameQualifier().getQualifyingElement() =
+    call.getEnclosingFunction().getDeclaringType().(Class).getABaseClass+()
+}
+
 from PureExprInVoidContext peivc, Locatable parent,
   Locatable info, string info_text, string tail
 where // EQExprs are covered by CompareWhereAssignMeant.ql
       not peivc instanceof EQExpr and
       // as is operator==
       not peivc.(FunctionCall).getTarget().hasName("operator==") and
-      // An assignment operator may have no side effects in its current
-      // implementation, but we should not advise callers to rely on this. That
-      // would break encapsulation.
-      not peivc.(FunctionCall).getTarget().hasName("operator=") and
+      not baseCall(peivc) and
       not accessInInitOfForStmt(peivc) and
       not peivc.isCompilerGenerated() and
       not exists(Macro m | peivc = m.getAnInvocation().getAnExpandedElement()) and

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

@@ -67,7 +67,10 @@ class NameQualifier extends NameQualifiableElement, @namequalifier {
  * the latter is qualified by `N1`.
  */
 class NameQualifiableElement extends Element, @namequalifiableelement {
-  /** Gets the name qualifier associated with this element. */
+  /**
+   * Gets the name qualifier associated with this element. For example, the
+   * name qualifier of `N::f()` is `N`.
+   */
   NameQualifier getNameQualifier() {
     namequalifiers(unresolveElement(result),underlyingElement(this),_,_)
   }

cpp/ql/test/query-tests/Likely Bugs/Likely Typos/ExprHasNoEffect/ExprHasNoEffect.expected

@@ -27,6 +27,8 @@
 | test.c:27:9:27:10 | 33 | This expression has no effect. | test.c:27:9:27:10 | 33 |  |
 | test.cpp:24:3:24:3 | call to operator++ | This expression has no effect (because $@ has no external side effects). | test.cpp:9:14:9:23 | operator++ | operator++ |
 | test.cpp:25:3:25:3 | call to operator++ | This expression has no effect (because $@ has no external side effects). | test.cpp:9:14:9:23 | operator++ | operator++ |
+| test.cpp:62:5:62:5 | call to operator= | This expression has no effect (because $@ has no external side effects). | test.cpp:47:14:47:22 | operator= | operator= |
+| test.cpp:65:5:65:5 | call to operator= | This expression has no effect (because $@ has no external side effects). | test.cpp:55:7:55:7 | operator= | operator= |
 | volatile.c:9:5:9:5 | c | This expression has no effect. | volatile.c:9:5:9:5 | c |  |
 | volatile.c:12:5:12:9 | access to array | This expression has no effect. | volatile.c:12:5:12:9 | access to array |  |
 | volatile.c:16:5:16:7 | * ... | This expression has no effect. | volatile.c:16:5:16:7 | * ... |  |