Skip to content

Conversation

@localspook
Copy link
Contributor

The important part of this PR is the changes to getDurationInverseForScale. I changed the other get*ForScale functions so that they all follow the same pattern, but those aren't as important.

@llvmbot
Copy link
Member

llvmbot commented Sep 11, 2025

@llvm/pr-subscribers-clang-tools-extra

Author: Victor Chernyakin (localspook)

Changes

The important part of this PR is the changes to getDurationInverseForScale. I changed the other get*ForScale functions so that they all follow the same pattern, but those aren't as important.


Full diff: https://github.com/llvm/llvm-project/pull/158166.diff

1 Files Affected:

  • (modified) clang-tools-extra/clang-tidy/abseil/DurationRewriter.cpp (+30-78)
diff --git a/clang-tools-extra/clang-tidy/abseil/DurationRewriter.cpp b/clang-tools-extra/clang-tidy/abseil/DurationRewriter.cpp
index 4cdbbd43c1431..1dab94558d0f7 100644
--- a/clang-tools-extra/clang-tidy/abseil/DurationRewriter.cpp
+++ b/clang-tools-extra/clang-tidy/abseil/DurationRewriter.cpp
@@ -6,24 +6,17 @@
 //
 //===----------------------------------------------------------------------===//
 
+#include <array>
 #include <cmath>
 #include <optional>
 
 #include "DurationRewriter.h"
 #include "clang/Tooling/FixIt.h"
-#include "llvm/ADT/IndexedMap.h"
 
 using namespace clang::ast_matchers;
 
 namespace clang::tidy::abseil {
 
-struct DurationScale2IndexFunctor {
-  using argument_type = DurationScale;
-  unsigned operator()(DurationScale Scale) const {
-    return static_cast<unsigned>(Scale);
-  }
-};
-
 /// Returns an integer if the fractional part of a `FloatingLiteral` is `0`.
 static std::optional<llvm::APSInt>
 truncateIfIntegral(const FloatingLiteral &FloatLiteral) {
@@ -39,31 +32,17 @@ truncateIfIntegral(const FloatingLiteral &FloatLiteral) {
 
 const std::pair<llvm::StringRef, llvm::StringRef> &
 getDurationInverseForScale(DurationScale Scale) {
-  static const llvm::IndexedMap<std::pair<llvm::StringRef, llvm::StringRef>,
-                                DurationScale2IndexFunctor>
-      InverseMap = []() {
-        // TODO: Revisit the immediately invoked lambda technique when
-        // IndexedMap gets an initializer list constructor.
-        llvm::IndexedMap<std::pair<llvm::StringRef, llvm::StringRef>,
-                         DurationScale2IndexFunctor>
-            InverseMap;
-        InverseMap.resize(6);
-        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(
-            "::absl::ToDoubleMilliseconds", "::absl::ToInt64Milliseconds");
-        InverseMap[DurationScale::Microseconds] = std::make_pair(
-            "::absl::ToDoubleMicroseconds", "::absl::ToInt64Microseconds");
-        InverseMap[DurationScale::Nanoseconds] = std::make_pair(
-            "::absl::ToDoubleNanoseconds", "::absl::ToInt64Nanoseconds");
-        return InverseMap;
-      }();
-
-  return InverseMap[Scale];
+  static constexpr std::array<std::pair<llvm::StringRef, llvm::StringRef>, 6>
+      InverseMap = {{
+          {"::absl::ToDoubleHours", "::absl::ToInt64Hours"},
+          {"::absl::ToDoubleMinutes", "::absl::ToInt64Minutes"},
+          {"::absl::ToDoubleSeconds", "::absl::ToInt64Seconds"},
+          {"::absl::ToDoubleMilliseconds", "::absl::ToInt64Milliseconds"},
+          {"::absl::ToDoubleMicroseconds", "::absl::ToInt64Microseconds"},
+          {"::absl::ToDoubleNanoseconds", "::absl::ToInt64Nanoseconds"},
+      }};
+
+  return InverseMap[llvm::to_underlying(Scale)];
 }
 
 /// If `Node` is a call to the inverse of `Scale`, return that inverse's
@@ -103,58 +82,31 @@ rewriteInverseTimeCall(const MatchFinder::MatchResult &Result,
 
 /// Returns the factory function name for a given `Scale`.
 llvm::StringRef getDurationFactoryForScale(DurationScale Scale) {
-  switch (Scale) {
-  case DurationScale::Hours:
-    return "absl::Hours";
-  case DurationScale::Minutes:
-    return "absl::Minutes";
-  case DurationScale::Seconds:
-    return "absl::Seconds";
-  case DurationScale::Milliseconds:
-    return "absl::Milliseconds";
-  case DurationScale::Microseconds:
-    return "absl::Microseconds";
-  case DurationScale::Nanoseconds:
-    return "absl::Nanoseconds";
-  }
-  llvm_unreachable("unknown scaling factor");
+  static constexpr std::array<llvm::StringRef, 6> FactoryMap = {
+      "absl::Hours",        "absl::Minutes",      "absl::Seconds",
+      "absl::Milliseconds", "absl::Microseconds", "absl::Nanoseconds",
+  };
+
+  return FactoryMap[llvm::to_underlying(Scale)];
 }
 
 llvm::StringRef getTimeFactoryForScale(DurationScale Scale) {
-  switch (Scale) {
-  case DurationScale::Hours:
-    return "absl::FromUnixHours";
-  case DurationScale::Minutes:
-    return "absl::FromUnixMinutes";
-  case DurationScale::Seconds:
-    return "absl::FromUnixSeconds";
-  case DurationScale::Milliseconds:
-    return "absl::FromUnixMillis";
-  case DurationScale::Microseconds:
-    return "absl::FromUnixMicros";
-  case DurationScale::Nanoseconds:
-    return "absl::FromUnixNanos";
-  }
-  llvm_unreachable("unknown scaling factor");
+  static constexpr std::array<llvm::StringRef, 6> FactoryMap = {
+      "absl::FromUnixHours",  "absl::FromUnixMinutes", "absl::FromUnixSeconds",
+      "absl::FromUnixMillis", "absl::FromUnixMicros",  "absl::FromUnixNanos",
+  };
+
+  return FactoryMap[llvm::to_underlying(Scale)];
 }
 
 /// Returns the Time factory function name for a given `Scale`.
 llvm::StringRef getTimeInverseForScale(DurationScale Scale) {
-  switch (Scale) {
-  case DurationScale::Hours:
-    return "absl::ToUnixHours";
-  case DurationScale::Minutes:
-    return "absl::ToUnixMinutes";
-  case DurationScale::Seconds:
-    return "absl::ToUnixSeconds";
-  case DurationScale::Milliseconds:
-    return "absl::ToUnixMillis";
-  case DurationScale::Microseconds:
-    return "absl::ToUnixMicros";
-  case DurationScale::Nanoseconds:
-    return "absl::ToUnixNanos";
-  }
-  llvm_unreachable("unknown scaling factor");
+  static constexpr std::array<llvm::StringRef, 6> InverseMap = {
+      "absl::ToUnixHours",  "absl::ToUnixMinutes", "absl::ToUnixSeconds",
+      "absl::ToUnixMillis", "absl::ToUnixMicros",  "absl::ToUnixNanos",
+  };
+
+  return InverseMap[llvm::to_underlying(Scale)];
 }
 
 /// Returns `true` if `Node` is a value which evaluates to a literal `0`.

@llvmbot
Copy link
Member

llvmbot commented Sep 11, 2025

@llvm/pr-subscribers-clang-tidy

Author: Victor Chernyakin (localspook)

Changes

The important part of this PR is the changes to getDurationInverseForScale. I changed the other get*ForScale functions so that they all follow the same pattern, but those aren't as important.


Full diff: https://github.com/llvm/llvm-project/pull/158166.diff

1 Files Affected:

  • (modified) clang-tools-extra/clang-tidy/abseil/DurationRewriter.cpp (+30-78)
diff --git a/clang-tools-extra/clang-tidy/abseil/DurationRewriter.cpp b/clang-tools-extra/clang-tidy/abseil/DurationRewriter.cpp
index 4cdbbd43c1431..1dab94558d0f7 100644
--- a/clang-tools-extra/clang-tidy/abseil/DurationRewriter.cpp
+++ b/clang-tools-extra/clang-tidy/abseil/DurationRewriter.cpp
@@ -6,24 +6,17 @@
 //
 //===----------------------------------------------------------------------===//
 
+#include <array>
 #include <cmath>
 #include <optional>
 
 #include "DurationRewriter.h"
 #include "clang/Tooling/FixIt.h"
-#include "llvm/ADT/IndexedMap.h"
 
 using namespace clang::ast_matchers;
 
 namespace clang::tidy::abseil {
 
-struct DurationScale2IndexFunctor {
-  using argument_type = DurationScale;
-  unsigned operator()(DurationScale Scale) const {
-    return static_cast<unsigned>(Scale);
-  }
-};
-
 /// Returns an integer if the fractional part of a `FloatingLiteral` is `0`.
 static std::optional<llvm::APSInt>
 truncateIfIntegral(const FloatingLiteral &FloatLiteral) {
@@ -39,31 +32,17 @@ truncateIfIntegral(const FloatingLiteral &FloatLiteral) {
 
 const std::pair<llvm::StringRef, llvm::StringRef> &
 getDurationInverseForScale(DurationScale Scale) {
-  static const llvm::IndexedMap<std::pair<llvm::StringRef, llvm::StringRef>,
-                                DurationScale2IndexFunctor>
-      InverseMap = []() {
-        // TODO: Revisit the immediately invoked lambda technique when
-        // IndexedMap gets an initializer list constructor.
-        llvm::IndexedMap<std::pair<llvm::StringRef, llvm::StringRef>,
-                         DurationScale2IndexFunctor>
-            InverseMap;
-        InverseMap.resize(6);
-        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(
-            "::absl::ToDoubleMilliseconds", "::absl::ToInt64Milliseconds");
-        InverseMap[DurationScale::Microseconds] = std::make_pair(
-            "::absl::ToDoubleMicroseconds", "::absl::ToInt64Microseconds");
-        InverseMap[DurationScale::Nanoseconds] = std::make_pair(
-            "::absl::ToDoubleNanoseconds", "::absl::ToInt64Nanoseconds");
-        return InverseMap;
-      }();
-
-  return InverseMap[Scale];
+  static constexpr std::array<std::pair<llvm::StringRef, llvm::StringRef>, 6>
+      InverseMap = {{
+          {"::absl::ToDoubleHours", "::absl::ToInt64Hours"},
+          {"::absl::ToDoubleMinutes", "::absl::ToInt64Minutes"},
+          {"::absl::ToDoubleSeconds", "::absl::ToInt64Seconds"},
+          {"::absl::ToDoubleMilliseconds", "::absl::ToInt64Milliseconds"},
+          {"::absl::ToDoubleMicroseconds", "::absl::ToInt64Microseconds"},
+          {"::absl::ToDoubleNanoseconds", "::absl::ToInt64Nanoseconds"},
+      }};
+
+  return InverseMap[llvm::to_underlying(Scale)];
 }
 
 /// If `Node` is a call to the inverse of `Scale`, return that inverse's
@@ -103,58 +82,31 @@ rewriteInverseTimeCall(const MatchFinder::MatchResult &Result,
 
 /// Returns the factory function name for a given `Scale`.
 llvm::StringRef getDurationFactoryForScale(DurationScale Scale) {
-  switch (Scale) {
-  case DurationScale::Hours:
-    return "absl::Hours";
-  case DurationScale::Minutes:
-    return "absl::Minutes";
-  case DurationScale::Seconds:
-    return "absl::Seconds";
-  case DurationScale::Milliseconds:
-    return "absl::Milliseconds";
-  case DurationScale::Microseconds:
-    return "absl::Microseconds";
-  case DurationScale::Nanoseconds:
-    return "absl::Nanoseconds";
-  }
-  llvm_unreachable("unknown scaling factor");
+  static constexpr std::array<llvm::StringRef, 6> FactoryMap = {
+      "absl::Hours",        "absl::Minutes",      "absl::Seconds",
+      "absl::Milliseconds", "absl::Microseconds", "absl::Nanoseconds",
+  };
+
+  return FactoryMap[llvm::to_underlying(Scale)];
 }
 
 llvm::StringRef getTimeFactoryForScale(DurationScale Scale) {
-  switch (Scale) {
-  case DurationScale::Hours:
-    return "absl::FromUnixHours";
-  case DurationScale::Minutes:
-    return "absl::FromUnixMinutes";
-  case DurationScale::Seconds:
-    return "absl::FromUnixSeconds";
-  case DurationScale::Milliseconds:
-    return "absl::FromUnixMillis";
-  case DurationScale::Microseconds:
-    return "absl::FromUnixMicros";
-  case DurationScale::Nanoseconds:
-    return "absl::FromUnixNanos";
-  }
-  llvm_unreachable("unknown scaling factor");
+  static constexpr std::array<llvm::StringRef, 6> FactoryMap = {
+      "absl::FromUnixHours",  "absl::FromUnixMinutes", "absl::FromUnixSeconds",
+      "absl::FromUnixMillis", "absl::FromUnixMicros",  "absl::FromUnixNanos",
+  };
+
+  return FactoryMap[llvm::to_underlying(Scale)];
 }
 
 /// Returns the Time factory function name for a given `Scale`.
 llvm::StringRef getTimeInverseForScale(DurationScale Scale) {
-  switch (Scale) {
-  case DurationScale::Hours:
-    return "absl::ToUnixHours";
-  case DurationScale::Minutes:
-    return "absl::ToUnixMinutes";
-  case DurationScale::Seconds:
-    return "absl::ToUnixSeconds";
-  case DurationScale::Milliseconds:
-    return "absl::ToUnixMillis";
-  case DurationScale::Microseconds:
-    return "absl::ToUnixMicros";
-  case DurationScale::Nanoseconds:
-    return "absl::ToUnixNanos";
-  }
-  llvm_unreachable("unknown scaling factor");
+  static constexpr std::array<llvm::StringRef, 6> InverseMap = {
+      "absl::ToUnixHours",  "absl::ToUnixMinutes", "absl::ToUnixSeconds",
+      "absl::ToUnixMillis", "absl::ToUnixMicros",  "absl::ToUnixNanos",
+  };
+
+  return InverseMap[llvm::to_underlying(Scale)];
 }
 
 /// Returns `true` if `Node` is a value which evaluates to a literal `0`.

@localspook localspook merged commit 5e1b416 into llvm:main Sep 18, 2025
12 checks passed
@localspook localspook deleted the abseil-things branch September 18, 2025 06:31
@llvm-ci
Copy link
Collaborator

llvm-ci commented Sep 18, 2025

LLVM Buildbot has detected a new failure on builder llvm-clang-aarch64-darwin running on doug-worker-4 while building clang-tools-extra at step 6 "test-build-unified-tree-check-all".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/190/builds/27702

Here is the relevant piece of the build log for the reference
Step 6 (test-build-unified-tree-check-all) failure: test (failure)
******************** TEST 'lit :: shtest-external-shell-kill.py' FAILED ********************
Exit Code: 1

Command Output (stdout):
--
# RUN: at line 23
env -u FILECHECK_OPTS "/Library/Developer/CommandLineTools/Library/Frameworks/Python3.framework/Versions/3.9/bin/python3.9" /Users/buildbot/buildbot-root/aarch64-darwin/llvm-project/llvm/utils/lit/lit.py -j1 --order=lexical -a Inputs/shtest-external-shell-kill | grep -v 'bash.exe: warning: could not find /tmp, please create!' | FileCheck /Volumes/RAMDisk/buildbot-root/aarch64-darwin/build/utils/lit/tests/shtest-external-shell-kill.py
# executed command: env -u FILECHECK_OPTS /Library/Developer/CommandLineTools/Library/Frameworks/Python3.framework/Versions/3.9/bin/python3.9 /Users/buildbot/buildbot-root/aarch64-darwin/llvm-project/llvm/utils/lit/lit.py -j1 --order=lexical -a Inputs/shtest-external-shell-kill
# note: command had no output on stdout or stderr
# error: command failed with exit status: 1
# executed command: grep -v 'bash.exe: warning: could not find /tmp, please create!'
# note: command had no output on stdout or stderr
# executed command: FileCheck /Volumes/RAMDisk/buildbot-root/aarch64-darwin/build/utils/lit/tests/shtest-external-shell-kill.py
# .---command stderr------------
# | /Volumes/RAMDisk/buildbot-root/aarch64-darwin/build/utils/lit/tests/shtest-external-shell-kill.py:29:15: error: CHECK-NEXT: is not on the line after the previous match
# | # CHECK-NEXT: end
# |               ^
# | <stdin>:22:6: note: 'next' match was here
# | echo end # RUN: at line 5
# |      ^
# | <stdin>:8:6: note: previous match ended here
# | start
# |      ^
# | <stdin>:9:1: note: non-matching line after previous match is here
# | 
# | ^
# | 
# | Input file: <stdin>
# | Check file: /Volumes/RAMDisk/buildbot-root/aarch64-darwin/build/utils/lit/tests/shtest-external-shell-kill.py
# | 
# | -dump-input=help explains the following input dump.
# | 
# | Input was:
# | <<<<<<
# |          .
# |          .
# |          .
# |         17: sleep 2 # RUN: at line 3 
# |         18: + sleep 2 
# |         19: + sleep 300 
# |         20: kill $PID # RUN: at line 4 
# |         21: + kill 54382 
# |         22: echo end # RUN: at line 5 
# | next:29          !~~                   error: match on wrong line
# |         23: /Volumes/RAMDisk/buildbot-root/aarch64-darwin/build/utils/lit/tests/Inputs/shtest-external-shell-kill/Output/test.txt.script: line 5: echo: write error: Interrupted system call 
# |         24: /Volumes/RAMDisk/buildbot-root/aarch64-darwin/build/utils/lit/tests/Inputs/shtest-external-shell-kill/Output/test.txt.script: line 6: 54382 Terminated: 15 sleep 300 
# |         25:  
# |         26: -- 
# |         27:  
# |          .
...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants