lebedev.ri added a comment. In D61827#1499347 <https://reviews.llvm.org/D61827#1499347>, @hintonda wrote:
> In D61827#1499335 <https://reviews.llvm.org/D61827#1499335>, @lebedev.ri > wrote: > > > In D61827#1499333 <https://reviews.llvm.org/D61827#1499333>, @hintonda > > wrote: > > > > > When I asked for a test above, I understood you to say you couldn't > > > provide one, but If I misunderstood, by all means, please add the test. > > > > > > Please do note that i have provided a testcase (godbolt link) in my very > > first comment, and quoted that line when replying the previous time. > > (Granted, that loop is not in a correct form for openmp, but the point > > being, the current check does not diagnose it either) > > > I'm really not sure I understand what you are trying to say. > Are you saying this patch is a regression? Not in general, no. This is most certainly an improvement for normal C++ code. > By "false positive", do you mean that diagnoses something it shouldn't change > and creates an erroneous fixit? For normal C++ - not that i know of, can't comment. For loops that are OpenMP loops - yes, this will be a false-positive with erroneous, compilation-breaking, fixit. (To be noted, the existing fix-its in this check already are that way.) But as i said, this is only "for your information", only to be added to docs (if it isn't there already). As i have said, this can not be reliably avoided, and i don't believe a partial avoidance will be good. > Of that it ignores something it should fix? That is called false-negative. No, this isn't the case here. Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61827/new/ https://reviews.llvm.org/D61827 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
