Page MenuHomePhabricator

[clang-tidy] Extend bugprone-sizeof-expression to check sizeof(pointers to structures)
ClosedPublic

Authored by baloghadamsoftware on Apr 29 2019, 6:38 AM.

Diff Detail

Repository
rL LLVM

Event Timeline

Herald added a project: Restricted Project. · View Herald TranscriptApr 29 2019, 6:38 AM
whisperity added inline comments.Apr 29 2019, 6:45 AM
test/clang-tidy/bugprone-sizeof-expression.cpp
196 ↗(On Diff #197101)

While I trust Clang and the matchers to unroll the type and still match, I'd prefer also adding a test case for

typedef TMyStruct* PMyStruct2;

or somesuch.

And perhaps a "copy" of these cases where they come from template arguments, in case the checker can also warn for that?

231 ↗(On Diff #197101)

Why is this printed at sizeof(A*)? Do we not print the name of the actual type used in the expression?

Aside from the testing request, this LGTM.

test/clang-tidy/bugprone-sizeof-expression.cpp
231 ↗(On Diff #197101)

The original check did this, so it's not really new behavior here. I don't recall the rationale off the top of my head, but I'd guess it's because the type is not really salient, but the fact that the type is a pointer is.

New tests added.

baloghadamsoftware marked 2 inline comments as done.May 3 2019, 5:32 AM
baloghadamsoftware added inline comments.
test/clang-tidy/bugprone-sizeof-expression.cpp
196 ↗(On Diff #197101)

I added the new test, but templates are outside the scope of this particular patch. They should be tested (and fixed if the tests fail) in another patch.

baloghadamsoftware marked an inline comment as done.May 5 2019, 10:43 PM
This revision is now accepted and ready to land.May 6 2019, 11:21 AM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptMay 6 2019, 11:15 PM