Page MenuHomePhabricator

[clang-tidy] Add check 'readability-pointer-type-star-placement'.
AbandonedPublic

Authored by balazske on Feb 12 2021, 7:20 AM.

Details

Summary

This check should warn if '*' character at pointer declarations is not aligned to right side.
I know that clang-format can do these changes but if the goal is not to reformat the whole code
this check can be useful.

Diff Detail

Unit TestsFailed

TimeTest
210 msx64 debian > Clang Tools.clang-tidy/checkers::readability-pointer-type-star-placement.cpp
Script: -- : 'RUN: at line 1'; /usr/bin/python3.8 /mnt/disks/ssd0/agent/llvm-project/clang-tools-extra/test/../test/clang-tidy/check_clang_tidy.py /mnt/disks/ssd0/agent/llvm-project/clang-tools-extra/test/clang-tidy/checkers/readability-pointer-type-star-placement.cpp readability-pointer-type-star-placement /mnt/disks/ssd0/agent/llvm-project/build/tools/clang/tools/extra/test/clang-tidy/checkers/Output/readability-pointer-type-star-placement.cpp.tmp
440 msx64 windows > Clang Tools.clang-tidy/checkers::readability-pointer-type-star-placement.cpp
Script: -- : 'RUN: at line 1'; C:/Python38/python.exe C:/ws/w16-1/llvm-project/premerge-checks/clang-tools-extra/test/../test\clang-tidy\check_clang_tidy.py C:\ws\w16-1\llvm-project\premerge-checks\clang-tools-extra\test\clang-tidy\checkers\readability-pointer-type-star-placement.cpp readability-pointer-type-star-placement C:\ws\w16-1\llvm-project\premerge-checks\build\tools\clang\tools\extra\test\clang-tidy\checkers\Output\readability-pointer-type-star-placement.cpp.tmp

Event Timeline

balazske created this revision.Feb 12 2021, 7:20 AM
balazske requested review of this revision.Feb 12 2021, 7:20 AM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 12 2021, 7:20 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript
Eugene.Zelenko added a project: Restricted Project.

I think it'll be much more reasonable to use clang-format and filter its output to select only changes with stars in build rule. clang-format-diff is another method to introduce formatting changes gradually.

clang-tools-extra/clang-tidy/readability/PointerTypeStarPlacementCheck.cpp
1

Two lines must be joined.

clang-tools-extra/clang-tidy/readability/PointerTypeStarPlacementCheck.h
19

Some description should be provided, or empty comment removed.

clang-tools-extra/docs/ReleaseNotes.rst
70

Merge artifact?

clang-tools-extra/docs/clang-tidy/checks/readability-pointer-type-star-placement.rst
10

Please remove empty line.

28

Copy-paste error?

I don't see the use case you suggest as a strong enough motivation for this, clang-format does this job very well and we shouldn't be reinventing the wheel here.

I don't see the use case you suggest as a strong enough motivation for this, clang-format does this job very well and we shouldn't be reinventing the wheel here.

Can you turn all mechanisms of clang-format off, except the pointer placement rule?


Several coding guidelines and projects put the pointer to the left, not to the right. So even if this check goes in, there should definitely be an option for which side you want to enforce and fix for...

I think now that not checkers are the best solution for this problem. There is already a separate tool for formatting. And we do not want to have a checker for every other formatting rule too. The clang-format could be improved to support selective formatting (apply only a part of the formatting rules). (And it has weaknesses that may prevent users from using it.) It seems difficult to filter out changes related to pointer declarations (these rules are not line-based and multiplications should be excluded) after the formatting.

balazske abandoned this revision.Mar 1 2021, 4:02 AM

Checks about code formatting are likely not to be accepted. Formatting can be done in external tool and is not subject for checks. The difference is that it is not possible to format separately for one rule only (with clang-format). But the better option should be to format the whole code instead of parts of it.