Updated to reflect feedback from review of the parent patch.
Did you intentionally make a stronger check than the usual SourceManager::isInSystemHeader()? If so i'm curious why - like, what's so special about this checker that requires this? Should we fix all of our other checkers? It must be something something module maps. I don't know much about those.
This looks unused.
As far as i'm aware, in C++ every RecordDecl is a CXXRecordDecl.
What's the motivation for skipping those?
Can we elaborate a bit on why it's bad? Even something like ... which is error-prone would be great.
I found members:: fairly unexpected. Maybe simply Class::Member? The surrounding namespace is obvious from the location of the warning and doesn't seem to be worth dumping.
No, there was no intention. I'll just use SourceManager::isInSystemHeader(). Thanks for pointing this out!
Indeed! Thanks
This checker enforces that there are no raw-pointers/references to uncounted types as member variable but since that's actually how WebKit smartpointers are implemented we need to skip those.
I'll add a comment.
Ok. Shall I add it to the checkers.rst or here?
Ok.
Here. I'd prefer to avoid this situation where the user reads the warning and is like "ok so what :/".
Perfect, thank you!
Hi, @jkorous
Thanks for working on this analyzer check.
Just give you some feedback on this check, it seems to trigger the assertion quite often (we noticed that internally there were a lot of crashes).
you can easily reproduce it by running it a LLVM file, e.g. ./bin/clang-tidy -checks="-*,clang-analyzer-webkit*" ../clang-tools-extra/clangd/ClangdLSPServer.cpp
Looks similar to. It sounds pretty important to address quickly because it crashes on a fairly large portion of C++ code with default settings.