[PATCH] D50250: [clang][ubsan] Implicit Conversion Sanitizer - integer sign change - clang part

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a project: clang.
lebedev.ri added inline comments.


================
Comment at: lib/CodeGen/CGExprScalar.cpp:1008
+  // We ignore conversions to/from pointer and/or bool.
+  if (!(SrcType->isIntegerType() && DstType->isIntegerType()))
+    return;
----------------
erichkeane wrote:
> lebedev.ri wrote:
> > erichkeane wrote:
> > > I'd rather !SrcType->isInt || !DestType->isInt
> > This i'd prefer to keep this as-is, since this is copied verbatim from `ScalarExprEmitter::EmitIntegerTruncationCheck()`.
> If so much is being copied from EmitIntegerTruncationCheck, perhaps the two of these need to be the same function with an option/check on the sanitizer option on which it should do?
I agree that code duplication is bad, but i'm not sure that inlining
both of these functions into an one huge one is the right solution.
The amount of actually duplicated code is somewhat small - one early-return
for non-integer types, and an assert that the llvm type is integer..


Repository:
  rC Clang

https://reviews.llvm.org/D50250