On 5/30/20 3:22 PM, Amos Jeffries wrote: > On 20/04/20 2:02 pm, Alex Rousskov wrote: >> Squid sources contain a lot of poorly written, obsolete, and >> inconsistent code that (objectively) complicates development and >> (unfortunately) increases tensions among developers during review. >> >> Some of those problems can be solved using tools that modify sources. >> Clang-tidy is one such tool: https://clang.llvm.org/extra/clang-tidy/ >> It contains 150+ "checks" that can automatically fix some common >> problems by studying the syntax tree produced by the clang compiler.
> In order to switch we should be looking for a tool that improves over > the status-quo. Which is both astyle plus the custom scripts. > > All of the above are high-level abstractions that the existing astyle > tool provides by itself. So no valid reason for changing is visible yet. Amos, you are mixing up two completely different subjects: modernizing code using clang-tidy (this thread) and formatting sources using clang-format (Francesco's effort). In hope to make progress, I am ignoring most comments about astyle. >> https://clang.llvm.org/extra/clang-tidy/checks/list.html >> * performance-... >> >> Clang-tidy has a few checks focusing on performance optimizations. The >> following commit shows a combination of the following four checks: >> performance-trivially-destructible, performance-unnecessary-value-param, >> performance-for-range-copy, performance-move-const-arg >> >> Diff: https://github.com/measurement-factory/squid/commit/1ae5d7c > IMO this is something we should have a Sorry, was this phrase cut prematurely or are you voicing support (in principle) for applying (some of these) performance-related changes? >> * modernize-use-nullptr >> >> Replaces NULL and 0 constants with C++11 nullptr: >> https://clang.llvm.org/extra/clang-tidy/checks/modernize-use-nullptr.html >> >> While replacing most NULLs is possible with a simple script, this check >> may be a better solution because it can safely cover more NULL uses and >> also converts bare 0s which would be impossible using a script. >> >> Diff: https://github.com/measurement-factory/squid/commit/4242604 > As I mentioned earlier when kinkie tried this; I do not like this > particular tool feature. The "upgrade" it does is far too simplistic for > a style polishing update. If the end goal were simply to eradicate NULL > - this would be fine. But the goal of using this tool is to ensure a > good quality formatting style. ... except it is not the goal. modernize-use-nullptr does not change code formatting. > A simplistic s/NULL/nullptr/ leaves quite > a few code lines looking just as ugly as they were with NULL. And, as I mentioned above, modernize-use-nullptr is not simplistic -- nothing else can detect and replace 0s with nullptr. > IMO we would be better off going the scripted way to remove the subset > of cases we can automate and catch the rest with manual edits and in review. I do not see a point of automating ourselves if there is an existing automation that works much better than anything we can do ourselves. > I like a few things the tool does. But so far it seems like something we > want to run across the code occasionally. eg as a Jenkins test job. Yes, or a Semaphore CI job, and/or on-demand. Please clarify regarding performance-* checks above. If we are in agreement regarding modernize-use-override and at least one more check, then I can start working on a polished proposal for those checks (at least) and the overall setup. Cheers, Alex. _______________________________________________ squid-dev mailing list squid-dev@lists.squid-cache.org http://lists.squid-cache.org/listinfo/squid-dev
