[clang-tools-extra] [clang-tidy] Add `bugprone-virtual-arithmetic` check (PR #91951)

[Julian Schmidt via cfe-commits]

================
@@ -0,0 +1,49 @@
+//===--- VirtualArithmeticCheck.cpp - clang-tidy---------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "VirtualArithmeticCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::bugprone {
+
+void VirtualArithmeticCheck::registerMatchers(MatchFinder *Finder) {
+  const auto PointerExprWithVirtualMethod =
+      expr(hasType(pointerType(pointee(hasDeclaration(
+               cxxRecordDecl(hasMethod(isVirtualAsWritten())))))))
+          .bind("pointer");
+
+  const auto ArraySubscript =
+      arraySubscriptExpr(hasBase(PointerExprWithVirtualMethod));
+
+  const auto BinaryOperators =
+      binaryOperator(hasAnyOperatorName("+", "-", "+=", "-="),
+                     hasEitherOperand(PointerExprWithVirtualMethod));
+
+  const auto UnaryOperators =
+      unaryOperator(hasAnyOperatorName("++", "--"),
+                    hasUnaryOperand(PointerExprWithVirtualMethod));
+
+  Finder->addMatcher(
+      expr(anyOf(ArraySubscript, BinaryOperators, UnaryOperators)), this);
----------------
5chmidti wrote:

The check does not diagnose a few cases that IMO should be:

### Inheritance
Diagnose expressions with a pointer type, where the class either directly declares a virtual method (current matcher), or it inherits a pure virtual function that was not yet overridden in the inheritance chain.

This can be fixed by using
```c++
  const auto VirtualRecord =
      cxxRecordDecl(anyOf(isAbstract(), hasMethod(isVirtualAsWritten())));

  const auto PointerExprWithVirtualMethod =
      expr(hasType(pointerType(pointee(hasDeclaration(VirtualRecord)))))
          .bind("pointer");
```

### Type Aliases
Secondly, the matcher does not check the underlying type of type aliases.
The 
```c++
expr(hasType(pointerType(pointee(hasDeclaration(
               cxxRecordDecl(hasMethod(isVirtualAsWritten())))))))
          .bind("pointer")
```
matcher should have a `hasCanonicalType` after the `hasType` and after the `pointee` to fix this.

E.g.:

```c++
using BaseAlias = Base;
using DerivedAlias = Derived;

using BasePtr = BaseAlias*;
using DerivedPtr = DerivedAlias*;

void foo(Derived* p1, BaseAlias* p2, DerivedAlias* p3, BasePtr p4, DerivedPtr p5,
         BasePtr* p6, DerivedPtr* p7) {
    ++p1;
    ++p2;
    ++p3;
    ++p4;
    ++p5;
    ++p6;
    ++p7;
}

class PureBase {
  virtual void foo()=0;
};
class PureDerived : public PureBase {};

using PureBaseAlias = PureBase;
using PureDerivedAlias = PureDerived;

using PureBasePtr = PureBaseAlias*;
using PureDerivedPtr = PureDerivedAlias*;

void pure_foo(PureDerived* p1, PureBaseAlias* p2, PureDerivedAlias* p3, PureBasePtr p4, PureDerivedPtr p5,
         PureBasePtr* p6, PureDerivedPtr* p7) {
    ++p1;
    ++p2;
    ++p3;
    ++p4;
    ++p5;
    ++p6;
    ++p7;
}
```

pure1-pure5 should be diagnosed, but they currently are not.

https://github.com/llvm/llvm-project/pull/91951