[PATCH] D57353: [clang-tidy] Add the abseil-duration-double-conversion check

Haojian Wu via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Feb 4 03:08:15 PST 2019


hokein added a comment.

The code looks good, but I have a concern about the check name -- `double` seems a confusing word, see my comment.



================
Comment at: clang-tidy/abseil/DurationDoubleConversionCheck.cpp:27
+
+  for (const auto &s : Scales) {
+    std::string DurationFactory = (llvm::Twine("::absl::") + s).str();
----------------
nit: `s` => `S`; might be inline the `Scales`?


================
Comment at: clang-tidy/abseil/DurationDoubleConversionCheck.cpp:51
+
+  diag(OuterCall->getBeginLoc(), "remove double conversion of absl::Duration")
+      << FixItHint::CreateReplacement(
----------------
The `double` word may cause confusion easily -- at the first glance, I thought that it means convert the `absl::Duration` to the `double` type, but it turned out not.. How about using `abseil-duration-necessary-conversion`?


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

https://reviews.llvm.org/D57353





More information about the cfe-commits mailing list