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

Hyrum Wright via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Dec 18 08:04:01 PST 2018


hwright 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));
----------------
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).


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

https://reviews.llvm.org/D55784





More information about the cfe-commits mailing list