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

[Hyrum Wright via Phabricator via cfe-commits]

hwright added inline comments.


================
Comment at: clang-tidy/abseil/DurationRewriter.cpp:20-39
+struct DurationScale2IndexFunctor {
+  using argument_type = DurationScale;
+  unsigned operator()(DurationScale Scale) const {
+    switch (Scale) {
+    case DurationScale::Hours:
+      return 0;
+    case DurationScale::Minutes:
----------------
lebedev.ri wrote:
> You should not need this if you change the `enum` instead.
The function is still required; the switch can be removed with a `static_cast`.


================
Comment at: clang-tidy/abseil/DurationRewriter.cpp:66-77
+        InverseMap[DurationScale::Hours] =
+            std::make_pair("::absl::ToDoubleHours", "::absl::ToInt64Hours");
+        InverseMap[DurationScale::Minutes] =
+            std::make_pair("::absl::ToDoubleMinutes", "::absl::ToInt64Minutes");
+        InverseMap[DurationScale::Seconds] =
+            std::make_pair("::absl::ToDoubleSeconds", "::absl::ToInt64Seconds");
+        InverseMap[DurationScale::Milliseconds] = std::make_pair(
----------------
lebedev.ri wrote:
> I was thinking of more like
> ```
> for(std::pair<??, ??> e : std::initializer_list({
>                            {DurationScale::Hours, std::make_pair("::absl::ToDoubleHours", "::absl::ToInt64Hours")}.
>                            }))
>   InverseMap[e.first] = e.second;
> ```
The compilable construction looks something like:
```
for (const auto &e : {std::make_pair(DurationScale::Hours,
                                    std::make_pair("::absl::ToDoubleHours",
                                                   "::absl::ToInt64Hours"))})
```
Which is a bit more verbose than just assigning values to the map (and not any more efficient), so I've just left this bit as-is.


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

https://reviews.llvm.org/D55245