[clang-tools-extra] [clang-tidy] Create bugprone-incorrect-enable-shared-from-this check (PR #102299)

[Julian Schmidt via cfe-commits]

5chmidti wrote:

> for me oryginal version that used registerMatchers instead of visitor were way better, specially that there are matchers that could be used for this, or could be written.

We do have the `isDerivedFrom` matcher, actually.

> in most projects not many classes actually used std::enable_shared_from_this and there will be no visible performance gain from "remembering" list of classes, and as we could see this approach makes this check more complicated.

Maybe we should just go with what is simpler, and check the performance of the implementation, but I think it could be more performant.

```c++
struct A {};
struct B : public A {};
struct C : public B {};
struct D : public B {};
```

When we are visiting a definition of a class, we know that every base class has been visited already, so we do not need to traverse the inheritance hierarchy and instead just need to check our set.
We mark `A` as either an `enable_shared_from_this` class, or not.
We check for `B`, if any of its base classes is already in the set, and the same goes for `C` and `D`. The base classes of `C` and `D` are never actually transitively traverse when we check `C` and `D`, but instead we do a lookup because we already computed the answer.

---

@MichelleCDjunaidi 

Untested, but this should be roughly what needs to be done (minus the check for a missing `std::` in front of `enable_shared_from_this`).

```c++
  bool VisitCXXRecordDecl(CXXRecordDecl *RDecl) {
      if (RDecl->getQualifiedNameAsString() ==
                            "std::enable_shared_from_this")
          EnableSharedClassSet.insert(RDecl->getCanonicalDecl());

      for (const auto &Base : RDecl->bases()) {
        const auto* BaseRecord = Base.getType()->getAsCXXRecordDecl()->getCanonicalDecl();
        
        if (EnableSharedClassSet.contains(BaseRecord))
        {
            if (Base.getAccessSpecifier() != AccessSpecifier::AS_public)
               Check.diag(...);
            else
                EnableSharedClassSet.insert(RDecl->getCanonicalDecl());
        }
      }
      return true;
    }
```

Your `VisitCXXBaseSpecifier` has the same name as the other visitor functions, but it is technically not in the format of the `RecursiveASTVisitor` and may be confusing. `Handle...` or `Check...` would be better IMO.

Either way, the direction is still an open question

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