Page MenuHomePhabricator

[clang-tidy] Be more liberal about literal zeroes in abseil checks
ClosedPublic

Authored by hwright on Dec 21 2018, 10:12 AM.

Details

Summary

Previously, we'd only match on literal floating or integral zeroes, but I've now also learned that some users spell that value as int{0} or float{0}, which also need to be matched.

Diff Detail

Repository
rL LLVM

Event Timeline

hwright created this revision.Dec 21 2018, 10:12 AM
lebedev.ri retitled this revision from Be more liberal about literal zeroes in abseil checks to [clang-tidy] Be more liberal about literal zeroes in abseil checks.Dec 21 2018, 10:18 AM
Eugene.Zelenko added inline comments.
test/clang-tidy/abseil-duration-factory-scale.cpp
3 ↗(On Diff #179306)

cstdint, please.

aaron.ballman added inline comments.Dec 21 2018, 12:07 PM
clang-tidy/abseil/DurationRewriter.cpp
110 ↗(On Diff #179306)

Spurious parens can be removed.

test/clang-tidy/abseil-duration-factory-scale.cpp
3 ↗(On Diff #179306)

Rather than write this as an include, I'd rather just do: namespace std { typedef long long int64_t; } using int64_t = std::int64_t; Then there's no question about whether this is using the host stdint.h or the current-clang-stdint.h.

34 ↗(On Diff #179306)

Do you also have users doing something like: absl::Seconds(int{});?

hwright updated this revision to Diff 179335.Dec 21 2018, 12:58 PM
hwright marked 6 inline comments as done.

Add documentation, adjust test.

btw, I think hasInit should probably be moved into the core set of matchers at some point.

clang-tidy/abseil/DurationRewriter.cpp
110 ↗(On Diff #179306)

Done. (Though there are similar parens in the implementation of hasArgument. :)

test/clang-tidy/abseil-duration-factory-scale.cpp
34 ↗(On Diff #179306)

I have not yet seen that.

This revision is now accepted and ready to land.Dec 21 2018, 1:05 PM
This revision was automatically updated to reflect the committed changes.
alexfh added a comment.Mon, Jan 7, 8:14 AM

One random late comment.

clang-tools-extra/trunk/test/clang-tidy/abseil-duration-factory-scale.cpp
38

I'd add more context to the CHECK-FIXES patterns, otherwise they are not very reliable. There's no implicit connection to the line numbers, so any long enough subsequence of lines containing 'absl::ZeroDuration()' would satisfy the test. One way to do that is to declare variables with unique names. Another is to add unique comments.