[PATCH] D54141: [clang-tidy] add deduplication support for run-clang-tidy.py

JonasToth abandoned this revision. JonasToth added a comment. Herald added a subscriber: mgehre. won't happen anymore realistically. Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54141/new/ https://reviews.llvm.org/D54141 __

[PATCH] D54141: [clang-tidy] add deduplication support for run-clang-tidy.py

aaron.ballman added a comment. In D54141#1289980 , @hokein wrote: > >> If you're suggesting proceeding with this regex based solution, I > > > > don't think that's a good idea. Why commit a hack which people will object > > to ever removing? Just see if

[PATCH] D54141: [clang-tidy] add deduplication support for run-clang-tidy.py

JonasToth added a comment. In D54141#1362924 , @MyDeveloperDay wrote: > > LLVM is very chatty as well, I don't consider LLVM to be a bad code-base. > > Take readability-braces-around-statements for example. > > Do we need a llvm-elide-braces-for-small-st

[PATCH] D54141: [clang-tidy] add deduplication support for run-clang-tidy.py

MyDeveloperDay added a comment. > LLVM is very chatty as well, I don't consider LLVM to be a bad code-base. > Take readability-braces-around-statements for example. Do we need a llvm-elide-braces-for-small-statements? This would make a great pre-review check Repository: rCTE Clang Tools Ext

[PATCH] D54141: [clang-tidy] add deduplication support for run-clang-tidy.py

JonasToth updated this revision to Diff 182359. JonasToth added a comment. Herald added a reviewer: serge-sans-paille. - make the script more useable in my buildbot context - reduce the test-files - fix unicode issues I encountered while using Repository: rCTE Clang Tools Extra CHANGES SINCE

[PATCH] D54141: [clang-tidy] add deduplication support for run-clang-tidy.py

JonasToth updated this revision to Diff 175459. JonasToth added a comment. - after countless attempts of fixing the unicode problem, it is finally done. - Remove all unnecessary whitespace - remove the `xxx warnings generated.` as well This setup now runs in my BB and is a good approximation (for

[PATCH] D54141: [clang-tidy] add deduplication support for run-clang-tidy.py

JonasToth updated this revision to Diff 175063. JonasToth added a comment. Push some fixes i did while working with this script, for reference of others potentially looking at this patch. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D54141 Files: clang-tidy/tool/run-clang-ti

[PATCH] D54141: [clang-tidy] add deduplication support for run-clang-tidy.py

JonasToth added a comment. > Could you please explain your motivation of catching clang-tidy stdout? > `--export-fixes` emits everything of `diagnostic` to YAML even the > `diagnostic` doesn't have fixes. I guess the reason is that you want code > snippets that you could show to users? If so, I

[PATCH] D54141: [clang-tidy] add deduplication support for run-clang-tidy.py

steveire added a comment. @hokein you and I seem to be making the same proposal :) Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D54141 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/li

[PATCH] D54141: [clang-tidy] add deduplication support for run-clang-tidy.py

hokein added a comment. >> If you're suggesting proceeding with this regex based solution, I > > don't think that's a good idea. Why commit a hack which people will object to > ever removing? Just see if we can do the right thing instead. +1, my main concern is the complexity of the patch and m

[PATCH] D54141: [clang-tidy] add deduplication support for run-clang-tidy.py

steveire added a comment. In https://reviews.llvm.org/D54141#1289326, @JonasToth wrote: > > That said, would you agree to have the parser-based deduplication as an > developer-only optin solution for now? :) If you're suggesting proceeding with this regex based solution, I don't think that

[PATCH] D54141: [clang-tidy] add deduplication support for run-clang-tidy.py

JonasToth added a comment. > At least the clang-tidy quiet mode is trivial to implement. Maybe instead of > `--quiet` we could have `--stdout=` where `output_format` can > be one of `none`, `diag`, `yaml` and in the future possibly `json` (requested > here: http://lists.llvm.org/pipermail/cfe-d

[PATCH] D54141: [clang-tidy] add deduplication support for run-clang-tidy.py

steveire added a comment. In https://reviews.llvm.org/D54141#1288930, @JonasToth wrote: > > Do you understand the proposal now? > > Yes better, I was under the impression that `clang-apply-replaments` is run > on the end and the YAMLs are kept until then. Now its clear. > I assume `--issue-diag

[PATCH] D54141: [clang-tidy] add deduplication support for run-clang-tidy.py

JonasToth added a comment. In https://reviews.llvm.org/D54141#1288930, @JonasToth wrote: > > Do you understand the proposal now? > > Yes better, I was under the impression that `clang-apply-replaments` is run > on the end and the YAMLs are kept until then. Now its clear. > I assume `--issue-dia

[PATCH] D54141: [clang-tidy] add deduplication support for run-clang-tidy.py

JonasToth added a comment. In https://reviews.llvm.org/D54141#1288993, @Eugene.Zelenko wrote: > Reducing log file size is good idea, but I think will be also good idea to > count duplicates. This will allow to concentrate clean-up efforts on place > where most of warnings originate. Places th

[PATCH] D54141: [clang-tidy] add deduplication support for run-clang-tidy.py

Eugene.Zelenko added a comment. Reducing log file size is good idea, but I think will be also good idea to count duplicates. This will allow to concentrate clean-up efforts on place where most of warnings originate. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D54141 _

[PATCH] D54141: [clang-tidy] add deduplication support for run-clang-tidy.py

JonasToth added a comment. > Do you understand the proposal now? Yes better, I was under the impression that `clang-apply-replaments` is run on the end and the YAMLs are kept until then. Now its clear. I assume `--issue-diags` produce the same result as the normal diagnostic engine. That could

[PATCH] D54141: [clang-tidy] add deduplication support for run-clang-tidy.py

steveire added a comment. In https://reviews.llvm.org/D54141#1288851, @JonasToth wrote: > > So, running `clang-apply-replacements --issue-diags the_new_file.yaml` > > would issue the warnings/fixit hints by processing the yaml and issuing the > > diagnostics the way clang-tidy would have done (

[PATCH] D54141: [clang-tidy] add deduplication support for run-clang-tidy.py

JonasToth added a comment. > Maybe my suggestion was not clear. The yaml file generated by clang-tidy > contains not only replacements, but all diagnostics, even without a fixit. > > So, running `clang-apply-replacements --issue-diags the_new_file.yaml` would > issue the warnings/fixit hints by

[PATCH] D54141: [clang-tidy] add deduplication support for run-clang-tidy.py

steveire added a comment. In https://reviews.llvm.org/D54141#1288818, @JonasToth wrote: > Thank you for the comment! > > In https://reviews.llvm.org/D54141#1288809, @steveire wrote: > > > This feature seems like a good idea. I started writing it too some months > > ago, but then I changed tactic

[PATCH] D54141: [clang-tidy] add deduplication support for run-clang-tidy.py

JonasToth added a comment. Thank you for the comment! In https://reviews.llvm.org/D54141#1288809, @steveire wrote: > This feature seems like a good idea. I started writing it too some months > ago, but then I changed tactic and worked on distributing the refactor over > the network instead. As

[PATCH] D54141: [clang-tidy] add deduplication support for run-clang-tidy.py

JonasToth added a comment. > - The output of clang-tidy diagnostic is YAML, and YAML is not an > space-efficient format (just for human readability). If you want to save > space further, we might consider using some compressed formats, e.g. > llvm::bitcode. Given the reduced YAML result (5.4MB)

[PATCH] D54141: [clang-tidy] add deduplication support for run-clang-tidy.py

steveire added a comment. This feature seems like a good idea. I started writing it too some months ago, but then I changed tactic and worked on distributing the refactor over the network instead. As far as I know, your deduplication would not work with a distributed environment. However, it s

[PATCH] D54141: [clang-tidy] add deduplication support for run-clang-tidy.py

hokein added a comment. Thanks for the patch and nice improvements. Some initial thoughts: - The output of clang-tidy diagnostic is YAML, and YAML is not an space-efficient format (just for human readability). If you want to save space further, we might consider using some compressed formats,

[PATCH] D54141: [clang-tidy] add deduplication support for run-clang-tidy.py

JonasToth added inline comments. Comment at: clang-tidy/tool/run_clang_tidy.py:1 +run-clang-tidy.py This simlink is required for my unittests, I don't know how to add the added tests in the `lit` test-suite so there is no change there yet. A bit of guidance the

[PATCH] D54141: [clang-tidy] add deduplication support for run-clang-tidy.py

JonasToth updated this revision to Diff 172726. JonasToth added a comment. - spurious change in my git Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D54141 Files: clang-tidy/tool/run-clang-tidy.py clang-tidy/tool/run_clang_tidy.py clang-tidy/tool/test_input/out_csa_cmake.

[PATCH] D54141: [clang-tidy] add deduplication support for run-clang-tidy.py

JonasToth created this revision. JonasToth added reviewers: alexfh, aaron.ballman, hokein, sammccall, lebedev.ri. Herald added subscribers: cfe-commits, xazax.hun, mgorny. `run-clang-tidy.py` is the parallel executor for `clang-tidy`. Due to the common header-inclusion problem in C++/C diagnostics