[PATCH] D38455: [clang-tidy] new cppcoreguidelines-narrowing-conversions check.

[Jonas Toth via Phabricator via llvm-commits]

JonasToth added inline comments.


================
Comment at: clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp:32
+  Finder->addMatcher(
+      implicitCastExpr(hasImplicitDestinationType(isInteger()),
+                       hasSourceExpression(IsFloatExpr),
----------------
Do you plan to check for `double` -> `float` conversion, too?

Having a limited scope for now is ok, but a FIXME would be nice.


================
Comment at: clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp:34
+                       hasSourceExpression(IsFloatExpr),
+                       unless(hasParent(castExpr()))).bind("cast"),
+      this);
----------------
Given C++ is weird sometimes: Is there a way that a cast might imply an narrowing conversion?

```
double value = 0.4;
int i = static_cast<float>(value);
```

or 

```
void some_function(int);

double d = 0.42;
some_function(static_cast<float>(d));
```
would come to mind.

Nice to have, IMHO not necessary.


================
Comment at: clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp:41
+      binaryOperator(
+          anyOf(hasOperatorName("+="), hasOperatorName("-=")),
+          hasLHS(hasType(isInteger())),
----------------
I would really like to add all other calc-and-assign operations. At least "*=" and "/=".


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D38455