aaron.ballman closed this revision.
aaron.ballman added a comment.
I've commit in r347163. Thank you for the patch!
https://reviews.llvm.org/D54246
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinf
aaron.ballman added a comment.
In https://reviews.llvm.org/D54246#1301940, @hwright wrote:
> @aaron.ballman I don't actually have the commit bit, can you commit this, or
> are we waiting for further review?
No further review required. I'm happy to commit for you. I'll do it later today
or tom
hwright added a comment.
@aaron.ballman I don't actually have the commit bit, can you commit this, or
are we waiting for further review?
https://reviews.llvm.org/D54246
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/c
aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.
LGTM!
Comment at: clang-tidy/abseil/DurationFactoryScaleCheck.cpp:73
+ case DurationScale::Hours:
+if (Multiplier <= 1.0 / 60.0)
+ return std::make_tu
hwright added a comment.
I think this is ready to go, please advise on next steps.
https://reviews.llvm.org/D54246
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
hwright marked 5 inline comments as done.
hwright added inline comments.
Comment at: clang-tidy/abseil/DurationFactoryScaleCheck.cpp:73
+ case DurationScale::Hours:
+if (Multiplier <= 1.0 / 60.0)
+ return std::make_tuple(DurationScale::Minutes, Multiplier * 60.0);
-
aaron.ballman added inline comments.
Comment at: clang-tidy/abseil/DurationFactoryScaleCheck.cpp:63
+ assert(FloatLit != nullptr && "Neither IntLit nor FloatLit set");
+ return FloatLit->getValueAsApproximateDouble();
+}
hwright wrote:
> aaron.ballman wrote:
>
hwright added inline comments.
Comment at: clang-tidy/abseil/DurationFactoryScaleCheck.cpp:57-58
+// One and only one of `IntLit` and `FloatLit` should be provided.
+static double GetValue(const IntegerLiteral *IntLit,
+ const FloatingLiteral *FloatLit) {
+
hwright updated this revision to Diff 174039.
hwright marked 11 inline comments as done.
hwright added a comment.
Combined multiplication and division logic, and also now handles scaling of
multiple steps (e.g., Seconds * 3600).
https://reviews.llvm.org/D54246
Files:
clang-tidy/abseil/Abseil
aaron.ballman added inline comments.
Comment at: clang-tidy/abseil/DurationFactoryScaleCheck.cpp:57-58
+// One and only one of `IntLit` and `FloatLit` should be provided.
+static double GetValue(const IntegerLiteral *IntLit,
+ const FloatingLiteral *FloatLit
hwright added inline comments.
Comment at: clang-tidy/abseil/DurationFactoryScaleCheck.cpp:57-58
+// One and only one of `IntLit` and `FloatLit` should be provided.
+static double GetValue(const IntegerLiteral *IntLit,
+ const FloatingLiteral *FloatLit) {
+
hwright updated this revision to Diff 173714.
hwright marked 11 inline comments as done.
hwright added a comment.
Addressed small concerns.
Larger issues pending feedback.
https://reviews.llvm.org/D54246
Files:
clang-tidy/abseil/AbseilTidyModule.cpp
clang-tidy/abseil/CMakeLists.txt
clang
aaron.ballman added inline comments.
Comment at: clang-tidy/abseil/DurationFactoryScaleCheck.cpp:39
+static llvm::Optional
+GetScaleForFactory(llvm::StringRef FactoryName) {
+ static const std::unordered_map ScaleMap(
GetScaleForFactory -> getScaleForFactory, pe
hwright updated this revision to Diff 173543.
hwright marked 16 inline comments as done.
hwright added a comment.
Addressed reviewer feedback.
https://reviews.llvm.org/D54246
Files:
clang-tidy/abseil/AbseilTidyModule.cpp
clang-tidy/abseil/CMakeLists.txt
clang-tidy/abseil/DurationFactorySc
hwright added inline comments.
Comment at: clang-tidy/abseil/DurationFactoryScaleCheck.cpp:74
+ case DurationScale::Minutes:
+if (Multiplier == 60.0)
+ return DurationScale::Hours;
hokein wrote:
> we are comparing with floats, I think we should use some
aaron.ballman added inline comments.
Comment at: clang-tidy/abseil/DurationFactoryScaleCheck.cpp:36
+GetScaleForFactory(llvm::StringRef FactoryName) {
+ static const auto *ScaleMap =
+ new std::unordered_map(
hokein wrote:
> aaron.ballman wrote:
> > hwright
hokein added inline comments.
Comment at: clang-tidy/abseil/DurationFactoryScaleCheck.cpp:36
+GetScaleForFactory(llvm::StringRef FactoryName) {
+ static const auto *ScaleMap =
+ new std::unordered_map(
aaron.ballman wrote:
> hwright wrote:
> > Eugene.Zelenk
aaron.ballman added inline comments.
Comment at: clang-tidy/abseil/DurationFactoryScaleCheck.cpp:36
+GetScaleForFactory(llvm::StringRef FactoryName) {
+ static const auto *ScaleMap =
+ new std::unordered_map(
hwright wrote:
> Eugene.Zelenko wrote:
> > This
Eugene.Zelenko added inline comments.
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 ze
hwright added inline comments.
Comment at: clang-tidy/abseil/DurationFactoryScaleCheck.cpp:36
+GetScaleForFactory(llvm::StringRef FactoryName) {
+ static const auto *ScaleMap =
+ new std::unordered_map(
Eugene.Zelenko wrote:
> This will cause memory leaks,
hwright updated this revision to Diff 173166.
hwright marked 4 inline comments as done.
hwright added a comment.
Address reviewer comments
https://reviews.llvm.org/D54246
Files:
clang-tidy/abseil/AbseilTidyModule.cpp
clang-tidy/abseil/CMakeLists.txt
clang-tidy/abseil/DurationFactoryScaleC
Eugene.Zelenko added inline comments.
Comment at: clang-tidy/abseil/DurationFactoryScaleCheck.cpp:36
+GetScaleForFactory(llvm::StringRef FactoryName) {
+ static const auto *ScaleMap =
+ new std::unordered_map(
This will cause memory leaks, so may be unique_
22 matches
Mail list logo
