[clang-tools-extra] [clang][dataflow] Change `diagnoseFunction` to take type of diagnostic list instead of diagnostic itself. (PR #66014)

via cfe-commits cfe-commits at lists.llvm.org
Wed Sep 13 02:21:52 PDT 2023


================
@@ -245,10 +245,10 @@ runDataflowAnalysis(
 ///   iterations.
 /// - This limit is still low enough to keep runtimes acceptable (on typical
 ///   machines) in cases where we hit the limit.
-template <typename AnalysisT, typename Diagnostic>
-llvm::Expected<std::vector<Diagnostic>> diagnoseFunction(
+template <typename AnalysisT, typename DiagnosticList>
+llvm::Expected<DiagnosticList> diagnoseFunction(
----------------
martinboehme wrote:

> > That's my intuition as well (that `llvm::SmallVector` is likely better). But, who knows? An analysis could expect to have a non-small number of diagnostics per line.
> 
> Even then, a `SmallVector` isn't a bad choice -- and potentially a better choice than a `std::vector`. (The "small" in `SmallVector` doesn't mean "this only performs well if the contents are small" -- it just means "this performs particularly well if the contents are small".)
> 
> The main cost of a `SmallVector` is that it occupies more memory than a `std::vector`. This isn't an issue here since we're allocating only one of these vectors at a time, on the stack. In terms of runtime, my understanding is that `SmallVector` is typically _faster_ than `std::vector` (for small _and_ large sizes). From the [LLVM Programmers' Manual](https://llvm.org/docs/ProgrammersManual.html#llvm-adt-smallvector-h):
> 
> > SmallVector has grown a few other minor advantages over std::vector, causing `SmallVector<Type, 0>` to be preferred over `std::vector<Type>`."
> > [...]
> > SmallVector understands std::is_trivially_copyable and uses realloc aggressively
> 
> This talks about `SmallVector<Type, 0>`, but this is just because for this general statement they're thinking about the size of the object. In our case, that isn't a concern, and we want to take advantage of the "small number of elements" optimization, so `SmallVector<Type>` seems like the right choice.

So bottom line, I don't expect anyone to ever want to use `std::vector` here.

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


More information about the cfe-commits mailing list