[PATCH] D55245: [clang-tidy] Add the abseil-duration-subtraction check

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

Please always upload all patches with the full context (`-U99999`)



================
Comment at: clang-tidy/abseil/DurationRewriter.cpp:35
+getInverseForScale(DurationScale Scale) {
+  static const std::unordered_map<DurationScale,
+                                  std::pair<llvm::StringRef, llvm::StringRef>>
----------------
https://llvm.org/docs/ProgrammersManual.html#other-map-like-container-options
> We never use hash_set and unordered_set because they are generally very expensive (each insertion requires a malloc) and very non-portable.

Since the key is an enum, [[ https://llvm.org/docs/ProgrammersManual.html#llvm-adt-indexedmap-h | `llvm/ADT/IndexedMap.h` ]] should be a much better fit.


================
Comment at: clang-tidy/abseil/DurationRewriter.cpp:68
+      getInverseForScale(Scale);
+  if (const Expr *MaybeCallArg = selectFirst<const Expr>(
+          "e", match(callExpr(callee(functionDecl(
----------------
`if (const auto *MaybeCallArg`


================
Comment at: clang-tidy/abseil/DurationRewriter.cpp:68
+      getInverseForScale(Scale);
+  if (const Expr *MaybeCallArg = selectFirst<const Expr>(
+          "e", match(callExpr(callee(functionDecl(
----------------
lebedev.ri wrote:
> `if (const auto *MaybeCallArg`
Early return?


================
Comment at: clang-tidy/abseil/DurationRewriter.cpp:74
+                     Node, *Result.Context))) {
+    return tooling::fixit::getText(*MaybeCallArg, *Result.Context).str();
+  }
----------------
So you generate fix-it, and then immediately degrade it into a string. Weird.


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

https://reviews.llvm.org/D55245