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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
