Charusso accepted this revision. Charusso marked an inline comment as done. Charusso added a comment. This revision is now accepted and ready to land.
My concern was the too heavy `Optional` and `bool` usage. Cool patch! ================ Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:61 + using FnCheck = bool (StreamChecker::*)(const CallEvent &, + CheckerContext &) const; + ---------------- balazske wrote: > Szelethus wrote: > > Charusso wrote: > > > I prefer `std::function`, because it is modern. > > > ``` > > > using StreamCheck = > > > std::function<void(const StreamChecker *, const CallEvent &, > > > CheckerContext &)>; > > > ``` > > > I think it is fine with pointers, but at some point we need to modernize > > > this. > > But its also a lot more expensive. > > https://blog.demofox.org/2015/02/25/avoiding-the-performance-hazzards-of-stdfunction/ > > > > `std::function` is able to wrap lambdas with different captures and all > > sorts of things like that, which comes at a cost. > Now `std::function` and `std::bind` is used. Probably more expensive but it > is called once in a `evalCall`, that should be no problem? There is no real overhead, yes. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67706/new/ https://reviews.llvm.org/D67706 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
