[clang-tidy] cppcoreguidelines Narrowing Conversions Check: detect narrowing conversions involving typedefs

[gridaphobe]

Summary

The check 'cppcoreguidelines-narrowing-conversions' does not detect conversions
involving typedef. This notably includes the standard fixed-width integer types
like int32_t, uint64_t, etc.

[gchatelet]

Thx for the patch!

[lebedev.ri]

Whoa, that's a fun bug :)

[aaron.ballman]

LGTM aside from some small nits, thank you for the fix!

clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions.cpp

346

351

[gridaphobe]

Add a couple passing tests involving typedefs.

[gridaphobe]

I've added the non-narrowing tests, are we good to merge?

[aaron.ballman]

In D91975#2436212, @gridaphobe wrote:

I've added the non-narrowing tests, are we good to merge?

Yes, this still LGTM! Thank you! Do you need someone to commit on your behalf?

[gridaphobe]

Yes please! I don't have commit rights.

[aaron.ballman]

In D91975#2440426, @gridaphobe wrote:

Yes please! I don't have commit rights.

I've commit on your behalf in c6348e8c95ee1eaa9dd2322b278def7a4127ff26, thank you for the patch!

clang-tools-extra/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp

const auto IsCeilFloorCallExpr = expr(callExpr(callee(functionDecl(
       hasAnyName("::ceil", "::std::ceil", "::floor", "::std::floor")))));
 
   // Casts:
   //   i = 0.5;
   //   void f(int); f(0.5);
   Finder->addMatcher(
       traverse(
           ast_type_traits::TK_AsIs,
-          implicitCastExpr(hasImplicitDestinationType(builtinType()),
-                           hasSourceExpression(hasType(builtinType())),
+          implicitCastExpr(hasImplicitDestinationType(
+                               hasUnqualifiedDesugaredType(builtinType())),
+                           hasSourceExpression(hasType(
+                               hasUnqualifiedDesugaredType(builtinType()))),
                            unless(hasSourceExpression(IsCeilFloorCallExpr)),
                            unless(hasParent(castExpr())),
                            unless(isInTemplateInstantiation()))
               .bind("cast")),
       this);
 
   // Binary operators:
   //   i += 0.5;
-  Finder->addMatcher(binaryOperator(isAssignmentOperator(),
-                                    hasLHS(expr(hasType(builtinType()))),
-                                    hasRHS(expr(hasType(builtinType()))),
-                                    unless(hasRHS(IsCeilFloorCallExpr)),
-                                    unless(isInTemplateInstantiation()),
-                                    // The `=` case generates an implicit cast
-                                    // which is covered by the previous matcher.
-                                    unless(hasOperatorName("=")))
-                         .bind("binary_op"),
-                     this);
+  Finder->addMatcher(
+      binaryOperator(
+          isAssignmentOperator(),
+          hasLHS(expr(hasType(hasUnqualifiedDesugaredType(builtinType())))),
+          hasRHS(expr(hasType(hasUnqualifiedDesugaredType(builtinType())))),
 }
 
 static const BuiltinType *getBuiltinType(const Expr &E) {
   return E.getType().getCanonicalType().getTypePtr()->getAs<BuiltinType>();
 }
 
 static QualType getUnqualifiedType(const Expr &E) {
   return E.getType().getUnqualifiedType();

clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions.cpp

 void macro_context() {
   int i = 0;
   DERP(i, 2);
   DERP(i, .5f);
   DERP(i, .5);
   DERP(i, .5l);
 }
 
+// We understand typedefs.
+void typedef_context() {
+  typedef long long myint64_t;
+  int i;
+  myint64_t i64;
+
+  i64 = i64; // Okay, no conversion.
+  i64 = i;   // Okay, no narrowing.
+
+  i = i64;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: narrowing conversion from 'myint64_t' (aka 'long long') to signed type 'int' is implementation-defined [cppcoreguidelines-narrowing-conversions]
+}
+
 } // namespace floats