From c70ac648b53a6a78c5fbf960e575f585b6847e93 Mon Sep 17 00:00:00 2001 From: Andrew Adams Date: Wed, 11 Mar 2020 15:30:20 -0700 Subject: [PATCH 1/2] Turn on some of the bugprone checkers - Calling a grandparent's virtual method instead of the parent - Constructing a string from 0 is UB (in the unevaluated contexts in the LHS of the Generator overrides) - sizeof a pointer is considered a code smell --- .clang-tidy | 2 +- src/AsyncProducers.cpp | 8 ++--- src/CodeGen_OpenCL_Dev.cpp | 2 +- src/CodeGen_OpenGL_Dev.cpp | 4 +-- src/CodeGen_PTX_Dev.cpp | 2 +- src/Generator.h | 66 +++++++++++++++++++------------------- src/RDom.cpp | 4 +-- 7 files changed, 44 insertions(+), 44 deletions(-) diff --git a/.clang-tidy b/.clang-tidy index 019103bdef38..3b491e354d9f 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -1,5 +1,5 @@ --- -Checks: '-*,modernize-use-override,performance-move-const-arg,modernize-use-emplace,performance-noexcept-move-constructor,performance-unnecessary-value-param,performance-unnecessary-copy-initialization,performance-trivially-destructible' +Checks: '-*,modernize-use-override,performance-move-const-arg,modernize-use-emplace,performance-noexcept-move-constructor,performance-unnecessary-value-param,performance-unnecessary-copy-initialization,performance-trivially-destructible,bugprone-parent-virtual-call,bugprone-macro-parenthesis,bugprone-string-constructor,bugprone-sizeof-expression' WarningsAsErrors: '*' HeaderFilterRegex: '.*' AnalyzeTemporaryDtors: false diff --git a/src/AsyncProducers.cpp b/src/AsyncProducers.cpp index 5932001dd0de..fbd8540d9400 100644 --- a/src/AsyncProducers.cpp +++ b/src/AsyncProducers.cpp @@ -216,7 +216,7 @@ class GenerateConsumerBody : public NoOpCollapsingMutator { return Acquire::make(acquire_sema, 1, op); } } else { - return IRMutator::visit(op); + return NoOpCollapsingMutator::visit(op); } } @@ -225,7 +225,7 @@ class GenerateConsumerBody : public NoOpCollapsingMutator { if (starts_with(op->name, func + ".folding_semaphore.") && ends_with(op->name, ".head")) { return mutate(op->body); } else { - return IRMutator::visit(op); + return NoOpCollapsingMutator::visit(op); } } @@ -233,7 +233,7 @@ class GenerateConsumerBody : public NoOpCollapsingMutator { if (starts_with(op->name, func + ".folding_semaphore.") && ends_with(op->name, ".head")) { return Evaluate::make(0); } else { - return IRMutator::visit(op); + return NoOpCollapsingMutator::visit(op); } } @@ -245,7 +245,7 @@ class GenerateConsumerBody : public NoOpCollapsingMutator { if (starts_with(var->name, func + ".folding_semaphore.")) { return mutate(op->body); } else { - return IRMutator::visit(op); + return NoOpCollapsingMutator::visit(op); } } diff --git a/src/CodeGen_OpenCL_Dev.cpp b/src/CodeGen_OpenCL_Dev.cpp index b7f9f354f360..7d570031e64a 100644 --- a/src/CodeGen_OpenCL_Dev.cpp +++ b/src/CodeGen_OpenCL_Dev.cpp @@ -645,7 +645,7 @@ void CodeGen_OpenCL_Dev::CodeGen_OpenCL_C::visit(const Atomic *op) { // Issue atomic stores. ScopedValue old_emit_atomic_stores(emit_atomic_stores, true); - IRVisitor::visit(op); + CodeGen_C::visit(op); } void CodeGen_OpenCL_Dev::add_kernel(Stmt s, diff --git a/src/CodeGen_OpenGL_Dev.cpp b/src/CodeGen_OpenGL_Dev.cpp index ef1cb093d8d3..6e864fae25f6 100644 --- a/src/CodeGen_OpenGL_Dev.cpp +++ b/src/CodeGen_OpenGL_Dev.cpp @@ -884,12 +884,12 @@ void CodeGen_GLSL::add_kernel(const Stmt &stmt, const string &name, ++num_varying_floats; } else if (args[i].type.is_float()) { header << "/// UNIFORM " - << CodeGen_C::print_type(args[i].type) << " " + << CodeGen_GLSLBase::print_type(args[i].type) << " " << print_name(args[i].name) << " uniformf" << args[i].packed_index / 4 << "[" << args[i].packed_index % 4 << "]\n"; ++num_uniform_floats; } else if (args[i].type.is_int()) { header << "/// UNIFORM " - << CodeGen_C::print_type(args[i].type) << " " + << CodeGen_GLSLBase::print_type(args[i].type) << " " << print_name(args[i].name) << " uniformi" << args[i].packed_index / 4 << "[" << args[i].packed_index % 4 << "]\n"; ++num_uniform_ints; } diff --git a/src/CodeGen_PTX_Dev.cpp b/src/CodeGen_PTX_Dev.cpp index afc5e690c8b2..a17d1c4fd659 100644 --- a/src/CodeGen_PTX_Dev.cpp +++ b/src/CodeGen_PTX_Dev.cpp @@ -328,7 +328,7 @@ void CodeGen_PTX_Dev::visit(const Atomic *op) { // Issue atomic stores. ScopedValue old_emit_atomic_stores(emit_atomic_stores, true); - IRVisitor::visit(op); + CodeGen_LLVM::visit(op); } string CodeGen_PTX_Dev::march() const { diff --git a/src/Generator.h b/src/Generator.h index 3c69a74472f2..8e777b60b3d4 100644 --- a/src/Generator.h +++ b/src/Generator.h @@ -1011,11 +1011,11 @@ class GeneratorParam : public Internal::GeneratorParamImplBase { * Returns type of underlying operator+. */ // @{ template -decltype((Other)0 + (T)0) operator+(const Other &a, const GeneratorParam &b) { +auto operator+(const Other &a, const GeneratorParam &b) -> decltype(a + (T)b) { return a + (T)b; } template -decltype((T)0 + (Other)0) operator+(const GeneratorParam &a, const Other &b) { +auto operator+(const GeneratorParam &a, const Other &b) -> decltype((T)a + b) { return (T)a + b; } // @} @@ -1024,11 +1024,11 @@ decltype((T)0 + (Other)0) operator+(const GeneratorParam &a, const Other &b) * Returns type of underlying operator-. */ // @{ template -decltype((Other)0 - (T)0) operator-(const Other &a, const GeneratorParam &b) { +auto operator-(const Other &a, const GeneratorParam &b) -> decltype(a - (T)b) { return a - (T)b; } template -decltype((T)0 - (Other)0) operator-(const GeneratorParam &a, const Other &b) { +auto operator-(const GeneratorParam &a, const Other &b) -> decltype((T)a - b) { return (T)a - b; } // @} @@ -1037,11 +1037,11 @@ decltype((T)0 - (Other)0) operator-(const GeneratorParam &a, const Other &b) * Returns type of underlying operator*. */ // @{ template -decltype((Other)0 * (T)0) operator*(const Other &a, const GeneratorParam &b) { +auto operator*(const Other &a, const GeneratorParam &b) -> decltype(a * (T)b) { return a * (T)b; } template -decltype((Other)0 * (T)0) operator*(const GeneratorParam &a, const Other &b) { +auto operator*(const GeneratorParam &a, const Other &b) -> decltype((T)a * b) { return (T)a * b; } // @} @@ -1050,11 +1050,11 @@ decltype((Other)0 * (T)0) operator*(const GeneratorParam &a, const Other &b) * Returns type of underlying operator/. */ // @{ template -decltype((Other)0 / (T)1) operator/(const Other &a, const GeneratorParam &b) { +auto operator/(const Other &a, const GeneratorParam &b) -> decltype(a / (T)b) { return a / (T)b; } template -decltype((T)0 / (Other)1) operator/(const GeneratorParam &a, const Other &b) { +auto operator/(const GeneratorParam &a, const Other &b) -> decltype((T)a / b) { return (T)a / b; } // @} @@ -1063,11 +1063,11 @@ decltype((T)0 / (Other)1) operator/(const GeneratorParam &a, const Other &b) * Returns type of underlying operator%. */ // @{ template -decltype((Other)0 % (T)1) operator%(const Other &a, const GeneratorParam &b) { +auto operator%(const Other &a, const GeneratorParam &b) -> decltype(a % (T)b) { return a % (T)b; } template -decltype((T)0 % (Other)1) operator%(const GeneratorParam &a, const Other &b) { +auto operator%(const GeneratorParam &a, const Other &b) -> decltype((T)a % b) { return (T)a % b; } // @} @@ -1076,11 +1076,11 @@ decltype((T)0 % (Other)1) operator%(const GeneratorParam &a, const Other &b) * Returns type of underlying operator>. */ // @{ template -decltype((Other)0 > (T)1) operator>(const Other &a, const GeneratorParam &b) { +auto operator>(const Other &a, const GeneratorParam &b) -> decltype(a > (T)b) { return a > (T)b; } template -decltype((T)0 > (Other)1) operator>(const GeneratorParam &a, const Other &b) { +auto operator>(const GeneratorParam &a, const Other &b) -> decltype((T)a > b) { return (T)a > b; } // @} @@ -1089,11 +1089,11 @@ decltype((T)0 > (Other)1) operator>(const GeneratorParam &a, const Other &b) * Returns type of underlying operator<. */ // @{ template -decltype((Other)0 < (T)1) operator<(const Other &a, const GeneratorParam &b) { +auto operator<(const Other &a, const GeneratorParam &b) -> decltype(a < (T)b) { return a < (T)b; } template -decltype((T)0 < (Other)1) operator<(const GeneratorParam &a, const Other &b) { +auto operator<(const GeneratorParam &a, const Other &b) -> decltype((T)a < b) { return (T)a < b; } // @} @@ -1102,11 +1102,11 @@ decltype((T)0 < (Other)1) operator<(const GeneratorParam &a, const Other &b) * Returns type of underlying operator>=. */ // @{ template -decltype((Other)0 >= (T)1) operator>=(const Other &a, const GeneratorParam &b) { +auto operator>=(const Other &a, const GeneratorParam &b) -> decltype(a >= (T)b) { return a >= (T)b; } template -decltype((T)0 >= (Other)1) operator>=(const GeneratorParam &a, const Other &b) { +auto operator>=(const GeneratorParam &a, const Other &b) -> decltype((T)a >= b) { return (T)a >= b; } // @} @@ -1115,11 +1115,11 @@ decltype((T)0 >= (Other)1) operator>=(const GeneratorParam &a, const Other &b * Returns type of underlying operator<=. */ // @{ template -decltype((Other)0 <= (T)1) operator<=(const Other &a, const GeneratorParam &b) { +auto operator<=(const Other &a, const GeneratorParam &b) -> decltype(a <= (T)b) { return a <= (T)b; } template -decltype((T)0 <= (Other)1) operator<=(const GeneratorParam &a, const Other &b) { +auto operator<=(const GeneratorParam &a, const Other &b) -> decltype((T)a <= b) { return (T)a <= b; } // @} @@ -1128,11 +1128,11 @@ decltype((T)0 <= (Other)1) operator<=(const GeneratorParam &a, const Other &b * Returns type of underlying operator==. */ // @{ template -decltype((Other)0 == (T)1) operator==(const Other &a, const GeneratorParam &b) { +auto operator==(const Other &a, const GeneratorParam &b) -> decltype(a == (T)b) { return a == (T)b; } template -decltype((T)0 == (Other)1) operator==(const GeneratorParam &a, const Other &b) { +auto operator==(const GeneratorParam &a, const Other &b) -> decltype((T)a == b) { return (T)a == b; } // @} @@ -1141,11 +1141,11 @@ decltype((T)0 == (Other)1) operator==(const GeneratorParam &a, const Other &b * Returns type of underlying operator!=. */ // @{ template -decltype((Other)0 != (T)1) operator!=(const Other &a, const GeneratorParam &b) { +auto operator!=(const Other &a, const GeneratorParam &b) -> decltype(a != (T)b) { return a != (T)b; } template -decltype((T)0 != (Other)1) operator!=(const GeneratorParam &a, const Other &b) { +auto operator!=(const GeneratorParam &a, const Other &b) -> decltype((T)a != b) { return (T)a != b; } // @} @@ -1154,15 +1154,15 @@ decltype((T)0 != (Other)1) operator!=(const GeneratorParam &a, const Other &b * Returns type of underlying operator&&. */ // @{ template -decltype((Other)0 && (T)1) operator&&(const Other &a, const GeneratorParam &b) { +auto operator&&(const Other &a, const GeneratorParam &b) -> decltype(a && (T)b) { return a && (T)b; } template -decltype((T)0 && (Other)1) operator&&(const GeneratorParam &a, const Other &b) { +auto operator&&(const GeneratorParam &a, const Other &b) -> decltype((T)a && b) { return (T)a && b; } template -decltype((T)0 && (T)1) operator&&(const GeneratorParam &a, const GeneratorParam &b) { +auto operator&&(const GeneratorParam &a, const GeneratorParam &b) -> decltype((T)a && (T)b) { return (T)a && (T)b; } // @} @@ -1171,15 +1171,15 @@ decltype((T)0 && (T)1) operator&&(const GeneratorParam &a, const GeneratorPar * Returns type of underlying operator||. */ // @{ template -decltype((Other)0 || (T)1) operator||(const Other &a, const GeneratorParam &b) { +auto operator||(const Other &a, const GeneratorParam &b) -> decltype(a || (T)b) { return a || (T)b; } template -decltype((T)0 || (Other)1) operator||(const GeneratorParam &a, const Other &b) { +auto operator||(const GeneratorParam &a, const Other &b) -> decltype((T)a || b) { return (T)a || b; } template -decltype((T)0 || (T)1) operator||(const GeneratorParam &a, const GeneratorParam &b) { +auto operator||(const GeneratorParam &a, const GeneratorParam &b) -> decltype((T)a || (T)b) { return (T)a || (T)b; } // @} @@ -1195,20 +1195,20 @@ using std::max; using std::min; template -decltype(min((Other)0, (T)1)) min_forward(const Other &a, const GeneratorParam &b) { +auto min_forward(const Other &a, const GeneratorParam &b) -> decltype(min(a, (T)b)) { return min(a, (T)b); } template -decltype(min((T)0, (Other)1)) min_forward(const GeneratorParam &a, const Other &b) { +auto min_forward(const GeneratorParam &a, const Other &b) -> decltype(min((T)a, b)) { return min((T)a, b); } template -decltype(max((Other)0, (T)1)) max_forward(const Other &a, const GeneratorParam &b) { +auto max_forward(const Other &a, const GeneratorParam &b) -> decltype(max(a, (T)b)) { return max(a, (T)b); } template -decltype(max((T)0, (Other)1)) max_forward(const GeneratorParam &a, const Other &b) { +auto max_forward(const GeneratorParam &a, const Other &b) -> decltype(max((T)a, b)) { return max((T)a, b); } @@ -1243,7 +1243,7 @@ auto max(const GeneratorParam &a, const Other &b) -> decltype(Internal::Gener /** Not operator for GeneratorParam */ template -decltype(!(T)0) operator!(const GeneratorParam &a) { +auto operator!(const GeneratorParam &a) -> decltype(!(T)a) { return !(T)a; } diff --git a/src/RDom.cpp b/src/RDom.cpp index ac60042e9dea..9903ba05cddc 100644 --- a/src/RDom.cpp +++ b/src/RDom.cpp @@ -77,9 +77,9 @@ ReductionDomain build_domain(ReductionVariable (&vars)[N]) { // This just initializes the predefined x, y, z, w members of RDom. void RDom::init_vars(const string &name) { const std::vector &dom_vars = dom.domain(); - RVar *vars[] = {&x, &y, &z, &w}; + std::array vars = {&x, &y, &z, &w}; - for (size_t i = 0; i < sizeof(vars) / sizeof(vars[0]); i++) { + for (size_t i = 0; i < vars.size(); i++) { if (i < dom_vars.size()) { *(vars[i]) = RVar(dom, i); } else { From d5e26ae7a7e89804d522ca1b42b1da3a329b1dbb Mon Sep 17 00:00:00 2001 From: Andrew Adams Date: Wed, 11 Mar 2020 17:51:05 -0700 Subject: [PATCH 2/2] Add missing include --- src/RDom.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/RDom.cpp b/src/RDom.cpp index 9903ba05cddc..bf9c8e5a6b2e 100644 --- a/src/RDom.cpp +++ b/src/RDom.cpp @@ -1,14 +1,14 @@ - -#include "RDom.h" +#include +#include #include "Generator.h" #include "IREquality.h" #include "IROperator.h" #include "IRPrinter.h" #include "ImageParam.h" +#include "RDom.h" #include "Simplify.h" #include "Util.h" -#include namespace Halide {