hwright marked 2 inline comments as done.
hwright added inline comments.

================
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);
----------------
aaron.ballman wrote:
> hwright wrote:
> > aaron.ballman wrote:
> > > What if this is changed to:
> > > ```
> > > #define VALUE_IF(v, e, type) (v ? (5 > 
> > > VALUE_IF_2(absl::To##type##Seconds(e))) : 0)
> > > ```
> > This also doesn't transform.
> Fantastic, thank you! I wouldn't expect a fix-it there due to the macro 
> shenanigans, but I'm surprised it does not diagnose given that it expands to 
> basically the same thing as `a` and `a2` above: https://godbolt.org/z/D1MvGX 
> (ignore the errors but look at the macro expansion from the diagnostics).
> 
> Granted, this situation is uncommon enough, so feel free to add a FIXME 
> comment to the `a4` test case to say that it probably should be diagnosed 
> without a fix-it hint (assuming you agree).
I actually prefer not to diagnose here, mostly because I'm not sure what value 
we'd actually give without a fix-it hint.  "You are holding it wrong, but we 
can't tell you how to hold it right."

Ultimately, though, I think you are right that the number of cases this occurs 
are vanishingly small, and I'd expect folks doing these kinds of craziness to 
know what they are doing (and take responsibility thereof).  Perhaps that's 
optimistic.


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

Reply via email to