This is an archive of the discontinued LLVM Phabricator instance.

[clang-tidy][NFC] Simplify a lot of bugprone-sizeof-expression matchers
ClosedPublic

Authored by njames93 on Apr 30 2021, 4:05 AM.

Details

Summary

There should be a follow up to this for changing the traversal mode, but some of the tests don't like that.

Diff Detail

Event Timeline

njames93 created this revision.Apr 30 2021, 4:05 AM
njames93 requested review of this revision.Apr 30 2021, 4:05 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 30 2021, 4:05 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript
steveire added inline comments.Apr 30 2021, 3:45 PM
clang-tools-extra/clang-tidy/bugprone/SizeofExpressionCheck.cpp
87

A lot of this MR is removal of redundant expr() and qualType(), but mixed in with the diff is changes like those to SizeOfExpr below. This MR would be much easier to review if you broke out the removal of redundant matchers to its own MR (or just commit it as an NFC). That would leave behind the more-complex changes like things like SizeOfExpr.

steveire accepted this revision.May 1 2021, 8:07 AM

LGTM, but I think you could split it into 3 commits with a commit message saying what each is doing. "Simplify a lot of" doesn't say anything specific about what this patch does. It looks like you could split it into

  • "Remove obsolete expr()" (because of the ones in hasLHS and hasRHS)
  • "Simplify anyOf(integerLiteral()...) to integerLiteral(anyOf())"
  • "Use hasArgumentOfType matcher with sizeOfExpr"

I'm not sure if this patch does anything else than those 3.

This revision is now accepted and ready to land.May 1 2021, 8:07 AM
njames93 marked an inline comment as done.May 2 2021, 5:16 AM