from:"Eric Seidel via Phabricator via cfe\-commits"

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

2020-12-08 Thread Eric Seidel via Phabricator via cfe-commits

gridaphobe added a comment.

Yes please! I don't have commit rights.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91975

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D91975: clang-tidy: detect narrowing conversions involving typedefs

2020-11-23 Thread Eric Seidel via Phabricator via cfe-commits

gridaphobe created this revision.
Herald added subscribers: cfe-commits, kbarton, nemanjai.
Herald added a project: clang.
gridaphobe requested review of this revision.

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.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D91975

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


Index: 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions.cpp
===
--- 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions.cpp
+++ 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions.cpp
@@ -343,4 +343,14 @@
   DERP(i, .5l);
 }
 
+// We understand typedefs
+void typedef_context() {
+  typedef long long myint64_t;
+  int i;
+  myint64_t i64;
+
+  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
Index: 
clang-tools-extra/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp
===
--- clang-tools-extra/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp
+++ clang-tools-extra/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp
@@ -48,8 +48,10 @@
   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()))
@@ -58,16 +60,18 @@
 
   // 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(),
+  unless(hasRHS(IsCeilFloorCallExpr)),
+  unless(isInTemplateInstantiation()),
+  // The `=` case generates an implicit cast
+  // which is covered by the previous matcher.
+  unless(hasOperatorName("=")))
+  .bind("binary_op"),
+  this);
 }
 
 static const BuiltinType *getBuiltinType(const Expr &E) {


Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions.cpp
@@ -343,4 +343,14 @@
   DERP(i, .5l);
 }
 
+// We understand typedefs
+void typedef_context() {
+  typedef long long myint64_t;
+  int i;
+  myint64_t i64;
+
+  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
Index: clang-tools-extra/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp
===
--- clang-tools-extra/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp
+++ clang-tools-extra/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp
@@ -48,8 +48,10 @@
   Finder->addMatcher(
   traverse(
   ast_type_traits::TK_AsIs,
-  implicitCastExpr(hasImplicitDestinationType(builtinType()),
-   hasSourceExpression(hasType(builtinType())),
+  im

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

2020-11-24 Thread Eric Seidel via Phabricator via cfe-commits

gridaphobe updated this revision to Diff 307348.
gridaphobe added a comment.

Add a couple passing tests involving typedefs.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91975

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


Index: 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions.cpp
===
--- 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions.cpp
+++ 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions.cpp
@@ -343,4 +343,17 @@
   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
Index: 
clang-tools-extra/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp
===
--- clang-tools-extra/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp
+++ clang-tools-extra/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp
@@ -48,8 +48,10 @@
   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()))
@@ -58,16 +60,18 @@
 
   // 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(),
+  unless(hasRHS(IsCeilFloorCallExpr)),
+  unless(isInTemplateInstantiation()),
+  // The `=` case generates an implicit cast
+  // which is covered by the previous matcher.
+  unless(hasOperatorName("=")))
+  .bind("binary_op"),
+  this);
 }
 
 static const BuiltinType *getBuiltinType(const Expr &E) {


Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions.cpp
@@ -343,4 +343,17 @@
   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
Index: clang-tools-extra/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp
===
--- clang-tools-extra/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp
+++ clang-tools-extra/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp
@@ -48,8 +48,10 @@
   Finder->addMatcher(
   traverse(
   ast_type_traits::TK_AsIs,
-  implicitCastExpr(hasImplicitDestinationType(builtinType()),
-   hasSourceExpression(hasType(builtinType())),
+  implicitCastEx

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

2020-12-06 Thread Eric Seidel via Phabricator via cfe-commits

gridaphobe marked 2 inline comments as done.
gridaphobe added a comment.
Herald added a subscriber: shchenz.

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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91975

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[PATCH] D91975: clang-tidy: detect narrowing conversions involving typedefs

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

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

4 matches

Site Navigation

Mail list logo

Footer information