From bea5681cc1dcba51dc2bd1b9198a96bbb3cd1094 Mon Sep 17 00:00:00 2001 From: Andreas Fertig Date: Wed, 20 May 2020 16:58:51 +0200 Subject: [PATCH] Fixed #313: Show `CK_NoOp` correctly. `CK_NoOp` stands for adding `const` or other qualifiers. Show this conversion, if `--show-all-implicit-casts` is active. This patch enables showing implicit casts for member-function calls as well. --- CodeGenerator.cpp | 28 ++++++++++++++++++++-------- tests/Issue223_2.expect | 2 +- tests/Issue313.cpp | 13 +++++++++++++ tests/Issue313.expect | 29 +++++++++++++++++++++++++++++ tests/Issue313_2.cpp | 14 ++++++++++++++ tests/Issue313_2.expect | 28 ++++++++++++++++++++++++++++ 6 files changed, 105 insertions(+), 9 deletions(-) create mode 100644 tests/Issue313.cpp create mode 100644 tests/Issue313.expect create mode 100644 tests/Issue313_2.cpp create mode 100644 tests/Issue313_2.expect diff --git a/CodeGenerator.cpp b/CodeGenerator.cpp index 8cc1b2f0..b9562d51 100644 --- a/CodeGenerator.cpp +++ b/CodeGenerator.cpp @@ -1382,9 +1382,6 @@ void CodeGenerator::InsertArg(const ImplicitCastExpr* stmt) case CastKind::CK_FloatingCast: [[fallthrough]]; case CastKind::CK_IntegralToFloating: [[fallthrough]]; case CastKind::CK_FloatingToIntegral: [[fallthrough]]; - /* these are implicit conversions. We get them right, but they may end up in a compiler internal type, - * which leads to compiler errors */ - // case CastKind::CK_NoOp: case CastKind::CK_NonAtomicToAtomic: return true; default: // Show this casts only if ShowAllImplicitCasts is turned on. @@ -1392,6 +1389,9 @@ void CodeGenerator::InsertArg(const ImplicitCastExpr* stmt) switch(kind) { case CastKind::CK_NullToPointer: [[fallthrough]]; case CastKind::CK_NullToMemberPointer: [[fallthrough]]; + /* these are implicit conversions. We get them right, but they may end up in a compiler internal + * type, which leads to compiler errors */ + case CastKind::CK_NoOp: [[fallthrough]]; case CastKind::CK_ArrayToPointerDecay: return true; default: break; } @@ -1406,9 +1406,9 @@ void CodeGenerator::InsertArg(const ImplicitCastExpr* stmt) } else if(isa(subExpr) && hideImplicitCasts) { InsertArg(stmt->IgnoreCasts()); - // If this is part of an explicit cast, for example a CStyleCast ignore it, if ShowAllImplicitCasts is not - // selected - } else if(stmt->isPartOfExplicitCast() && hideImplicitCasts) { + // If this is part of an explicit cast, for example a CStyleCast or static_cast, ignore it, because it belongs + // to the cast written by the user. + } else if(stmt->isPartOfExplicitCast()) { InsertArg(stmt->IgnoreCasts()); } else { @@ -1699,8 +1699,20 @@ void CodeGenerator::InsertArg(const CXXOperatorCallExpr* stmt) const bool isCXXMethod{callee && isa(callee->getDecl())}; if(2 == stmt->getNumArgs()) { - const auto* param1 = dyn_cast_or_null(stmt->getArg(0)->IgnoreImpCasts()); - const auto* param2 = dyn_cast_or_null(stmt->getArg(1)->IgnoreImpCasts()); + auto getArg = [&](unsigned idx) { + const auto* arg = stmt->getArg(idx); + + // In show all casts mode don't filter this. It shows how the compiler adds const to arguments, if the + // argument is non-const but the parameter demands a const object + if(not GetInsightsOptions().ShowAllImplicitCasts) { + arg = arg->IgnoreImpCasts(); + } + + return dyn_cast_or_null(arg); + }; + + const auto* param1 = getArg(0); + const auto* param2 = getArg(1); if(callee && param1 && param2) { diff --git a/tests/Issue223_2.expect b/tests/Issue223_2.expect index 1f699dcf..4d9f4f29 100644 --- a/tests/Issue223_2.expect +++ b/tests/Issue223_2.expect @@ -3,7 +3,7 @@ int main() { - float f = static_cast(static_cast((3.0))); + float f = static_cast((3.0)); return static_cast(f); } diff --git a/tests/Issue313.cpp b/tests/Issue313.cpp new file mode 100644 index 00000000..af3bcafa --- /dev/null +++ b/tests/Issue313.cpp @@ -0,0 +1,13 @@ +// cmdlineinsights:--show-all-implicit-casts + +struct A {}; +struct B { + explicit operator A() { return {}; } +}; + +void DangerousFunc(const A&) {} + +int main() +{ + DangerousFunc(static_cast(B{})); +} diff --git a/tests/Issue313.expect b/tests/Issue313.expect new file mode 100644 index 00000000..b3b35bc0 --- /dev/null +++ b/tests/Issue313.expect @@ -0,0 +1,29 @@ +// cmdlineinsights:--show-all-implicit-casts + +struct A +{ +}; + + +struct B +{ + using retType_5_4 = A; + inline operator retType_5_4 () + { + return {}; + } + +}; + + + +void DangerousFunc(const A &) +{ +} + + +int main() +{ + DangerousFunc(static_cast(static_cast(B{}.operator A()))); +} + diff --git a/tests/Issue313_2.cpp b/tests/Issue313_2.cpp new file mode 100644 index 00000000..0382325f --- /dev/null +++ b/tests/Issue313_2.cpp @@ -0,0 +1,14 @@ +// cmdlineinsights:--show-all-implicit-casts +struct A +{ + bool operator==(const A&) const { return true; } + bool operator!=(const A&) { return true; } +}; + +int main() +{ + A a{}; + + if(a == a) {} // a gets constified as object and parameter + if(a != a) {} // only the parameter gets constified, as != is not const +} diff --git a/tests/Issue313_2.expect b/tests/Issue313_2.expect new file mode 100644 index 00000000..ca4bb86c --- /dev/null +++ b/tests/Issue313_2.expect @@ -0,0 +1,28 @@ +// cmdlineinsights:--show-all-implicit-casts +struct A +{ + inline bool operator==(const A &) const + { + return true; + } + + inline bool operator!=(const A &) + { + return true; + } + +}; + + + +int main() +{ + A a = {}; + if(static_cast(a).operator==(static_cast(a))) { + } + + if(a.operator!=(static_cast(a))) { + } + +} +