hwright marked 4 inline comments as done.
hwright added inline comments.
================
Comment at: clang-tidy/abseil/DurationFactoryFloatCheck.cpp:61
+
+ if (SimpleArg) {
diag(MatchedCall->getBeginLoc(),
----------------
JonasToth wrote:
> hwright wrote:
> > JonasToth wrote:
> > > hwright wrote:
> > > > JonasToth wrote:
> > > > > The diagnostic is not printed if for some reason the fixit was not
> > > > > creatable. I think that the warning could still be emitted (`auto
> > > > > Diag = diag(...); if (Fix) Diag << Fixit::-...`)
> > > > I'm not sure under which conditions you'd expect this to be an issue.
> > > > Could you give me an example?
> > > >
> > > > My expectation is that if we don't get a value back in `SimpleArg`, we
> > > > don't have anything to change, so there wouldn't be a warning to emit.
> > > I don't expect this to fail, failure there would mean a bug i guess.
> > > Having bugs is somewhat expected :)
> > > And that would be our way to find the bug, because some user reports that
> > > there is no transformation done, that is my motivation behind that.
> > >
> > > The warning itself should be correct, otherwise the matcher does not
> > > work, right? This would just be precaution.
> > I guess what I'm saying is that I'm not sure what kind of warning we'd give
> > if we weren't able to offer a fix. The optionality here means that we
> > found a result which was already as simple as we can make it, so there's no
> > reason to bother the user.
> Lets say the result comes out of arithmetic and then there is a comparison
> (would be good test btw :)), the warning is still valueable, even if the
> transformation fails for some reason. The transformation could fail as well,
> because macros interfere or so. Past experience tells me, there are enough
> language constructs to break everything in weird combinations :)
I can buy that, but I'm still having trouble seeing the specifics. Do you have
a concrete example?
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D54737/new/
https://reviews.llvm.org/D54737
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
