This is an archive of the discontinued LLVM Phabricator instance.

[clang-tidy] extend misc-misplaced-const to detect using besides typedef
ClosedPublic

Authored by AlexanderLanin on Jan 7 2020, 3:52 PM.

Details

Diff Detail

Event Timeline

AlexanderLanin created this revision.Jan 7 2020, 3:52 PM
Herald added a project: Restricted Project. · View Herald TranscriptJan 7 2020, 3:52 PM

looking at this at least many are indeed valid results:


I guess false positives could be reduced by eliminating those cases where the variables are (intentionally) modified.

FIX-IT isn't quite that obvious. Some options:

  • look for other typedef which contains the same as const
  • create new typedef/using
  • remove "*" from typedef and adjust all usage accordingly. Implies removing "Ptr" suffix.
clang-tools-extra/test/clang-tidy/checkers/misc-misplaced-const.cpp
2

is this good practice? I didn't want to duplicate everything here.

32

not sure how to remove the regex here without duplicating everything

Eugene.Zelenko added inline comments.
clang-tools-extra/clang-tidy/misc/MisplacedConstCheck.cpp
64

llvm_unreachable

Eugene.Zelenko added a project: Restricted Project.
AlexanderLanin marked an inline comment as done.

Updated revision with llvm_unreachable

aaron.ballman accepted this revision.Jan 16 2020, 6:20 AM

LGTM!

FIX-IT isn't quite that obvious. Some options:

  • look for other typedef which contains the same as const
  • create new typedef/using
  • remove "*" from typedef and adjust all usage accordingly. Implies removing "Ptr" suffix.

I think we can punt on the fixit for now as this is a good incremental improvement. Another possible option is to remove the const from the declaration using the typedef (there may be situations where you cannot add a new typedef or modify the existing one because the typedef is in a system header).

clang-tools-extra/test/clang-tidy/checkers/misc-misplaced-const.cpp
2

Yes, this is a good way to reduce duplication.

32

I think the regex is reasonable enough.

This revision is now accepted and ready to land.Jan 16 2020, 6:20 AM

Thanks for the review!

Could someone commit this? As I can not.
Alexander Lanin <alex@lanin.de>

ping

Could someone commit this? As I can not.
Alexander Lanin <alex@lanin.de>

aaron.ballman closed this revision.Jan 22 2020, 5:46 AM

I've commit on your behalf in ecc7dae50c41bc8a129a158ecf0ae0270126505c. Sorry about the delay in committing and thank you for the patch!

Updated misc-misplaced-const.c to reflect new output and fixed one wrong col in misc-misplaced-const.cpp - I really do not understand how that happened.
Rebased and verified with check-clang-tools.

Updated misc-misplaced-const.c to reflect new output and fixed one wrong col in misc-misplaced-const.cpp - I really do not understand how that happened.
Rebased and verified with check-clang-tools.

Thanks! I've committed on your behalf in 84c5f196370065388779cd96d033c84d31031543