[PATCH] D55784: [clang-tidy] Update abseil-duration-comparison to handle macro arguments

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.


================
Comment at: test/clang-tidy/abseil-duration-comparison.cpp:131
+  // We should still transform the expression inside this macro invocation
+#define VALUE_IF(v, e) v ? (e) : 0
+  int a = VALUE_IF(1, 5 > absl::ToDoubleSeconds(d1));
----------------
hwright wrote:
> aaron.ballman wrote:
> > Can you add another example that has one more level of macro arg expansion? e.g.,
> > ```
> > #define VALUE_IF_2(e) (e)
> > #define VALUE_IF(v, e) v ? VALUE_IF_2(e) : VALUE_IF_2(0)
> > int a = VALUE_IF(1, 5 > absl::ToDoubleSeconds(d1));
> > ```
> > Which makes me wonder -- is this transformation valid in all cases? For instance, how do the fixes look with this abomination?
> > ```
> > #define VALUE_IF_2(e) (e)
> > #define VALUE_IF(v, e, type) (v ? VALUE_IF_2(absl::To##type##Seconds(e)) : 0)
> > int a = VALUE_IF(1, d1, Double);
> > ```
> > I would expect these shenanigans to be vanishingly rare, so if this turns out to be a bad FIXIT you may want to document it as a known deficiency if you don't feel like handling such a case.
> I've added both these as test cases.
> 
> The first one works as expected, and I'm pleasantly surprised to see that the second one does as well (which is to say that it doesn't transform).
Great, thank you!


================
Comment at: test/clang-tidy/abseil-duration-comparison.cpp:146
+#define VALUE_IF_2(e) (e)
+#define VALUE_IF(v, e, type) (v ? VALUE_IF_2(absl::To##type##Seconds(e)) : 0)
+  int a3 = VALUE_IF(1, d1, Double);
----------------
What if this is changed to:
```
#define VALUE_IF(v, e, type) (v ? (5 > VALUE_IF_2(absl::To##type##Seconds(e))) : 0)
```


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

https://reviews.llvm.org/D55784