gribozavr marked an inline comment as done. gribozavr added inline comments.
================ Comment at: clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h:408 -public: - enum Kind { BasicBRKind, PathSensitiveBRKind }; - ---------------- NoQ wrote: > gribozavr wrote: > > NoQ wrote: > > > Hey, i just added that! :D (well, renamed) (rC369320) > > > > > > I believe we do want a separation between a {bug report, bug reporter} > > > classes that's only suitable for path-insensitive reports (which would > > > live in libAnalysis and we could handle them to clang-tidy while still > > > being able to compile it without CLANG_ENABLE_STATIC_ANALYZER) and all > > > the path-sensitive report logic that is pretty huge but only Static > > > Analyzer needs it. For that purpose we'd better leave this in. WDYT? See > > > also D66460. > > > > > > Should i ask on the mailing list whether you're willing to sacrifice > > > building clang-tidy without CLANG_ENABLE_STATIC_ANALYZER in order to > > > transition to BugReporter? Cause i thought it was obvious that it's not a > > > great idea, even if it causes me to do a bit of cleanup work on my end. > > > > > > That said, i'm surprised that it's deadcode, i.e. that nobody ever > > > dyn_casts bug reporters, even though we already have two bug reporter > > > classes. Maybe we can indeed remove this facility. > > > I believe we do want a separation between a {bug report, bug reporter} > > > classes [...] > > > > Yes, the separation is nice. > > > > > For that purpose we'd better leave this in. > > > > `Kind` is only needed for dynamic casting between different bug reporters. > > I'm not sure that is useful in practice (case in point -- the `classof` is > > not used today), specifically because the client that produces diagnostics > > will need to work with a bug reporter of the correct kind. If a > > path-sensitive client is handed a pointer to the base class, `BugReporter`, > > would it try to `dyn_cast` it to the derived class?.. what if it fails?.. > > > > Basically, I don't understand why one would want dynamic casting for these > > classes. I totally agree with the separation though. > > > > > Should i ask on the mailing list whether you're willing to sacrifice > > > building clang-tidy without CLANG_ENABLE_STATIC_ANALYZER in order to > > > transition to BugReporter? > > > > I personally don't mind CLANG_ENABLE_STATIC_ANALYZER going away completely > > (I have a fast machine and use a build system with strong caching), > > however, there are other people who are a lot more sensitive to build time, > > and for whom it might be important. > > I personally don't mind CLANG_ENABLE_STATIC_ANALYZER going away completely > > (I have a fast machine and use a build system with strong caching), > > however, there are other people who are a lot more sensitive to build time, > > and for whom it might be important. > > I think for clang it's mostly about binary size; people occasionally want > compact clangs. > > > I'm not sure that is useful in practice (case in point -- the classof is > > not used today), specifically because the client that produces diagnostics > > will need to work with a bug reporter of the correct kind. > > Yeah, i agree. I'll add it back if i ever need it. > > I think for clang it's mostly about binary size; people occasionally want > compact clangs. That's true -- some people also want to have a small clang binary; however you asked about clang-tidy without CLANG_ENABLE_STATIC_ANALYZER :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66473/new/ https://reviews.llvm.org/D66473 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits