[PATCH] D48866: [clang-tidy] Add incorrect-pointer-cast checker

[Balázs Benics via Phabricator via cfe-commits]

steakhal marked 8 inline comments as done.
steakhal added inline comments.


================
Comment at: clang-tools-extra/clang-tidy/bugprone/IncorrectPointerCastCheck.cpp:69-70
+         "cast from %0 to %1 may lead to access memory based on invalid "
+         "memory layout; pointed to type is strictly aligned than the "
+         "allocated type")
+        << SrcPointedType << DestPointedType;
----------------
lebedev.ri wrote:
> "*more* strictly aligned" perhaps?
> Similarly, specify the actual values.
Which metrics should I use? Bits or char units? For now I will stick to bits.


================
Comment at: clang-tools-extra/clang-tidy/bugprone/IncorrectPointerCastCheck.cpp:98-107
+  } else if (WarnForDifferentSignedness &&
+             ((SrcPointedType->isSignedIntegerType() &&
+               DestPointedType->isUnsignedIntegerType()) ||
+              (SrcPointedType->isUnsignedIntegerType() &&
+               DestPointedType->isSignedIntegerType()))) {
+    diag(castExpr->getBeginLoc(),
+         "cast from %0 to %1 may lead to access memory based on invalid "
----------------
lebedev.ri wrote:
> What does this mean?
> How can signedness affect memory layout?
You are right, signess difference is **explicitly** allowed in bitcasts. It has nothing to do with the actual memory layout. I thought it might be useful for detecting those cases too. It should be in a different checker in the future.
For now I've removed the related code from this checker.


================
Comment at: clang-tools-extra/clang-tidy/bugprone/IncorrectPointerCastCheck.h:18
+
+/// Warns for cases when pointer is cast and the pointed to type is incompatible
+/// with allocated memory area type. This may lead to access memory based on
----------------
lebedev.ri wrote:
> is cast*ed*
> s/pointed to/new/
Casted fixed now.

What do you think about **destination type** and **original type** instead of **new type**?


================
Comment at: clang-tools-extra/clang-tidy/bugprone/IncorrectPointerCastCheck.h:19
+/// Warns for cases when pointer is cast and the pointed to type is incompatible
+/// with allocated memory area type. This may lead to access memory based on
+/// invalid memory layout.
----------------
lebedev.ri wrote:
> How do you know anything about the actual `allocated memory area type`.
> You probably want to talk about the original type before cast.
You are right, fixed.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D48866/new/

https://reviews.llvm.org/D48866