[PATCH] D111534: [analyzer][NFC] Refactor CallEvent::isCalled()

[Balázs Benics via Phabricator via cfe-commits]

steakhal marked 3 inline comments as done.
steakhal added inline comments.


================
Comment at: clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h:1289
+  /// E.g. { "std", "vector", "data" } -> "vector", "std"
+  auto begin_qualified_name_parts() const {
+    return std::next(QualifiedName.rbegin());
----------------
ASDenysPetrov wrote:
> What rules did you use to name this functions? It seems that it goes against [[ https://llvm.org/docs/CodingStandards.html#name-types-functions-variables-and-enumerators-properly | LLVM naming rules]].
Not exactly. There is an exception for `begin()` and `end()`.
That being said, `begin` should have been a suffix instead of being a prefix.
But I still like this more :D


================
Comment at: clang/lib/StaticAnalyzer/Core/CallEvent.cpp:336-352
+  const auto ExactMatchArgAndParamCounts =
+      [](const CallEvent &Call, const CallDescription &CD) -> bool {
+    const bool ArgsMatch =
+        !CD.RequiredArgs || CD.RequiredArgs == Call.getNumArgs();
+    const bool ParamsMatch =
+        !CD.RequiredParams || CD.RequiredParams == Call.parameters().size();
+    return ArgsMatch && ParamsMatch;
----------------
ASDenysPetrov wrote:
> Can we move these lambdas in separate functions? IMO it could make the code even more readable.
We could, but I'm not planning to move them.
That would be more appropriate to move all `CallDescription` implementation to its own translation unit.


================
Comment at: clang/lib/StaticAnalyzer/Core/CallEvent.cpp:349
+        [](const DeclContext *Ctx) -> const DeclContext * {
+      while (Ctx && !isa<NamespaceDecl>(Ctx) && !isa<RecordDecl>(Ctx))
+        Ctx = Ctx->getParent();
----------------
martong wrote:
> steakhal wrote:
> > TBH I don't understand why doesn't `isa<NamespaceDecl, RecordDecl>()` work. I could have used this variadic form so many times.
> > WDYT, should I propose turning `isa` and `isa_and_nonnull`˙into variadic functions?
> > TBH I don't understand why doesn't `isa<NamespaceDecl, RecordDecl>()` work. I could have used this variadic form so many times.
> > WDYT, should I propose turning `isa` and `isa_and_nonnull`˙into variadic functions?
> 
> Yes, that's a good idea!
Actually, it's already variadic :D
I created a follow-up patch for addressing similar issues in D111982.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D111534/new/

https://reviews.llvm.org/D111534