Make variable renaming while inlining more robust

It now prevents accidentally hiding global variables too.

diff --git a/source/glsl/optimize.cpp b/source/glsl/optimize.cpp
index c8c9041fd43c7c9814cbd53e4d3ab8a719252cb2..c4118dee205d09f2f84c1e468001b6c6706de1c9 100644 (file)
--- a/source/glsl/optimize.cpp
+++ b/source/glsl/optimize.cpp
@@ -74,6 +74,10 @@ const string &InlineContentInjector::apply(Stage &stage, FunctionDeclaration &ta
 {
        target_block = &tgt_blk;
        source_func = &src;
+       remap_prefix = source_func->name;
+
+       vector<RefPtr<Statement> > inlined;
+       inlined.reserve(src.body.body.size());
        for(NodeList<Statement>::iterator i=src.body.body.begin(); i!=src.body.body.end(); ++i)
        {
                r_inlined_statement = 0;
@@ -81,11 +85,18 @@ const string &InlineContentInjector::apply(Stage &stage, FunctionDeclaration &ta
                if(!r_inlined_statement)
                        r_inlined_statement = (*i)->clone();
 
-               SetFlag set_remap(remap_names);
+               SetForScope<unsigned> set_remap(remap_names, 2);
                r_inlined_statement->visit(*this);
-               tgt_blk.body.insert(ins_pt, r_inlined_statement);
+               inlined.push_back(r_inlined_statement);
        }
 
+       SetForScope<unsigned> set_remap(remap_names, 1);
+       SetForScope<string> set_prefix(remap_prefix, target_func.name);
+       variable_map.clear();
+       target_func.visit(*this);
+
+       tgt_blk.body.insert(ins_pt, inlined.begin(), inlined.end());
+
        NodeReorderer().apply(stage, target_func, dependencies);
 
        return r_result_name;
@@ -102,7 +113,11 @@ void InlineContentInjector::visit(VariableReference &var)
        else if(var.declaration)
        {
                SetFlag set_deps(deps_only);
-               dependencies.insert(var.declaration);
+               if(!variable_map.count(var.name))
+               {
+                       dependencies.insert(var.declaration);
+                       referenced_names.insert(var.name);
+               }
                var.declaration->visit(*this);
        }
 }
@@ -113,6 +128,7 @@ void InlineContentInjector::visit(InterfaceBlockReference &iface)
        {
                SetFlag set_deps(deps_only);
                dependencies.insert(iface.declaration);
+               referenced_names.insert(iface.name);
                iface.declaration->visit(*this);
        }
 }
@@ -120,7 +136,10 @@ void InlineContentInjector::visit(InterfaceBlockReference &iface)
 void InlineContentInjector::visit(FunctionCall &call)
 {
        if(!remap_names && call.declaration)
+       {
                dependencies.insert(call.declaration);
+               referenced_names.insert(call.name);
+       }
        TraversingVisitor::visit(call);
 }
 
@@ -128,28 +147,29 @@ void InlineContentInjector::visit(VariableDeclaration &var)
 {
        TraversingVisitor::visit(var);
 
-       if(var.type_declaration)
+       if(remap_names)
+       {
+               if(remap_names==2 || referenced_names.count(var.name))
+               {
+                       string mapped_name = get_unused_variable_name(*target_block, var.name, remap_prefix);
+                       variable_map[var.name] = &var;
+                       var.name = mapped_name;
+               }
+       }
+       else if(var.type_declaration)
        {
                SetFlag set_deps(deps_only);
                dependencies.insert(var.type_declaration);
+               referenced_names.insert(var.type_declaration->name);
                var.type_declaration->visit(*this);
        }
-
-       if(!remap_names && !deps_only)
-       {
-               RefPtr<VariableDeclaration> inlined_var = var.clone();
-               inlined_var->name = get_unused_variable_name(*target_block, var.name, source_func->name);
-               r_inlined_statement = inlined_var;
-
-               variable_map[var.name] = inlined_var.get();
-       }
 }
 
 void InlineContentInjector::visit(Return &ret)
 {
        TraversingVisitor::visit(ret);
 
-       if(ret.expression)
+       if(!remap_names && ret.expression)
        {
                /* Create a new variable to hold the return value of the inlined
                function. */

diff --git a/source/glsl/optimize.h b/source/glsl/optimize.h
index ab954b58a471431feab1d6da0f75e9a2abe3bbaa..8a888e15bec7c11e019b172570e758a06f46963b 100644 (file)
--- a/source/glsl/optimize.h
+++ b/source/glsl/optimize.h
@@ -43,10 +43,12 @@ private:
        FunctionDeclaration *source_func;
        Block *target_block;
        std::map<std::string, VariableDeclaration *> variable_map;
-       bool remap_names;
+       std::string remap_prefix;
+       unsigned remap_names;
        bool deps_only;
        RefPtr<Statement> r_inlined_statement;
        std::set<Node *> dependencies;
+       std::set<std::string> referenced_names;
        std::string r_result_name;
 
 public:

diff --git a/source/glsl/syntax.cpp b/source/glsl/syntax.cpp
index cb50fee18b79538c126482594a023ea8f3864001..c94e2e27042c63a6a7f0b2c91f3d26c5b00f1e30 100644 (file)
--- a/source/glsl/syntax.cpp
+++ b/source/glsl/syntax.cpp
@@ -472,8 +472,14 @@ string get_unused_variable_name(const Block &block, const string &base, const st
        bool prefixed = false;
        unsigned number = 1;
        unsigned size_without_number = name.size();
-       while(block.variables.count(name))
+       while(1)
        {
+               bool unused = true;
+               for(const Block *b=█ (unused && b); b=b->parent)
+                       unused = !b->variables.count(name);
+               if(unused)
+                       return name;
+
                if(!prefixed && !prefix_hint.empty())
                {
                        if(name.front()!='_')
@@ -491,8 +497,6 @@ string get_unused_variable_name(const Block &block, const string &base, const st
                        ++number;
                }
        }
-
-       return name;
 }
 
 } // namespace SL

diff --git a/tests/glsl/function_inline_global_name_conflict.glsl b/tests/glsl/function_inline_global_name_conflict.glsl
new file mode 100644 (file)
index 0000000..8ebbfb4
--- /dev/null
+++ b/tests/glsl/function_inline_global_name_conflict.glsl
@@ -0,0 +1,28 @@
+#pragma MSP stage(vertex)
+layout(location=0) in vec4 position;
+layout(location=1) in float scale;
+layout(location=2) in float size;
+float func()
+{
+       float scale = size*2.0;
+       return scale*scale;
+}
+void main()
+{
+       float size = scale+1.0;
+       float s = func()+1.0;
+       gl_Position = position*size*size*s*s;
+}
+
+/* Expected output: vertex
+layout(location=0) in vec4 position;
+layout(location=1) in float scale;
+layout(location=2) in float size;
+void main()
+{
+       float _main_size = scale+1.0;
+       float _func_scale = size*2.0;
+       float s = _func_scale*_func_scale+1.0;
+       gl_Position = position*_main_size*_main_size*s*s;
+}
+*/