Partially apply clang-tidy fixes we don't enforce yet (#8376)

* Partially apply clang-tidy fixes we don't use yet - Put a bunch of stuff into anonymous namespaces - Delete some redundant casts (e.g. casting an int to int) - Add some const refs to avoid copies - Remove meaningless inline qualifiers on in-class definitions and constexpr functions - Remove return-with-value from functions returning void - Delete a little dead code - Use std::min/max where appropriate - Don't use a variable after std::forwarding it. It may have been moved from. - Use std::string::empty instead of comparing length to zero * Undo unintentional formatting change * Restore some necessary casts * Add NOLINT to silence older clang-tidy
halide · Aug 16, 2024 · 4f30d2b · 4f30d2b
1 parent 818f42d
commit 4f30d2b
Show file tree

Hide file tree

Showing 65 changed files with 200 additions and 181 deletions.
diff --git a/python_bindings/src/halide/halide_/PyRDom.cpp b/python_bindings/src/halide/halide_/PyRDom.cpp
@@ -5,6 +5,7 @@
 namespace Halide {
 namespace PythonBindings {
 
+namespace {
 void define_rvar(py::module &m) {
     auto rvar_class =
         py::class_<RVar>(m, "RVar")
@@ -24,6 +25,7 @@ void define_rvar(py::module &m) {
 
     add_binary_operators(rvar_class);
 }
+}  // namespace
 
 void define_rdom(py::module &m) {
     define_rvar(m);

diff --git a/src/AddImageChecks.cpp b/src/AddImageChecks.cpp
@@ -526,7 +526,7 @@ Stmt add_image_checks_inner(Stmt s,
                         << "as the first output buffer.\n";
 
                     stride_constrained = param.stride_constraint(i);
-                } else if (image.defined() && (int)i < image.dimensions()) {
+                } else if (image.defined() && i < image.dimensions()) {
                     stride_constrained = image.dim(i).stride();
                 }
 
@@ -543,7 +543,7 @@ Stmt add_image_checks_inner(Stmt s,
                 } else {
                     extent_constrained = Variable::make(Int(32), extent0_name);
                 }
-            } else if (image.defined() && (int)i < image.dimensions()) {
+            } else if (image.defined() && i < image.dimensions()) {
                 stride_constrained = image.dim(i).stride();
                 extent_constrained = image.dim(i).extent();
                 min_constrained = image.dim(i).min();

diff --git a/src/BoundaryConditions.cpp b/src/BoundaryConditions.cpp
@@ -14,7 +14,7 @@ Func repeat_edge(const Func &source,
 
     std::vector<Expr> actuals;
     for (size_t i = 0; i < bounds.size(); i++) {
-        Var arg_var = args[i];
+        const Var &arg_var = args[i];
         Expr min = bounds[i].min;
         Expr extent = bounds[i].extent;
 
@@ -39,16 +39,15 @@ Func repeat_edge(const Func &source,
 
 Func constant_exterior(const Func &source, const Tuple &value,
                        const Region &bounds) {
-    std::vector<Var> source_args = source.args();
-    std::vector<Var> args(source_args);
+    std::vector<Var> args(source.args());
     user_assert(args.size() >= bounds.size())
         << "constant_exterior called with more bounds (" << bounds.size()
-        << ") than dimensions (" << source_args.size()
+        << ") than dimensions (" << args.size()
         << ") Func " << source.name() << " has.\n";
 
     Expr out_of_bounds = cast<bool>(false);
     for (size_t i = 0; i < bounds.size(); i++) {
-        Var arg_var = source_args[i];
+        const Var &arg_var = args[i];
         Expr min = bounds[i].min;
         Expr extent = bounds[i].extent;
 
@@ -91,7 +90,7 @@ Func repeat_image(const Func &source,
 
     std::vector<Expr> actuals;
     for (size_t i = 0; i < bounds.size(); i++) {
-        Var arg_var = args[i];
+        const Var &arg_var = args[i];
         Expr min = bounds[i].min;
         Expr extent = bounds[i].extent;
 
@@ -146,7 +145,7 @@ Func mirror_image(const Func &source,
 
     std::vector<Expr> actuals;
     for (size_t i = 0; i < bounds.size(); i++) {
-        Var arg_var = args[i];
+        const Var &arg_var = args[i];
 
         Expr min = bounds[i].min;
         Expr extent = bounds[i].extent;
@@ -187,7 +186,7 @@ Func mirror_interior(const Func &source,
 
     std::vector<Expr> actuals;
     for (size_t i = 0; i < bounds.size(); i++) {
-        Var arg_var = args[i];
+        const Var &arg_var = args[i];
 
         Expr min = bounds[i].min;
         Expr extent = bounds[i].extent;

diff --git a/src/Bounds.cpp b/src/Bounds.cpp
@@ -1857,12 +1857,6 @@ Interval bounds_of_expr_in_scope_with_indent(const Expr &expr, const Scope<Inter
     return b.interval;
 }
 
-}  // namespace
-
-Interval bounds_of_expr_in_scope(const Expr &expr, const Scope<Interval> &scope, const FuncValueBounds &fb, bool const_bound) {
-    return bounds_of_expr_in_scope_with_indent(expr, scope, fb, const_bound, 0);
-}
-
 Region region_union(const Region &a, const Region &b) {
     internal_assert(a.size() == b.size()) << "Mismatched dimensionality in region union\n";
     Region result;
@@ -1877,6 +1871,12 @@ Region region_union(const Region &a, const Region &b) {
     return result;
 }
 
+}  // namespace
+
+Interval bounds_of_expr_in_scope(const Expr &expr, const Scope<Interval> &scope, const FuncValueBounds &fb, bool const_bound) {
+    return bounds_of_expr_in_scope_with_indent(expr, scope, fb, const_bound, 0);
+}
+
 void merge_boxes(Box &a, const Box &b) {
     if (b.empty()) {
         return;
@@ -3085,8 +3085,6 @@ class BoxesTouched : public IRGraphVisitor {
     }
 };
 
-}  // namespace
-
 map<string, Box> boxes_touched(const Expr &e, Stmt s, bool consider_calls, bool consider_provides,
                                const string &fn, const Scope<Interval> &scope, const FuncValueBounds &fb) {
     if (!fn.empty() && s.defined()) {
@@ -3275,6 +3273,7 @@ Box box_touched(const Expr &e, Stmt s, bool consider_calls, bool consider_provid
     internal_assert(boxes.size() <= 1);
     return boxes[fn];
 }
+}  // namespace
 
 map<string, Box> boxes_required(const Expr &e, const Scope<Interval> &scope, const FuncValueBounds &fb) {
     return boxes_touched(e, Stmt(), true, false, "", scope, fb);
@@ -3324,6 +3323,7 @@ Box box_touched(Stmt s, const string &fn, const Scope<Interval> &scope, const Fu
     return box_touched(Expr(), std::move(s), true, true, fn, scope, fb);
 }
 
+namespace {
 // Compute interval of all possible function's values (default + specialized values)
 Interval compute_pure_function_definition_value_bounds(
     const Definition &def, const Scope<Interval> &scope, const FuncValueBounds &fb, int dim) {
@@ -3338,14 +3338,15 @@ Interval compute_pure_function_definition_value_bounds(
     }
     return result;
 }
+}  // namespace
 
 FuncValueBounds compute_function_value_bounds(const vector<string> &order,
                                               const map<string, Function> &env) {
     FuncValueBounds fb;
 
     for (const auto &func_name : order) {
         Function f = env.find(func_name)->second;
-        const vector<string> f_args = f.args();
+        const vector<string> &f_args = f.args();
         for (int j = 0; j < f.outputs(); j++) {
             pair<string, int> key = {f.name(), j};
 

diff --git a/src/CPlusPlusMangle.cpp b/src/CPlusPlusMangle.cpp
@@ -44,8 +44,6 @@ Type non_null_void_star_type() {
     return Handle(1, &t);
 }
 
-}  // namespace
-
 namespace WindowsMangling {
 
 struct PreviousDeclarations {
@@ -615,6 +613,8 @@ std::string cplusplus_function_mangled_name(const std::string &name, const std::
 
 }  // namespace ItaniumABIMangling
 
+}  // namespace
+
 std::string cplusplus_function_mangled_name(const std::string &name, const std::vector<std::string> &namespaces,
                                             Type return_type, const std::vector<ExternFuncArgument> &args,
                                             const Target &target) {

diff --git a/src/CodeGen_C.cpp b/src/CodeGen_C.cpp
@@ -797,7 +797,7 @@ void CodeGen_C::emit_metadata_getter(const std::string &function_name,
         stream << get_indent() << kind_names[arg.kind] << ",\n";
         stream << get_indent() << (int)arg.dimensions << ",\n";
         internal_assert(arg.type.code() < sizeof(type_code_names) / sizeof(type_code_names[0]));
-        stream << get_indent() << "{" << type_code_names[arg.type.code()] << ", " << (int)arg.type.bits() << ", " << (int)arg.type.lanes() << "},\n";
+        stream << get_indent() << "{" << type_code_names[arg.type.code()] << ", " << arg.type.bits() << ", " << arg.type.lanes() << "},\n";
         stream << get_indent() << "scalar_def_" << legalized_name << ",\n";
         stream << get_indent() << "scalar_min_" << legalized_name << ",\n";
         stream << get_indent() << "scalar_max_" << legalized_name << ",\n";
@@ -873,8 +873,8 @@ void CodeGen_C::emit_constexpr_function_info(const std::string &function_name,
         const auto name = map_name(arg.name);
 
         stream << get_indent() << "{\"" << name << "\", " << kind_names[arg.kind] << ", " << (int)arg.dimensions
-               << ", halide_type_t{" << type_code_names[arg.type.code()] << ", " << (int)arg.type.bits()
-               << ", " << (int)arg.type.lanes() << "}},\n";
+               << ", halide_type_t{" << type_code_names[arg.type.code()] << ", " << arg.type.bits()
+               << ", " << arg.type.lanes() << "}},\n";
     }
     indent -= 1;
     stream << get_indent() << "}};\n";

diff --git a/src/CodeGen_D3D12Compute_Dev.cpp b/src/CodeGen_D3D12Compute_Dev.cpp
@@ -958,7 +958,7 @@ void CodeGen_D3D12Compute_Dev::CodeGen_D3D12Compute_C::visit(const FloatImm *op)
     // have seen division-by-zero shader warnings, and we postulated that it
     // could be indirectly related to compiler assumptions on signed integer
     // overflow when float_from_bits() is called, but we don't know for sure
-    return CodeGen_GPU_C::visit(op);
+    CodeGen_GPU_C::visit(op);
 }
 
 void CodeGen_D3D12Compute_Dev::add_kernel(Stmt s,
@@ -1146,10 +1146,12 @@ void CodeGen_D3D12Compute_Dev::CodeGen_D3D12Compute_C::add_kernel(Stmt s,
         using IRVisitor::visit;
         void visit(const For *loop) override {
             if (!is_gpu(loop->for_type)) {
-                return loop->body.accept(this);
+                loop->body.accept(this);
+                return;
             }
             if (loop->for_type != ForType::GPUThread) {
-                return loop->body.accept(this);
+                loop->body.accept(this);
+                return;
             }
             internal_assert(is_const_zero(loop->min));
             int index = thread_loop_workgroup_index(loop->name);

diff --git a/src/CodeGen_Hexagon.cpp b/src/CodeGen_Hexagon.cpp
@@ -1044,7 +1044,7 @@ Value *CodeGen_Hexagon::interleave_vectors(const vector<llvm::Value *> &v) {
             // Break them into native vectors, use vshuffvdd, and
             // concatenate the shuffled results.
             llvm::Type *native2_ty = get_vector_type(element_ty, native_elements * 2);
-            Value *bytes = codegen(-static_cast<int>(element_bits / 8));
+            Value *bytes = codegen(-(element_bits / 8));
             vector<Value *> ret;
             for (int i = 0; i < result_elements / 2; i += native_elements) {
                 Value *a_i = slice_vector(a, i, native_elements);
@@ -1147,7 +1147,7 @@ Value *CodeGen_Hexagon::shuffle_vectors(Value *a, Value *b,
     llvm::Type *b_ty = b->getType();
     internal_assert(a_ty == b_ty);
 
-    int a_elements = static_cast<int>(get_vector_num_elements(a_ty));
+    int a_elements = get_vector_num_elements(a_ty);
 
     llvm::Type *element_ty = get_vector_element_type(a->getType());
     internal_assert(element_ty);

diff --git a/src/CodeGen_Internal.cpp b/src/CodeGen_Internal.cpp
@@ -292,6 +292,7 @@ Expr lower_int_uint_mod(const Expr &a, const Expr &b) {
     }
 }
 
+namespace {
 std::pair<Expr, Expr> unsigned_long_div_mod_round_to_zero(Expr &num, const Expr &den,
                                                           const uint64_t *upper_bound) {
     internal_assert(num.type() == den.type());
@@ -329,6 +330,7 @@ std::pair<Expr, Expr> unsigned_long_div_mod_round_to_zero(Expr &num, const Expr
     }
     return {q, r};
 }
+}  // namespace
 
 std::pair<Expr, Expr> long_div_mod_round_to_zero(const Expr &num, const Expr &den,
                                                  const uint64_t *max_abs) {
@@ -557,6 +559,7 @@ Expr lower_round_to_nearest_ties_to_even(const Expr &x) {
     return common_subexpression_elimination(a - correction);
 }
 
+namespace {
 bool get_md_bool(llvm::Metadata *value, bool &result) {
     if (!value) {
         return false;
@@ -585,6 +588,7 @@ bool get_md_string(llvm::Metadata *value, std::string &result) {
     }
     return false;
 }
+}  // namespace
 
 void get_target_options(const llvm::Module &module, llvm::TargetOptions &options) {
     bool use_soft_float_abi = false;

diff --git a/src/CodeGen_LLVM.cpp b/src/CodeGen_LLVM.cpp
@@ -570,7 +570,7 @@ std::unique_ptr<llvm::Module> CodeGen_LLVM::compile(const Module &input) {
     // Define all functions
     int idx = 0;
     for (const auto &f : input.functions()) {
-        const auto names = function_names[idx++];
+        const auto &names = function_names[idx++];
 
         run_with_large_stack([&]() {
             compile_func(f, names.simple_name, names.extern_name);
@@ -3228,7 +3228,7 @@ void CodeGen_LLVM::visit(const Call *op) {
         builder->SetInsertPoint(global_not_inited_bb);
         llvm::Value *selected_value = nullptr;
         for (int i = sub_fns.size() - 1; i >= 0; i--) {
-            const auto sub_fn = sub_fns[i];
+            const auto &sub_fn = sub_fns[i];
             if (!selected_value) {
                 selected_value = sub_fn.fn_ptr;
             } else {

diff --git a/src/CodeGen_PTX_Dev.cpp b/src/CodeGen_PTX_Dev.cpp
@@ -20,14 +20,6 @@
 
 #include <fstream>
 
-// This is declared in NVPTX.h, which is not exported. Ugly, but seems better than
-// hardcoding a path to the .h file.
-#ifdef WITH_NVPTX
-namespace llvm {
-FunctionPass *createNVVMReflectPass(const StringMap<int> &Mapping);
-}
-#endif
-
 namespace Halide {
 namespace Internal {
 

diff --git a/src/CodeGen_PowerPC.cpp b/src/CodeGen_PowerPC.cpp
@@ -129,7 +129,7 @@ void CodeGen_PowerPC::visit(const Min *op) {
             return;
         }
     }
-    return CodeGen_Posix::visit(op);
+    CodeGen_Posix::visit(op);
 }
 
 void CodeGen_PowerPC::visit(const Max *op) {
@@ -139,7 +139,7 @@ void CodeGen_PowerPC::visit(const Max *op) {
             return;
         }
     }
-    return CodeGen_Posix::visit(op);
+    CodeGen_Posix::visit(op);
 }
 
 string CodeGen_PowerPC::mcpu_target() const {

diff --git a/src/CodeGen_Vulkan_Dev.cpp b/src/CodeGen_Vulkan_Dev.cpp
@@ -1261,10 +1261,10 @@ void CodeGen_Vulkan_Dev::SPIRV_Emitter::visit(const Call *op) {
                 one_constant_id = builder.declare_constant(op->type, &one_value);
             }
         } else if (op->type.is_float() && op->type.bits() == 32) {
-            float one_value = float(1.0f);
+            float one_value = 1.0f;
             one_constant_id = builder.declare_constant(op->type, &one_value);
         } else if (op->type.is_float() && op->type.bits() == 64) {
-            double one_value = double(1.0);
+            double one_value = 1.0;
             one_constant_id = builder.declare_constant(op->type, &one_value);
         } else {
             internal_error << "Vulkan: Unhandled float type in fast_inverse intrinsic!\n";
@@ -1832,7 +1832,7 @@ void CodeGen_Vulkan_Dev::SPIRV_Emitter::visit(const Allocate *op) {
             array_size = op->constant_allocation_size();
             array_type_id = builder.declare_type(op->type, array_size);
             builder.add_symbol(variable_name + "_array_type", array_type_id, builder.current_module().id());
-            debug(2) << "Vulkan: Allocate (fixed-size) " << op->name << " type=" << op->type << " array_size=" << (uint32_t)array_size << " in shared memory on device in global scope\n";
+            debug(2) << "Vulkan: Allocate (fixed-size) " << op->name << " type=" << op->type << " array_size=" << array_size << " in shared memory on device in global scope\n";
 
         } else {
             // dynamic allocation with unknown size at compile time ...
@@ -1844,7 +1844,7 @@ void CodeGen_Vulkan_Dev::SPIRV_Emitter::visit(const Allocate *op) {
             array_type_id = builder.add_array_with_default_size(storage_type_id, array_size_id);
             builder.add_symbol(variable_name + "_array_type", array_type_id, builder.current_module().id());
 
-            debug(2) << "Vulkan: Allocate (dynamic size) " << op->name << " type=" << op->type << " default_size=" << (uint32_t)array_size << " in shared memory on device in global scope\n";
+            debug(2) << "Vulkan: Allocate (dynamic size) " << op->name << " type=" << op->type << " default_size=" << array_size << " in shared memory on device in global scope\n";
 
             // bind the specialization constant to the next slot
             std::string constant_name = variable_name + "_array_size";
@@ -1876,7 +1876,7 @@ void CodeGen_Vulkan_Dev::SPIRV_Emitter::visit(const Allocate *op) {
             << "Allocation " << op->name << " has a dynamic size. "
             << "Only fixed-size local allocations are supported with Vulkan.";
 
-        debug(2) << "Vulkan: Allocate " << op->name << " type=" << op->type << " size=" << (uint32_t)array_size << " on device in function scope\n";
+        debug(2) << "Vulkan: Allocate " << op->name << " type=" << op->type << " size=" << array_size << " on device in function scope\n";
 
         array_type_id = builder.declare_type(op->type, array_size);
         storage_class = SpvStorageClassFunction;  // function scope
@@ -2705,7 +2705,7 @@ void CodeGen_Vulkan_Dev::SPIRV_Emitter::declare_device_args(const Stmt &s, uint3
 
             // Set descriptor set and binding indices
             SpvBuilder::Literals dset_index = {entry_point_index};
-            SpvBuilder::Literals binding_index = {uint32_t(binding_counter++)};
+            SpvBuilder::Literals binding_index = {binding_counter++};
             builder.add_annotation(buffer_block_var_id, SpvDecorationDescriptorSet, dset_index);
             builder.add_annotation(buffer_block_var_id, SpvDecorationBinding, binding_index);
             symbol_table.push(arg.name, {buffer_block_var_id, storage_class});

diff --git a/src/CodeGen_WebGPU_Dev.cpp b/src/CodeGen_WebGPU_Dev.cpp
@@ -408,7 +408,7 @@ void CodeGen_WebGPU_Dev::CodeGen_WGSL::add_kernel(
 
     close_scope("shader " + name);
 
-    for (auto [name, alloc] : workgroup_allocations) {
+    for (const auto &[name, alloc] : workgroup_allocations) {
         std::stringstream length;
         if (is_const(alloc->extents[0])) {
             length << alloc->extents[0];