[PATCH] D66572: [analyzer] NFC: BugReporter Separation Ep.I.

Kristóf Umann via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Sep 6 16:27:45 PDT 2019


Szelethus accepted this revision.
Szelethus added a comment.
This revision is now accepted and ready to land.

This patch set the goal of splitting `BugReport` into two different report kinds, and I think it did that well. Not only that, we drastically improved the documentation, formalized many things that weren't in the core before, so I wouldn't like you to bear the burdern of never ending rebases (it doesn't make reviewing easier either). Let's work on the rest of the code in a followup patch.

I agree with @gribozavr, we could do better, and should do better. Many core classes in the analyzer feel like everyone just added 1-2 functions, 1-2 branches to get something done quickly, and while those functions in the context of the patch they were added in may have been obvious, it lead us to a cluster on non-descriptive, undocumented, confusing interface and implementation code (n+1 location related functions in this instance). While adding new checkers, better support for C++17 and whatnot is what will ultimately make the end user experience better, I like how we're investing a lot of effort into the health of the codebase nowadays, and I think it'll pay off in the long term.


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

https://reviews.llvm.org/D66572





More information about the cfe-commits mailing list