detect/prevent Func vs input name collisions

src/FindCalls.cpp

@@ -3,6 +3,8 @@
 #include "ExternFuncArgument.h"
 #include "Function.h"
 #include "IRVisitor.h"
+#include "Parameter.h"
+#include <set>
 #include <utility>
 
 namespace Halide {
@@ -97,6 +99,64 @@ std::map<std::string, Function> build_environment(const std::vector<Function> &f
     for (const Function &f : funcs) {
         populate_environment_helper(f, &env, &order, true, true);
     }
+
+    // Validate the environment: no Parameter (ImageParam, Generator
+    // Input<Buffer>, or scalar Param) may share a name with a Func in the
+    // pipeline. Such a collision otherwise causes confusing internal errors
+    // later in lowering. Output Funcs intentionally share names with their
+    // output buffer Parameters, so exclude buffer params whose name matches
+    // an output Func.
+    class FindParamNames : public IRVisitor {
+        using IRVisitor::visit;
+        void record(const Parameter &p) {
+            if (!p.defined()) {
+                return;
+            }
+            if (p.is_buffer()) {
+                buffer_names.insert(p.name());
+            } else {
+                scalar_names.insert(p.name());
+            }
+        }
+        void visit(const Variable *op) override {
+            record(op->param);
+        }
+        void visit(const Call *op) override {
+            IRVisitor::visit(op);
+            record(op->param);
+        }
+
+    public:
+        std::set<std::string> buffer_names;
+        std::set<std::string> scalar_names;
+    } finder;
+    for (const auto &p : env) {
+        p.second.accept(&finder);
+    }
+    std::set<std::string> output_names;
+    for (const Function &f : funcs) {
+        output_names.insert(f.name());
+    }
+    for (const std::string &name : finder.buffer_names) {
+        if (output_names.count(name)) {
+            continue;
+        }
+        if (env.count(name)) {
+            user_error << "The name \"" << name << "\" is used for both "
+                       << "an input buffer (ImageParam or Generator Input<Buffer>) "
+                       << "and a Func in the same pipeline. "
+                       << "Input buffers and Funcs must have distinct names.\n";
+        }
+    }
+    for (const std::string &name : finder.scalar_names) {
+        if (env.count(name)) {
+            user_error << "The name \"" << name << "\" is used for both "
+                       << "a scalar Param (or Generator Input scalar) "
+                       << "and a Func in the same pipeline. "
+                       << "Params and Funcs must have distinct names.\n";
+        }
+    }
+
     return env;
 }
 

src/Generator.cpp

@@ -1919,6 +1919,8 @@ void GeneratorInputBase::init_internals() {
     funcs_.clear();
     for (size_t i = 0; i < array_size(); ++i) {
         auto name = array_name(i);
+        // Discourage future Funcs from having the same name as this Input.
+        Internal::unique_name(name);
         parameters_.emplace_back(gio_type(), kind() != ArgInfoKind::Scalar, dims(), name);
         auto &p = parameters_[i];
         if (kind() != ArgInfoKind::Scalar) {

src/Param.h

@@ -37,6 +37,8 @@ class Param {
             << "is no longer used to control whether Halide functions take explicit "
             << "user_context arguments. Use set_custom_user_context() when jitting, "
             << "or add Target::UserContext to the Target feature set when compiling ahead of time.";
+        // Discourage future Funcs from having the same name as this Param.
+        Internal::unique_name(param.name());
     }
 
     // Allow all Param<> variants friend access to each other

test/correctness/CMakeLists.txt

@@ -188,6 +188,7 @@ tests(GROUPS correctness
       infer_arguments.cpp
       inline_reduction.cpp
       inlined_generator.cpp
+      input_func_name_unique.cpp
       input_image_bounds_check.cpp
       input_larger_than_two_gigs.cpp
       integer_powers.cpp

test/correctness/input_func_name_unique.cpp

@@ -0,0 +1,64 @@
+#include "Halide.h"
+#include <stdio.h>
+
+using namespace Halide;
+
+namespace {
+
+// Generator with an Input<Buffer> declared before any Func of the same
+// name. The Func created inside generate() must be renamed so the
+// pipeline compiles without collision.
+class GenInputBufferThenFunc : public Halide::Generator<GenInputBufferThenFunc> {
+public:
+    Input<Buffer<int, 1>> input_foo{"foo"};
+    Output<Buffer<int, 1>> out{"out"};
+
+    void generate() {
+        Var x;
+        Func foo("foo");
+        foo(x) = x;
+        out(x) = input_foo(x) + foo(x);
+    }
+};
+
+}  // namespace
+
+HALIDE_REGISTER_GENERATOR(GenInputBufferThenFunc, gen_input_buffer_then_func)
+
+int main(int argc, char **argv) {
+    // An ImageParam followed by a Func of the same name: the Func is
+    // renamed so the names differ.
+    {
+        ImageParam ip(Int(32), 1, "foo");
+        Func foo("foo");
+        assert(ip.name() != foo.name() &&
+               "ImageParam should reserve its name against later Funcs");
+    }
+
+    // A scalar Param followed by a Func of the same name: the Func is
+    // renamed so the names differ.
+    {
+        Param<int> p("foo");
+        Func foo("foo");
+        assert(p.name() != foo.name() &&
+               "Param should reserve its name against later Funcs");
+    }
+
+    // A Generator Input<Buffer> followed by a Func of the same name
+    // inside generate(): the Func is renamed and the pipeline compiles.
+    {
+        GeneratorContext ctx(get_jit_target_from_environment());
+        Callable c = create_callable_from_generator(ctx, "gen_input_buffer_then_func");
+
+        Buffer<int> in(10), out(10);
+        in.fill(0);
+        int r = c(in, out);
+        assert(r == 0);
+        for (int i = 0; i < 10; i++) {
+            assert(out(i) == i);
+        }
+    }
+
+    printf("Success!\n");
+    return 0;
+}

test/error/CMakeLists.txt

@@ -74,6 +74,9 @@ tests(GROUPS error
       impossible_constraints.cpp
       incomplete_target.cpp
       init_def_should_be_all_vars.cpp
+      input_buffer_func_name_collision.cpp
+      input_generator_buffer_func_name_collision.cpp
+      input_param_func_name_collision.cpp
       inspect_loop_level.cpp
       lerp_float_weight_out_of_range.cpp
       lerp_mismatch.cpp

test/error/input_buffer_func_name_collision.cpp

@@ -0,0 +1,22 @@
+#include "Halide.h"
+#include <stdio.h>
+
+using namespace Halide;
+
+int main(int argc, char **argv) {
+    // Func declared before an ImageParam of the same name. Lowering
+    // should produce a clean user_error rather than crashing.
+    Var x;
+    Func existing("foo");
+    existing(x) = x;
+
+    ImageParam ip(Int(32), 1, "foo");
+
+    Func out("out");
+    out(x) = existing(x) + ip(x);
+
+    out.compile_jit();
+
+    printf("Should not get here\n");
+    return 0;
+}

test/error/input_generator_buffer_func_name_collision.cpp

@@ -0,0 +1,35 @@
+#include "Halide.h"
+#include <stdio.h>
+
+using namespace Halide;
+
+namespace {
+
+// A Func declared as a member is constructed before the Input<Buffer>
+// member (declaration order), so by the time the Input's Parameter is
+// created, the Func has already reserved "foo". The Parameter still keeps
+// its literal name, so they collide in the resulting pipeline.
+class GenFuncBeforeInputBuffer : public Halide::Generator<GenFuncBeforeInputBuffer> {
+public:
+    Func foo{"foo"};
+    Input<Buffer<int, 1>> input_foo{"foo"};
+    Output<Buffer<int, 1>> out{"out"};
+
+    void generate() {
+        Var x;
+        foo(x) = x;
+        out(x) = input_foo(x) + foo(x);
+    }
+};
+
+}  // namespace
+
+HALIDE_REGISTER_GENERATOR(GenFuncBeforeInputBuffer, gen_input_buffer_func_collision)
+
+int main(int argc, char **argv) {
+    GeneratorContext ctx(get_jit_target_from_environment());
+    (void)create_callable_from_generator(ctx, "gen_input_buffer_func_collision");
+
+    printf("Should not get here\n");
+    return 0;
+}

test/error/input_param_func_name_collision.cpp

@@ -0,0 +1,22 @@
+#include "Halide.h"
+#include <stdio.h>
+
+using namespace Halide;
+
+int main(int argc, char **argv) {
+    // Func declared before a scalar Param of the same name. Lowering
+    // should produce a clean user_error rather than crashing.
+    Var x;
+    Func existing("foo");
+    existing(x) = x;
+
+    Param<int> p("foo");
+
+    Func out("out");
+    out(x) = existing(x) + p;
+
+    out.compile_jit();
+
+    printf("Should not get here\n");
+    return 0;
+}