[PATCH] D119004: [NFC][analyzer] Allow CallDescriptions to be matched with CallExprs

[Kristóf Umann via Phabricator via cfe-commits]

Szelethus added inline comments.


================
Comment at: clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallDescription.h:102-103
 
+  /// Returns true if the CallEvent is a call to a function that matches
+  /// the CallDescription.
+  ///
----------------
steakhal wrote:
> NoQ wrote:
> > 
> ping
This is actually the `CallEvent` variant, I corrected the `CallExpr` one :)


================
Comment at: clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallDescription.h:111
 
-  /// \copydoc clang::ento::matchesAny(const CallEvent &, const CallDescription &)
+  /// \copydoc clang::ento::CallDescription::matchesAny(const CallEvent &, const CallDescription &)
   template <typename... Ts>
----------------
steakhal wrote:
> I think it's a free function. I know that copydoc did not work for this example.
> Are you sure adding the `::CallDescription` fixes the doc comment?
Yes! Its still in `CallDescription`s "namespace", as its defined in-class. I ran `doxygen-clang` and can confirm that this works, but only without line breaks (or at least I haven't figured out how to do it without it, not even with backslash)


================
Comment at: clang/unittests/StaticAnalyzer/CallDescriptionTest.cpp:501
+//===----------------------------------------------------------------------===//
+// Testing through a checker interface.
+//
----------------
steakhal wrote:
> You could have define `ResultMap` ad a virtual base class, which would be implemented by two different classes. One of which would use the `asWritten` lookups, etc.
> You could `make_unique` of the required one and inject it to the `Action`.
> 
> That way those tests would look just the same as the previous ones.
My rationale was that reusing an already existing machinery which is already used in many of the static analyzer unit tests is far friendlier for beginners. I think fracturing the file specific stuff here would increase the barrier of entry for very little gain.

One of the things I like a lot on unit tests is that they demonstrate on a small scale a part of the analyzer's core machinery. I think dividing this up would go against that as well.


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

https://reviews.llvm.org/D119004