https://github.com/Sirraide commented:
Just a few things I’ve noticed so far. I definitely need to take a closer look at all of this, because it is a lot of code, but before I do that, I have a few questions/remarks about the overall design of this: 1. I think there already is a comment about that, but this is a lot of code. All the code added to `AnalysisBasedWarnings.cpp` should probably just be in a separate file to make this a bit easier to review (and also because it’s probably only going to grow in size as time goes on...) 2. This pr feels like it’s spending a lot of time defining data structures (often just for optimisation purposes), if that makes sense; I’ve seen at least three separate classes that define an `insert` function. I’m wondering if we can’t just reuse existing facilities for a lot of this (e.g. `PartialDiagnostic`), and whether we really need to be this up-front about optimising certain things, e.g. a `std::unique_ptr<SmallVector<...>>` for optimising for the case of having 0 elements feels like it’s a bit much, at least to me. https://github.com/llvm/llvm-project/pull/99656 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
