hwright added inline comments.
================
Comment at: clang-tidy/abseil/DurationFactoryScaleCheck.cpp:36
+GetScaleForFactory(llvm::StringRef FactoryName) {
+ static const auto *ScaleMap =
+ new std::unordered_map<std::string, DurationScale>(
----------------
Eugene.Zelenko wrote:
> This will cause memory leaks, so may be unique_ptr should be used to hold
> pointer? May be LLVM ADT has better container?
This is a tradeoff between leaking a small amount of known memory for the
duration of the program, and constructing this map every time this function is
invoked. Since I expect the latter to occur frequently, that's a tradeoff I
think is acceptable. (Ideally, this would be a compile-time constant, but
sadly we don't yet have a `constexpr` dictionary type.)
Of course, if there is a more typical way of doing that here, I'm happy to use
it.
================
Comment at: docs/ReleaseNotes.rst:88
+ Checks for cases where arguments to ``absl::Duration`` factory functions are
+ scaled internally and could be changed to a different factory function. This
+ check also looks for arguements with a zero value and suggests using
----------------
Eugene.Zelenko wrote:
> Second statement belongs to documentation.
It already is in the user-facing documentation, are saying it should be removed
from here?
https://reviews.llvm.org/D54246
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
