[gtk+/wip/otte/shader] gsksl: Error on redeclaration of variable

[Benjamin Otte <otte src gnome org>]

commit a966940e05a15a27680605f267bf3e7923be3b07
Author: Benjamin Otte <otte redhat com>
Date:   Wed Oct 25 05:53:56 2017 +0200

    gsksl: Error on redeclaration of variable
    
    Also, emit shadow warnings whenever possible.

 gsk/gsksldeclaration.c                             |    6 +-
 gsk/gskslfunction.c                                |   17 +------
 gsk/gskslscope.c                                   |   51 ++++++++++++++++++++
 gsk/gskslscopeprivate.h                            |    3 +
 gsk/gskslstatement.c                               |    2 +-
 .../gsksl/errors/duplicate-function-parameter.glsl |   10 ++++
 .../local-redeclaration-of-function-parameter.glsl |   11 ++++
 .../local-redeclaration-of-local-variable.glsl     |    5 ++
 ...tion-of-global-variable-in-anonymous-block.glsl |    9 ++++
 .../errors/redeclaration-of-global-variable.glsl   |    7 +++
 10 files changed, 101 insertions(+), 20 deletions(-)
---
diff --git a/gsk/gsksldeclaration.c b/gsk/gsksldeclaration.c
index 5431093..539f178 100644
--- a/gsk/gsksldeclaration.c
+++ b/gsk/gsksldeclaration.c
@@ -316,7 +316,7 @@ gsk_sl_declaration_parse_block (GskSlScope           *scope,
       
       variable = gsk_sl_declaration_new (&GSK_SL_DECLARATION_VARIABLE);
       variable->variable = gsk_sl_variable_new (name, type, qualifier, NULL);
-      gsk_sl_scope_add_variable (scope, variable->variable);
+      gsk_sl_scope_try_add_variable (scope, preproc, variable->variable);
 
       token = gsk_sl_preprocessor_get (preproc);
     }
@@ -334,7 +334,7 @@ gsk_sl_declaration_parse_block (GskSlScope           *scope,
           GskSlVariable *sub;
 
           sub = gsk_sl_variable_new_block_member (variable->variable, i);
-          gsk_sl_scope_add_variable (scope, sub);
+          gsk_sl_scope_try_add_variable (scope, preproc, sub);
           gsk_sl_variable_unref (sub);
         }
     }
@@ -413,7 +413,7 @@ gsk_sl_declaration_parse_variable (GskSlScope           *scope,
   variable = gsk_sl_declaration_new (&GSK_SL_DECLARATION_VARIABLE);
   variable->variable = gsk_sl_variable_new (name, type, qualifier, initial_value);
   variable->initial = initial;
-  gsk_sl_scope_add_variable (scope, variable->variable);
+  gsk_sl_scope_try_add_variable (scope, preproc, variable->variable);
 
   gsk_sl_type_unref (type);
 
diff --git a/gsk/gskslfunction.c b/gsk/gskslfunction.c
index a7c508b..33b1aa3 100644
--- a/gsk/gskslfunction.c
+++ b/gsk/gskslfunction.c
@@ -604,21 +604,6 @@ gsk_sl_function_new_parse (GskSlScope        *scope,
           if (gsk_sl_token_is (token, GSK_SL_TOKEN_IDENTIFIER))
             {
               char *name;
-              guint i;
-
-              if (gsk_sl_scope_lookup_variable (function->scope, token->str))
-                {
-                  for (i = 0; i < arguments->len; i++)
-                    {
-                      if (g_str_equal (gsk_sl_variable_get_name (g_ptr_array_index (arguments, i)), 
token->str))
-                        {
-                          gsk_sl_preprocessor_error (preproc, DECLARATION, "Duplicate argument name 
\"%s\".", token->str);
-                          break;
-                        }
-                    }
-                  if (i == arguments->len)
-                    gsk_sl_preprocessor_warn (preproc, SHADOW, "Function argument \"%s\" shadows global 
variable of same name.", token->str);
-                }
 
               name = g_strdup (token->str);
               gsk_sl_preprocessor_consume (preproc, (GskSlStatement *) function);
@@ -631,7 +616,7 @@ gsk_sl_function_new_parse (GskSlScope        *scope,
                                                                            type);
               g_ptr_array_add (arguments, variable);
               
-              gsk_sl_scope_add_variable (function->scope, variable);
+              gsk_sl_scope_try_add_variable (function->scope, preproc, variable);
 
               g_free (name);
             }
diff --git a/gsk/gskslscope.c b/gsk/gskslscope.c
index 07fddf7..f4b8f7d 100644
--- a/gsk/gskslscope.c
+++ b/gsk/gskslscope.c
@@ -21,6 +21,8 @@
 #include "gskslscopeprivate.h"
 
 #include "gskslfunctionprivate.h"
+#include "gskslpreprocessorprivate.h"
+#include "gskslqualifierprivate.h"
 #include "gsksltypeprivate.h"
 #include "gskslvariableprivate.h"
 
@@ -153,6 +155,55 @@ gsk_sl_scope_add_variable (GskSlScope    *scope,
   g_hash_table_replace (scope->variables, (gpointer) gsk_sl_variable_get_name (variable), 
gsk_sl_variable_ref (variable));
 }
 
+static const char *
+gsk_sl_scope_describe_variable (GskSlVariable *variable)
+{
+  switch (gsk_sl_qualifier_get_location (gsk_sl_variable_get_qualifier (variable)))
+  {
+    default:
+      g_assert_not_reached ();
+    case GSK_SL_QUALIFIER_GLOBAL:
+      return "global variable";
+    case GSK_SL_QUALIFIER_PARAMETER:
+      return "function parameter";
+    case GSK_SL_QUALIFIER_LOCAL:
+      return "local variable";
+    }
+}
+
+void
+gsk_sl_scope_try_add_variable (GskSlScope        *scope,
+                               GskSlPreprocessor *preproc,
+                               GskSlVariable     *variable)
+{
+  GskSlVariable *shadow;
+
+  /* only look in current scope for error */
+  shadow = g_hash_table_lookup (scope->variables, gsk_sl_variable_get_name (variable));
+  if (shadow)
+    {
+      gsk_sl_preprocessor_error (preproc, DECLARATION,
+                                 "Redefinition of %s \"%s\".",
+                                 gsk_sl_scope_describe_variable (shadow),
+                                 gsk_sl_variable_get_name (variable));
+      return;
+    }
+
+  if (scope->parent)
+    {
+      shadow = gsk_sl_scope_lookup_variable (scope->parent, gsk_sl_variable_get_name (variable));
+      if (shadow)
+        {
+          gsk_sl_preprocessor_warn (preproc, SHADOW,
+                                    "Name \"%s\" shadows %s of same name.",
+                                    gsk_sl_variable_get_name (variable),
+                                    gsk_sl_scope_describe_variable (shadow));
+        }
+    }
+
+  gsk_sl_scope_add_variable (scope, variable);
+}
+
 GskSlVariable *
 gsk_sl_scope_lookup_variable (GskSlScope *scope,
                               const char *name)
diff --git a/gsk/gskslscopeprivate.h b/gsk/gskslscopeprivate.h
index cb4bac8..4cd81b2 100644
--- a/gsk/gskslscopeprivate.h
+++ b/gsk/gskslscopeprivate.h
@@ -42,6 +42,9 @@ gboolean                gsk_sl_scope_is_global                  (const GskSlScop
 
 void                    gsk_sl_scope_add_variable               (GskSlScope           *scope,
                                                                  GskSlVariable        *variable);
+void                    gsk_sl_scope_try_add_variable           (GskSlScope           *scope,
+                                                                 GskSlPreprocessor    *preproc,
+                                                                 GskSlVariable        *variable);
 GskSlVariable *         gsk_sl_scope_lookup_variable            (GskSlScope           *scope,
                                                                  const char           *name);
 void                    gsk_sl_scope_add_function               (GskSlScope           *scope,
diff --git a/gsk/gskslstatement.c b/gsk/gskslstatement.c
index 1b2547b..e1d26b4 100644
--- a/gsk/gskslstatement.c
+++ b/gsk/gskslstatement.c
@@ -743,7 +743,7 @@ gsk_sl_statement_parse_declaration (GskSlScope           *scope,
 
   declaration->variable = gsk_sl_variable_new (name, type, qualifier, initial_value);
   g_free (name);
-  gsk_sl_scope_add_variable (scope, declaration->variable);
+  gsk_sl_scope_try_add_variable (scope, stream, declaration->variable);
   gsk_sl_type_unref (type);
 
   return (GskSlStatement *) declaration;
diff --git a/testsuite/gsksl/errors/duplicate-function-parameter.glsl 
b/testsuite/gsksl/errors/duplicate-function-parameter.glsl
new file mode 100644
index 0000000..8d1e19c
--- /dev/null
+++ b/testsuite/gsksl/errors/duplicate-function-parameter.glsl
@@ -0,0 +1,10 @@
+void
+foo (int x, int x)
+{
+}
+
+void
+main ()
+{
+  foo (4, 2);
+}
diff --git a/testsuite/gsksl/errors/local-redeclaration-of-function-parameter.glsl 
b/testsuite/gsksl/errors/local-redeclaration-of-function-parameter.glsl
new file mode 100644
index 0000000..2c0d2c5
--- /dev/null
+++ b/testsuite/gsksl/errors/local-redeclaration-of-function-parameter.glsl
@@ -0,0 +1,11 @@
+
+void
+foo (int x)
+{
+  int x;
+}
+
+void main()
+{
+  foo (1);
+}
diff --git a/testsuite/gsksl/errors/local-redeclaration-of-local-variable.glsl 
b/testsuite/gsksl/errors/local-redeclaration-of-local-variable.glsl
new file mode 100644
index 0000000..47503f7
--- /dev/null
+++ b/testsuite/gsksl/errors/local-redeclaration-of-local-variable.glsl
@@ -0,0 +1,5 @@
+void main()
+{
+  int x;
+  int x;
+}
diff --git a/testsuite/gsksl/errors/redeclaration-of-global-variable-in-anonymous-block.glsl 
b/testsuite/gsksl/errors/redeclaration-of-global-variable-in-anonymous-block.glsl
new file mode 100644
index 0000000..61f0c3e
--- /dev/null
+++ b/testsuite/gsksl/errors/redeclaration-of-global-variable-in-anonymous-block.glsl
@@ -0,0 +1,9 @@
+int x;
+uniform Block {
+  int x;
+};
+
+void
+main ()
+{
+}
diff --git a/testsuite/gsksl/errors/redeclaration-of-global-variable.glsl 
b/testsuite/gsksl/errors/redeclaration-of-global-variable.glsl
new file mode 100644
index 0000000..a9e2908
--- /dev/null
+++ b/testsuite/gsksl/errors/redeclaration-of-global-variable.glsl
@@ -0,0 +1,7 @@
+int x;
+int x;
+
+void
+main ()
+{
+}