steakhal accepted this revision. steakhal added a comment. This revision is now accepted and ready to land.
In D95860#2539453 <https://reviews.llvm.org/D95860#2539453>, @balazske wrote: > Probably it is not worth to find a better solution. In the case of > CloneChecker the range starts and ends on different line that is not possible > to print on a single line in the output. > This code could work: > [code...] > With this solution the following result is printed: > > llvm-project/clang/test/Analysis/copypaste/clone-end-in-other-file.cpp:9:3: > warning: Duplicate code detected [alpha.clone.CloneChecker] > if (i == 10) // expected-warning{{Duplicate code detected}} > ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > > llvm-project/clang/test/Analysis/copypaste/clone-end-in-other-file.cpp:11:3: > note: Similar code here > if (i == 10) // expected-note{{Similar code here}} > ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > 1 warning generated. > > This is not better than the result with the existing code: > > llvm-project/clang/test/Analysis/copypaste/clone-end-in-other-file.cpp:9:3: > warning: Duplicate code detected [alpha.clone.CloneChecker] > if (i == 10) // expected-warning{{Duplicate code detected}} > ^ > > llvm-project/clang/test/Analysis/copypaste/clone-end-in-other-file.cpp:11:3: > note: Similar code here > if (i == 10) // expected-note{{Similar code here}} > ^ > 1 warning generated. I think you are right. There is no gain in complicating this. I don't think there is anything that we can do about this. Your original patch, simply ignoring the `end` if that is invalid or from a different file is fine by me. Thank you for going the extra mile! In D95860#2538845 <https://reviews.llvm.org/D95860#2538845>, @balazske wrote: > I realized that the added test cases show different problems: One is when one > of `Begin` or `End` is invalid, other if the FileID's are different. Probably > we can find a better solution for the second problem. So the patch needs to > be splitted and the case with the **crash-diagnostic-renderer.cpp** goes into > the first part. I don't mind committing this in one go. However, whichever you choose, please update the revision summary according to the commit message. You should definitely describe both of these issues you found. Thanks for your effort. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95860/new/ https://reviews.llvm.org/D95860 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
