From bc35ad35efa7038cbf844e5cd7f29f9094b54ae2 Mon Sep 17 00:00:00 2001
From: Frederich Munch <marsupial@users.noreply.github.com>
Date: Thu, 28 Dec 2017 16:35:22 -0500
Subject: [PATCH 1/6] Refactor ASTNodes who have both a Symbol and name into
 common base class ASTnamed_symbol.

---
 CMakeLists.txt                       |   3 +-
 src/liboslcomp/ast.cpp               | 171 +++++++++++++++++++--------
 src/liboslcomp/ast.h                 |  78 +++++++++---
 src/liboslcomp/oslgram.y             |  18 +--
 testsuite/oslc-err-names/ref/out.txt |  14 +++
 testsuite/oslc-err-names/run.py      |   5 +
 testsuite/oslc-err-names/test.osl    |  30 +++++
 7 files changed, 234 insertions(+), 85 deletions(-)
 create mode 100644 testsuite/oslc-err-names/ref/out.txt
 create mode 100755 testsuite/oslc-err-names/run.py
 create mode 100644 testsuite/oslc-err-names/test.osl

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 5555b42df..653d9962c 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -271,8 +271,9 @@ TESTSUITE ( aastep allowconnect-err and-or-not-synonyms arithmetic
             oslc-comma oslc-D
             oslc-err-arrayindex oslc-err-closuremul oslc-err-field
             oslc-err-format oslc-err-funcoverload
-            oslc-err-intoverflow oslc-err-noreturn oslc-err-notfunc
+            oslc-err-intoverflow
             oslc-err-initlist-args oslc-err-initlist-return
+            oslc-err-names oslc-err-noreturn oslc-err-notfunc
             oslc-err-outputparamvararray oslc-err-paramdefault
             oslc-err-struct-array-init oslc-err-struct-ctr
             oslc-err-struct-dup oslc-err-struct-print
diff --git a/src/liboslcomp/ast.cpp b/src/liboslcomp/ast.cpp
index 4bf8195f7..53fcc77c1 100644
--- a/src/liboslcomp/ast.cpp
+++ b/src/liboslcomp/ast.cpp
@@ -291,6 +291,109 @@ ASTNode::list_to_types_string (const ASTNode *node)
 
 
 
+ASTnamed_symbol::ASTnamed_symbol (NodeType node_type, OSLCompilerImpl *comp,
+                                  ustring name)
+    : ASTNode (node_type, comp, Nothing), m_name(name), m_sym(nullptr)
+{
+}
+
+
+
+ASTnamed_symbol::ASTnamed_symbol (NodeType node_type, OSLCompilerImpl *comp,
+                                  ustring name, ASTNode *a)
+    : ASTNode (node_type, comp, Nothing, a), m_name(name), m_sym(nullptr)
+{
+}
+
+
+
+ASTnamed_symbol::ASTnamed_symbol (NodeType node_type, OSLCompilerImpl *comp,
+                                  ustring name, ASTNode *a, ASTNode *b)
+    : ASTNode (node_type, comp, Nothing, a, b), m_name(name), m_sym(nullptr)
+{
+}
+
+
+
+ASTnamed_symbol::ASTnamed_symbol (NodeType node_type, OSLCompilerImpl *comp,
+                                  ustring name, ASTNode *a, ASTNode *b,
+                                  ASTNode *c)
+    : ASTNode (node_type, comp, Nothing, a, b, c), m_name(name), m_sym(nullptr)
+{
+}
+
+
+
+void
+ASTnamed_symbol::check_reserved (ustring name, OSLCompilerImpl *comp)
+{
+    if (Strutil::starts_with(name, "___")) {
+        comp->error (comp->filename(), comp->lineno(),
+                     "'%s' : sorry, can't start with three underscores",
+                     name);
+    }
+}
+
+
+
+std::string
+ASTnamed_symbol::previous_decl (ASTNode* node)
+{
+    if (! node)
+        return "";
+
+    return Strutil::format ("\n\t\tprevious declaration was at %s:%d",
+                             OIIO::Filesystem::filename(node->sourcefile().string()),
+                             node->sourceline());
+}
+
+
+
+Symbol*
+ASTnamed_symbol::validate (ustring name, OSLCompilerImpl *comp,
+                           Validation vflags, int allowed)
+{
+    check_reserved (name, comp);
+
+    Symbol *f;
+	bool shadow = vflags & warn_shadow;
+    if (! (vflags & check_clashes)) {
+        f = comp->symtab().find (name);
+        if (! f) {
+            comp->error (comp->filename(), comp->lineno(),
+                         "'%s' was not declared in this scope", name);
+            // FIXME -- would be fun to troll through the symtab and try to
+            // find the things that almost matched and offer suggestions.
+        }
+        else if (shadow && f->symtype() == SymTypeFunction) {
+            comp->error (comp->filename(), comp->lineno(),
+                         "function '%s' can't be used as a variable", name);
+        }
+    } else {
+        f = comp->symtab().clash (name);
+        // If no symbol, or symbol matches allowed type: no clash.
+        if (! f || f->symtype() == allowed)
+            return f;
+
+        std::string e = Strutil::format ("'%s' already declared in this scope",
+                                         name);
+        e += previous_decl (f->node());
+
+        if (shadow) {
+            // special case: only a warning for param to mask global function
+            shadow = f->scope() == 0 && f->symtype() == SymTypeFunction;
+        }
+
+        if (shadow)
+            comp->warning(comp->filename(), comp->lineno(), "%s", e);
+        else
+            comp->error(comp->filename(), comp->lineno(), "%s", e);
+    }
+    return f;
+}
+
+
+
 ASTshader_declaration::ASTshader_declaration (OSLCompilerImpl *comp,
                                 int stype, ustring name, ASTNode *form,
                                 ASTNode *stmts, ASTNode *meta)
@@ -346,8 +449,8 @@ ASTfunction_declaration::ASTfunction_declaration (OSLCompilerImpl *comp,
                              TypeSpec type, ustring name,
                              ASTNode *form, ASTNode *stmts, ASTNode *meta,
                              int sourceline_start)
-    : ASTNode (function_declaration_node, comp, 0, meta, form, stmts),
-      m_name(name), m_sym(NULL), m_is_builtin(false)
+    : ASTnamed_symbol (function_declaration_node, comp, name, meta, form, stmts),
+      m_is_builtin(false)
 {
     // Some trickery -- the compiler's idea of the "current" source line
     // is the END of the function body, so if a hint was passed about the
@@ -355,11 +458,8 @@ ASTfunction_declaration::ASTfunction_declaration (OSLCompilerImpl *comp,
     if (sourceline_start >= 0)
         m_sourceline = sourceline_start;
 
-    if (Strutil::starts_with (name, "___"))
-        error ("\"%s\" : sorry, can't start with three underscores", name);
-
     // Get a pointer to the first of the existing symbols of that name.
-    Symbol *existing_syms = comp->symtab().clash (name);
+    Symbol *existing_syms = validate (check_clashes, SymTypeFunction);
     if (existing_syms && existing_syms->symtype() != SymTypeFunction) {
         error ("\"%s\" already declared in this scope as a %s",
                name, existing_syms->typespec());
@@ -493,8 +593,7 @@ ASTvariable_declaration::ASTvariable_declaration (OSLCompilerImpl *comp,
                                                   ustring name, ASTNode *init,
                                                   bool isparam, bool ismeta,
                                                   bool isoutput, bool initlist)
-    : ASTNode (variable_declaration_node, comp, 0, init, NULL /* meta */),
-      m_name(name), m_sym(NULL),
+    : ASTnamed_symbol (variable_declaration_node, comp, name, init, NULL /* meta */),
       m_isparam(isparam), m_isoutput(isoutput), m_ismetadata(ismeta),
       m_initlist(initlist)
 {
@@ -505,25 +604,11 @@ ASTvariable_declaration::ASTvariable_declaration (OSLCompilerImpl *comp,
     }
 
     m_typespec = type;
-    Symbol *f = comp->symtab().clash (name);
-    if (f  &&  ! m_ismetadata) {
-        std::string e = Strutil::format ("\"%s\" already declared in this scope", name.c_str());
-        if (f->node()) {
-            std::string filename = OIIO::Filesystem::filename(f->node()->sourcefile().string());
-            e += Strutil::format ("\n\t\tprevious declaration was at %s:%d",
-                                  filename, f->node()->sourceline());
-        }
-        if (f->scope() == 0 && f->symtype() == SymTypeFunction && isparam) {
-            // special case: only a warning for param to mask global function
-            warning ("%s", e.c_str());
-        } else {
-            error ("%s", e.c_str());
-        }
-    }
-    if (name[0] == '_' && name[1] == '_' && name[2] == '_') {
-        error ("\"%s\" : sorry, can't start with three underscores",
-               name.c_str());
-    }
+    if (! m_ismetadata)
+        validate (warn_function_clash);
+    else
+        check_reserved ();
+
     SymType symtype = isparam ? (isoutput ? SymTypeOutputParam : SymTypeParam)
                               : SymTypeLocal;
     // Sneaky debugging aid: a local that starts with "__debug_tmp__"
@@ -584,20 +669,11 @@ ASTvariable_declaration::print (std::ostream &out, int indentlevel) const
 
 
 ASTvariable_ref::ASTvariable_ref (OSLCompilerImpl *comp, ustring name)
-    : ASTNode (variable_ref_node, comp), m_name(name), m_sym(NULL)
+    : ASTnamed_symbol (variable_ref_node, comp, name)
 {
-    m_sym = comp->symtab().find (name);
-    if (! m_sym) {
-        error ("'%s' was not declared in this scope", name.c_str());
-        // FIXME -- would be fun to troll through the symtab and try to
-        // find the things that almost matched and offer suggestions.
-        return;
-    }
-    if (m_sym->symtype() == SymTypeFunction) {
-        error ("function '%s' can't be used as a variable", name.c_str());
-        return;
-    }
-    m_typespec = m_sym->typespec();
+    m_sym = validate (warn_function_exists);
+    if (m_sym)
+        m_typespec = m_sym->typespec();
 }
 
 
@@ -1111,25 +1187,18 @@ ASTtype_constructor::childname (size_t i) const
 
 ASTfunction_call::ASTfunction_call (OSLCompilerImpl *comp, ustring name,
                                     ASTNode *args, FunctionSymbol *funcsym)
-    : ASTNode (function_call_node, comp, 0, args), m_name(name),
-      m_sym(funcsym ? funcsym : comp->symtab().find (name)), // Look it up.
+    : ASTnamed_symbol (function_call_node, comp, name, args),
       m_poly(funcsym),    // Default - resolved symbol or null
       m_argread(~1),      // Default - all args are read except the first
       m_argwrite(1),      // Default - first arg only is written by the op
       m_argtakesderivs(0) // Default - doesn't take derivs
 {
-    if (! m_sym) {
-        error ("function '%s' was not declared in this scope", name);
-        // FIXME -- would be fun to troll through the symtab and try to
-        // find the things that almost matched and offer suggestions.
+    m_sym = funcsym ? funcsym : validate (err_must_exist); // Look it up.
+    if (! m_sym || is_struct_ctr())
         return;
-    }
-    if (is_struct_ctr()) {
-        return;  // It's a struct constructor
-    }
     if (m_sym->symtype() != SymTypeFunction) {
         error ("'%s' is not a function", name.c_str());
-        m_sym = NULL;
+        m_sym = nullptr;
         return;
     }
 }
diff --git a/src/liboslcomp/ast.h b/src/liboslcomp/ast.h
index de6965771..57bde84ec 100644
--- a/src/liboslcomp/ast.h
+++ b/src/liboslcomp/ast.h
@@ -392,6 +392,60 @@ class ASTNode : public OIIO::RefCnt {
 };
 
 
+// Base class for any ASTNode that holds a name and Symbol that allows for
+// common error handling of using reserved or clashing names.
+class ASTnamed_symbol : public ASTNode
+{
+public:
+    ASTnamed_symbol (NodeType node_type, OSLCompilerImpl *comp, ustring name);
+
+    ASTnamed_symbol (NodeType node_type, OSLCompilerImpl *comp, ustring name,
+                     ASTNode *a);
+
+    ASTnamed_symbol (NodeType node_type, OSLCompilerImpl *comp, ustring name,
+                     ASTNode *a, ASTNode *b);
+
+    ASTnamed_symbol (NodeType node_type, OSLCompilerImpl *comp, ustring name,
+                     ASTNode *a, ASTNode *b, ASTNode *c);
+
+    Symbol *sym () const { return m_sym; }
+    ustring name () const { return m_name; }
+    std::string mangled () const { ASSERT (m_sym); return m_sym->mangled(); }
+    
+    enum Validation {
+        err_must_exist  = 0,  ///< Error when the named Symbol does not exist.
+        check_clashes   = 1,  ///< If named Symbol exists error as a clash.
+        warn_shadow     = 2,  ///< Warn if name shadows a known function.
+
+        /// Convenient aliases of validation schemes.
+        warn_function_exists = err_must_exist | warn_shadow,
+        warn_function_clash  = check_clashes | warn_shadow,
+    };
+
+    /// Validate that m_name is a legal name.
+    static void check_reserved (ustring name, OSLCompilerImpl *comp);
+
+    /// Validate that m_name is a legal name, and doesn't conflict with rules
+    /// given in vflags allowing duplicate symbol if its SymType matches allowed.
+    static Symbol* validate (ustring name, OSLCompilerImpl *comp,
+                             Validation vflags, int allowed = -1);
+
+protected:
+    void check_reserved () {
+        ASTnamed_symbol::check_reserved (m_name, m_compiler);
+    }
+
+    Symbol* validate (Validation vflags, int allowed = -1) {
+        return validate (m_name, m_compiler, vflags, allowed);
+    }
+
+    static std::string previous_decl (ASTNode* node);
+
+    ustring m_name;     ///< Name of the symbol (unmangled)
+    Symbol *m_sym;      ///< Ptr to the symbol this declares
+};
+
+
 
 class ASTshader_declaration : public ASTNode
 {
@@ -416,7 +470,7 @@ class ASTshader_declaration : public ASTNode
 
 
 
-class ASTfunction_declaration : public ASTNode
+class ASTfunction_declaration : public ASTnamed_symbol
 {
 public:
     ASTfunction_declaration (OSLCompilerImpl *comp, TypeSpec type, ustring name,
@@ -439,14 +493,12 @@ class ASTfunction_declaration : public ASTNode
     void add_meta (ref meta);
 
 private:
-    ustring m_name;
-    Symbol *m_sym;
     bool m_is_builtin;
 };
 
 
 
-class ASTvariable_declaration : public ASTNode
+class ASTvariable_declaration : public ASTnamed_symbol
 {
 public:
     ASTvariable_declaration (OSLCompilerImpl *comp, const TypeSpec &type,
@@ -468,9 +520,6 @@ class ASTvariable_declaration : public ASTNode
         m_children[1] = meta;  // beware changing the order!
     }
 
-    Symbol *sym () const { return m_sym; }
-    ustring name () const { return m_name; }
-
     bool is_output () const { return m_isoutput; }
 
     /// For shader params, generate the string that gives the
@@ -487,8 +536,6 @@ class ASTvariable_declaration : public ASTNode
     }
 
 private:
-    ustring m_name;     ///< Name of the symbol (unmangled)
-    Symbol *m_sym;      ///< Ptr to the symbol this declares
     bool m_isparam;     ///< Is this a parameter?
     bool m_isoutput;    ///< Is this an output parameter?
     bool m_ismetadata;  ///< Is this declaration a piece of metadata?
@@ -501,7 +548,7 @@ class ASTvariable_declaration : public ASTNode
 
 
 
-class ASTvariable_ref : public ASTNode
+class ASTvariable_ref : public ASTnamed_symbol
 {
 public:
     ASTvariable_ref (OSLCompilerImpl *comp, ustring name);
@@ -510,12 +557,6 @@ class ASTvariable_ref : public ASTNode
     void print (std::ostream &out, int indentlevel=0) const;
     TypeSpec typecheck (TypeSpec expected);
     Symbol *codegen (Symbol *dest = NULL);
-    ustring name () const { return m_name; }
-    std::string mangled () const { return m_sym->mangled(); }
-    Symbol *sym () const { return m_sym; }
-private:
-    ustring m_name;
-    Symbol *m_sym;
 };
 
 
@@ -882,7 +923,8 @@ class ASTtypecast_expression : public ASTNode
 
 
 
-class ASTfunction_call : public ASTNode
+
+class ASTfunction_call : public ASTnamed_symbol
 {
 public:
     ASTfunction_call (OSLCompilerImpl *comp, ustring name, ASTNode *args,
@@ -985,8 +1027,6 @@ class ASTfunction_call : public ASTNode
                                  ustring formal, ustring actual,
                                  Symbol *arrayindex = NULL);
 
-    ustring m_name;                 ///< Name of the function being called
-    Symbol *m_sym;                  ///< Symbol of the function
     FunctionSymbol *m_poly;         ///< The specific polymorphic variant
     unsigned int m_argread;         ///< Bit field - which args are read
     unsigned int m_argwrite;        ///< Bit field - which args are written
diff --git a/src/liboslcomp/oslgram.y b/src/liboslcomp/oslgram.y
index af4842b96..e3fa8d608 100644
--- a/src/liboslcomp/oslgram.y
+++ b/src/liboslcomp/oslgram.y
@@ -347,20 +347,8 @@ struct_declaration
         : STRUCT IDENTIFIER '{' 
                 {
                     ustring name ($2);
-                    Symbol *s = oslcompiler->symtab().clash (name);
-                    if (s) {
-                        oslcompiler->error (oslcompiler->filename(), 
-                                            oslcompiler->lineno(), 
-                                            "\"%s\" already declared in this scope",
-                                            name.c_str());
-                        // FIXME -- print the file and line of the other definition
-                    }
-                    if (name[0] == '_' && name[1] == '_' && name[2] == '_') {
-                        oslcompiler->error (oslcompiler->filename(), 
-                            oslcompiler->lineno(),
-                            "\"%s\" : sorry, can't start with three underscores",
-                            name.c_str());
-                    }
+                    ASTnamed_symbol::validate (name, oslcompiler,
+                                               ASTnamed_symbol::check_clashes);
                     oslcompiler->symtab().new_struct (name);
                 }
           field_declarations '}' ';'
@@ -387,6 +375,7 @@ typed_field
         : IDENTIFIER
                 {
                     ustring name ($1);
+                    ASTnamed_symbol::check_reserved (name, oslcompiler);
                     TypeSpec t = oslcompiler->current_typespec();
                     StructSpec *s = oslcompiler->symtab().current_struct();
                     if (s->lookup_field (name) >= 0)
@@ -402,6 +391,7 @@ typed_field
                 {
                     // Grab the current declaration type, modify it to be array
                     ustring name ($1);
+                    ASTnamed_symbol::check_reserved (name, oslcompiler);
                     TypeSpec t = oslcompiler->current_typespec();
                     t.make_array ($2);
                     if (t.arraylength() < 1)
diff --git a/testsuite/oslc-err-names/ref/out.txt b/testsuite/oslc-err-names/ref/out.txt
new file mode 100644
index 000000000..fb86e21bf
--- /dev/null
+++ b/testsuite/oslc-err-names/ref/out.txt
@@ -0,0 +1,14 @@
+test.osl:8: error: function 'reusename' can't be used as a variable
+test.osl:9: error: function 'globalfunc' can't be used as a variable
+test.osl:12: error: 'reusename' already declared in this scope
+		previous declaration was at test.osl:7
+test.osl:16: error: 'parm0' already declared in this scope
+		previous declaration was at test.osl:15
+test.osl:19: error: '___parm' : sorry, can't start with three underscores
+test.osl:19: error: '___meta' : sorry, can't start with three underscores
+test.osl:20: error: '___vartoo' : sorry, can't start with three underscores
+test.osl:21: error: '___threeunderscores' : sorry, can't start with three underscores
+test.osl:23: error: '___invalid' : sorry, can't start with three underscores
+test.osl:24: error: '___invalidfield' : sorry, can't start with three underscores
+test.osl:25: error: '___invalidarray' : sorry, can't start with three underscores
+FAILED test.osl
diff --git a/testsuite/oslc-err-names/run.py b/testsuite/oslc-err-names/run.py
new file mode 100755
index 000000000..d901e097a
--- /dev/null
+++ b/testsuite/oslc-err-names/run.py
@@ -0,0 +1,5 @@
+#!/usr/bin/env python
+
+# command = oslc("test.osl")
+# don't even need that -- it's automatic
+failureok = 1     # this test is expected to have oslc errors
diff --git a/testsuite/oslc-err-names/test.osl b/testsuite/oslc-err-names/test.osl
new file mode 100644
index 000000000..4976757dc
--- /dev/null
+++ b/testsuite/oslc-err-names/test.osl
@@ -0,0 +1,30 @@
+
+void globalfunc() {
+}
+
+void nestedfunc() {
+    void reusename() {
+    }
+    reusename;
+    globalfunc;
+
+    int globalfunc = 64;
+    int reusename = 32;
+}
+
+void reuseparm(int parm0) {
+    float parm0 = 3;
+}
+
+void ___threeunderscores(int ___parm, int parm2 [[ string ___meta = ""]] ) {
+    int ___vartoo = 4;
+}
+
+struct ___invalid {
+    float ___invalidfield;
+    float ___invalidarray[3];
+};
+
+shader test ()
+{
+}

From 5957d236e9a48914d37f2e3f068262147fe70776 Mon Sep 17 00:00:00 2001
From: Frederich Munch <marsupial@users.noreply.github.com>
Date: Fri, 29 Dec 2017 00:49:16 -0500
Subject: [PATCH 2/6] Fail gracefully and continue parsing when using 'this'
 keyword.

---
 src/liboslcomp/ast.cpp               | 13 ++++++++++---
 src/liboslcomp/ast.h                 |  6 +++---
 src/liboslcomp/osllex.l              |  2 +-
 testsuite/oslc-err-names/ref/out.txt | 14 ++++++++++++++
 testsuite/oslc-err-names/test.osl    | 20 +++++++++++++++++++-
 5 files changed, 47 insertions(+), 8 deletions(-)

diff --git a/src/liboslcomp/ast.cpp b/src/liboslcomp/ast.cpp
index 53fcc77c1..fe775cfd2 100644
--- a/src/liboslcomp/ast.cpp
+++ b/src/liboslcomp/ast.cpp
@@ -324,14 +324,20 @@ ASTnamed_symbol::ASTnamed_symbol (NodeType node_type, OSLCompilerImpl *comp,
 
 
 
-void
+bool
 ASTnamed_symbol::check_reserved (ustring name, OSLCompilerImpl *comp)
 {
     if (Strutil::starts_with(name, "___")) {
         comp->error (comp->filename(), comp->lineno(),
                      "'%s' : sorry, can't start with three underscores",
                      name);
+        return false;
+    } else if (name == "this") {
+        comp->error (comp->filename(), comp->lineno(),
+                     "'this' not allowed in this context");
+        return false;
     }
+    return true;
 }
 
 
@@ -597,6 +603,9 @@ ASTvariable_declaration::ASTvariable_declaration (OSLCompilerImpl *comp,
       m_isparam(isparam), m_isoutput(isoutput), m_ismetadata(ismeta),
       m_initlist(initlist)
 {
+    if (! check_reserved ())
+        return;
+
     if (m_initlist && init) {
         // Typecheck the init list early.
         ASSERT (init->nodetype() == compound_initializer_node);
@@ -606,8 +615,6 @@ ASTvariable_declaration::ASTvariable_declaration (OSLCompilerImpl *comp,
     m_typespec = type;
     if (! m_ismetadata)
         validate (warn_function_clash);
-    else
-        check_reserved ();
 
     SymType symtype = isparam ? (isoutput ? SymTypeOutputParam : SymTypeParam)
                               : SymTypeLocal;
diff --git a/src/liboslcomp/ast.h b/src/liboslcomp/ast.h
index 57bde84ec..0c4bfbd5e 100644
--- a/src/liboslcomp/ast.h
+++ b/src/liboslcomp/ast.h
@@ -423,7 +423,7 @@ class ASTnamed_symbol : public ASTNode
     };
 
     /// Validate that m_name is a legal name.
-    static void check_reserved (ustring name, OSLCompilerImpl *comp);
+    static bool check_reserved (ustring name, OSLCompilerImpl *comp);
 
     /// Validate that m_name is a legal name, and doesn't conflict with rules
     /// given in vflags allowing duplicate symbol if its SymType matches allowed.
@@ -431,8 +431,8 @@ class ASTnamed_symbol : public ASTNode
                              Validation vflags, int allowed = -1);
 
 protected:
-    void check_reserved () {
-        ASTnamed_symbol::check_reserved (m_name, m_compiler);
+    bool check_reserved () {
+        return ASTnamed_symbol::check_reserved (m_name, m_compiler);
     }
 
     Symbol* validate (Validation vflags, int allowed = -1) {
diff --git a/src/liboslcomp/osllex.l b/src/liboslcomp/osllex.l
index 5f4ab1eaf..3d8f005c4 100644
--- a/src/liboslcomp/osllex.l
+++ b/src/liboslcomp/osllex.l
@@ -179,7 +179,7 @@ void preprocess (const char *yytext);
 "bool"|"case"|"char"|"class"|"const"|"default"|"double" |    \
 "enum"|"extern"|"false"|"friend"|"inline"|"long"|"private" | \
 "protected"|"short"|"signed"|"sizeof"|"static"|"struct" |    \
-"switch"|"template"|"this"|"true"|"typedef"|"uniform" |      \
+"switch"|"template"|"true"|"typedef"|"uniform" |      \
 "union"|"unsigned"|"varying"|"virtual" {
                             oslcompiler->error (oslcompiler->filename(),
                                                 oslcompiler->lineno(),
diff --git a/testsuite/oslc-err-names/ref/out.txt b/testsuite/oslc-err-names/ref/out.txt
index fb86e21bf..79cc2ac25 100644
--- a/testsuite/oslc-err-names/ref/out.txt
+++ b/testsuite/oslc-err-names/ref/out.txt
@@ -11,4 +11,18 @@ test.osl:21: error: '___threeunderscores' : sorry, can't start with three unders
 test.osl:23: error: '___invalid' : sorry, can't start with three underscores
 test.osl:24: error: '___invalidfield' : sorry, can't start with three underscores
 test.osl:25: error: '___invalidarray' : sorry, can't start with three underscores
+test.osl:29: error: 'this' not allowed in this context
+test.osl:34: error: 'this' not allowed in this context
+test.osl:35: error: 'this' not allowed in this context
+test.osl:35: error: 'this' was not declared in this scope
+test.osl:36: error: 'this' not allowed in this context
+test.osl:36: error: 'this' was not declared in this scope
+test.osl:36: error: type 'unknown' does not have a member 'obj'
+test.osl:37: error: 'this' not allowed in this context
+test.osl:38: error: 'this' not allowed in this context
+test.osl:40: error: 'this' not allowed in this context
+test.osl:41: error: 'this' not allowed in this context
+test.osl:42: error: 'this' not allowed in this context
+test.osl:43: error: 'this' not allowed in this context
+test.osl:45: error: 'this' not allowed in this context
 FAILED test.osl
diff --git a/testsuite/oslc-err-names/test.osl b/testsuite/oslc-err-names/test.osl
index 4976757dc..5b1fe8c76 100644
--- a/testsuite/oslc-err-names/test.osl
+++ b/testsuite/oslc-err-names/test.osl
@@ -25,6 +25,24 @@ struct ___invalid {
     float ___invalidarray[3];
 };
 
-shader test ()
+struct sthis {
+    float this;
+};
+
+struct A { float f; };
+
+void this(float this) {
+    this;
+    this.obj;
+    float this;
+    float this[5];
+
+    float this = 5;
+    float this[2] = { 1, 1 };
+    A this = { 5 };
+}
+
+shader test (output int this = 5)
 {
 }
+

From 44e993c71e4e2d2ecfd348957e907281b74d426c Mon Sep 17 00:00:00 2001
From: Frederich Munch <marsupial@users.noreply.github.com>
Date: Fri, 29 Dec 2017 01:01:07 -0500
Subject: [PATCH 3/6] Have SymbolTable track and retrieve the currently opened
 struct.

---
 src/liboslcomp/oslgram.y  |  1 +
 src/liboslcomp/symtab.cpp | 16 ++++++++++++----
 src/liboslcomp/symtab.h   |  5 ++++-
 3 files changed, 17 insertions(+), 5 deletions(-)

diff --git a/src/liboslcomp/oslgram.y b/src/liboslcomp/oslgram.y
index e3fa8d608..a8b61d61d 100644
--- a/src/liboslcomp/oslgram.y
+++ b/src/liboslcomp/oslgram.y
@@ -353,6 +353,7 @@ struct_declaration
                 }
           field_declarations '}' ';'
                 {
+                    oslcompiler->symtab().end_struct ();
                     $$ = 0;
                 }
         ;
diff --git a/src/liboslcomp/symtab.cpp b/src/liboslcomp/symtab.cpp
index b60c0414b..67365cdb2 100644
--- a/src/liboslcomp/symtab.cpp
+++ b/src/liboslcomp/symtab.cpp
@@ -237,9 +237,17 @@ SymbolTable::insert (Symbol *sym)
 int
 SymbolTable::new_struct (ustring name)
 {
-    int structid = TypeSpec::new_struct (new StructSpec (name, scopeid()));
-    insert (new Symbol (name, TypeSpec ("",structid), SymTypeType));
-    return structid;
+    m_structid = TypeSpec::new_struct (new StructSpec (name, scopeid()));
+    insert (new Symbol (name, TypeSpec ("", m_structid), SymTypeType));
+    return m_structid;
+}
+
+
+
+void
+SymbolTable::end_struct ()
+{
+    m_structid = 0;
 }
 
 
@@ -247,7 +255,7 @@ SymbolTable::new_struct (ustring name)
 StructSpec *
 SymbolTable::current_struct ()
 {
-    return TypeSpec::struct_list().back().get();
+    return m_structid ? TypeSpec::struct_list().back().get() : nullptr;
 }
 
 
diff --git a/src/liboslcomp/symtab.h b/src/liboslcomp/symtab.h
index 94cfd390e..3cbe205ee 100644
--- a/src/liboslcomp/symtab.h
+++ b/src/liboslcomp/symtab.h
@@ -208,7 +208,7 @@ class SymbolTable {
     typedef SymbolPtrVec::const_iterator const_iterator;
 
     SymbolTable (OSLCompilerImpl &comp)
-        : m_comp(comp), m_scopeid(-1), m_nextscopeid(0)
+        : m_comp(comp), m_scopeid(-1), m_nextscopeid(0), m_structid(0)
     {
         m_scopetables.reserve (20);  // So unlikely to ever copy tables
         push ();                     // Create scope 0 -- global scope
@@ -246,6 +246,8 @@ class SymbolTable {
 
     StructSpec *current_struct ();
 
+    void end_struct ();
+
     /// Return the current scope ID
     ///
     int scopeid () const {
@@ -283,6 +285,7 @@ class SymbolTable {
     ScopeTable m_allmangled;         ///< All syms, mangled, in a hash table
     int m_scopeid;                   ///< Current scope ID
     int m_nextscopeid;               ///< Next unique scope ID
+    int m_structid;                  ///< Currently declaring a struct
 };
 
 

From 9953d0d230c727a80cd4644b3a1e76a259915a8f Mon Sep 17 00:00:00 2001
From: Frederich Munch <marsupial@users.noreply.github.com>
Date: Fri, 29 Dec 2017 01:31:04 -0500
Subject: [PATCH 4/6] Basic method definition and invocation.

---
 CMakeLists.txt                        |   6 +-
 src/include/osl_pvt.h                 |   4 +
 src/liboslcomp/ast.cpp                | 120 +++++++++++++++++++++-----
 src/liboslcomp/ast.h                  |  21 +++--
 src/liboslcomp/oslgram.y              |  56 ++++++++++--
 src/liboslcomp/typecheck.cpp          |  16 ++--
 testsuite/function-method/ref/out.txt |   9 ++
 testsuite/function-method/run.py      |   3 +
 testsuite/function-method/test.osl    |  26 ++++++
 testsuite/oslc-err-method/ref/out.txt |   4 +
 testsuite/oslc-err-method/run.py      |   5 ++
 testsuite/oslc-err-method/test.osl    |  15 ++++
 12 files changed, 241 insertions(+), 44 deletions(-)
 create mode 100644 testsuite/function-method/ref/out.txt
 create mode 100755 testsuite/function-method/run.py
 create mode 100644 testsuite/function-method/test.osl
 create mode 100644 testsuite/oslc-err-method/ref/out.txt
 create mode 100755 testsuite/oslc-err-method/run.py
 create mode 100644 testsuite/oslc-err-method/test.osl

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 653d9962c..00e787688 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -250,8 +250,8 @@ TESTSUITE ( aastep allowconnect-err and-or-not-synonyms arithmetic
             draw_string
             error-dupes exit exponential
             fprintf
-            function-earlyreturn function-simple function-outputelem
-            function-overloads function-redef
+            function-earlyreturn function-method function-simple
+            function-outputelem function-overloads function-redef
             geomath getattribute-camera getattribute-shader
             getsymbol-nonheap gettextureinfo
             group-outputs groupstring
@@ -273,7 +273,7 @@ TESTSUITE ( aastep allowconnect-err and-or-not-synonyms arithmetic
             oslc-err-format oslc-err-funcoverload
             oslc-err-intoverflow
             oslc-err-initlist-args oslc-err-initlist-return
-            oslc-err-names oslc-err-noreturn oslc-err-notfunc
+            oslc-err-method oslc-err-names oslc-err-noreturn oslc-err-notfunc
             oslc-err-outputparamvararray oslc-err-paramdefault
             oslc-err-struct-array-init oslc-err-struct-ctr
             oslc-err-struct-dup oslc-err-struct-print
diff --git a/src/include/osl_pvt.h b/src/include/osl_pvt.h
index 8ea432c15..e0f9fbaa9 100644
--- a/src/include/osl_pvt.h
+++ b/src/include/osl_pvt.h
@@ -558,6 +558,10 @@ class Symbol {
         m_alias = other;
     }
 
+    /// Get a reference to whatever this symbol aliases.
+    ///
+    const Symbol* alias () const { return m_alias; }
+
     /// Return a string representation ("param", "global", etc.) of the
     /// SymType s.
     static const char *symtype_shortname (SymType s);
diff --git a/src/liboslcomp/ast.cpp b/src/liboslcomp/ast.cpp
index fe775cfd2..8f049dd34 100644
--- a/src/liboslcomp/ast.cpp
+++ b/src/liboslcomp/ast.cpp
@@ -325,14 +325,15 @@ ASTnamed_symbol::ASTnamed_symbol (NodeType node_type, OSLCompilerImpl *comp,
 
 
 bool
-ASTnamed_symbol::check_reserved (ustring name, OSLCompilerImpl *comp)
+ASTnamed_symbol::check_reserved (ustring name, OSLCompilerImpl *comp,
+                                 int vflags)
 {
     if (Strutil::starts_with(name, "___")) {
         comp->error (comp->filename(), comp->lineno(),
                      "'%s' : sorry, can't start with three underscores",
                      name);
         return false;
-    } else if (name == "this") {
+    } else if (!(vflags & allow_this) && name == "this") {
         comp->error (comp->filename(), comp->lineno(),
                      "'this' not allowed in this context");
         return false;
@@ -357,9 +358,9 @@ ASTnamed_symbol::previous_decl (ASTNode* node)
 
 Symbol*
 ASTnamed_symbol::validate (ustring name, OSLCompilerImpl *comp,
-                           Validation vflags, int allowed)
+                           int vflags, int allowed)
 {
-    check_reserved (name, comp);
+    check_reserved (name, comp, vflags);
 
     Symbol *f;
 	bool shadow = vflags & warn_shadow;
@@ -381,19 +382,35 @@ ASTnamed_symbol::validate (ustring name, OSLCompilerImpl *comp,
         if (! f || f->symtype() == allowed)
             return f;
 
-        std::string e = Strutil::format ("'%s' already declared in this scope",
-                                         name);
-        e += previous_decl (f->node());
-
+        std::string msg;
+        ASTNode* hint = f->node();
         if (shadow) {
-            // special case: only a warning for param to mask global function
+            // Downgrade to warning if param to mask global function
             shadow = f->scope() == 0 && f->symtype() == SymTypeFunction;
+
+            // also downgrade to warning if local variable shadowing a field
+            if (!shadow && f->alias() && f->alias()->node()) {
+                if (f->alias()->node()->typespec().is_structure() &&
+                    f->alias()->name() == Strutil::format("this.%s", name)) {
+                    msg = Strutil::format ("\"%s\" shadows a field with the same name",
+                                           name);
+                    // FIXME: Would be nice to hint about where the field was
+                    // declared, but f->alias()->node() is the 'this.field' node.
+                    hint = nullptr;
+                    shadow = true;
+                }
+            }
         }
 
+        if (msg.empty())
+            msg = Strutil::format ("'%s' already declared in this scope", name);
+
+        msg += previous_decl (hint);
+
         if (shadow)
-            comp->warning(comp->filename(), comp->lineno(), "%s", e);
+            comp->warning(comp->filename(), comp->lineno(), "%s", msg);
         else
-            comp->error(comp->filename(), comp->lineno(), "%s", e);
+            comp->error(comp->filename(), comp->lineno(), "%s", msg);
     }
     return f;
 }
@@ -598,12 +615,14 @@ ASTvariable_declaration::ASTvariable_declaration (OSLCompilerImpl *comp,
                                                   const TypeSpec &type,
                                                   ustring name, ASTNode *init,
                                                   bool isparam, bool ismeta,
-                                                  bool isoutput, bool initlist)
+                                                  bool isoutput, bool initlist,
+                                                  bool isthis)
     : ASTnamed_symbol (variable_declaration_node, comp, name, init, NULL /* meta */),
       m_isparam(isparam), m_isoutput(isoutput), m_ismetadata(ismeta),
       m_initlist(initlist)
 {
-    if (! check_reserved ())
+    int vflags = isthis ? allow_this : 0;
+    if (! check_reserved (m_name, m_compiler, vflags))
         return;
 
     if (m_initlist && init) {
@@ -614,7 +633,7 @@ ASTvariable_declaration::ASTvariable_declaration (OSLCompilerImpl *comp,
 
     m_typespec = type;
     if (! m_ismetadata)
-        validate (warn_function_clash);
+        validate (vflags | warn_function_clash);
 
     SymType symtype = isparam ? (isoutput ? SymTypeOutputParam : SymTypeParam)
                               : SymTypeLocal;
@@ -638,6 +657,50 @@ ASTvariable_declaration::ASTvariable_declaration (OSLCompilerImpl *comp,
 
 
 
+ASTvariable_declaration::ASTvariable_declaration (OSLCompilerImpl *comp,
+                                                  const TypeSpec &type,
+                                                  ASTNode *args)
+    : ASTvariable_declaration(comp, type, ustring("this"), NULL,
+                              false, false, false, false, true)
+{
+    // Push 'this' to front of given arguments
+    concat(this, args);
+    if (StructSpec* sspec = type.structure() ? type.structspec() : nullptr) {
+        // Add the fields as accessible without 'this', warning about possible
+        // shadowing/ambiguity as we go.
+        for (int i = 0, n = sspec->numfields(); i < n; ++i) {
+            const auto& field = sspec->field(i);
+            bool conflict = false;
+            for (ref next = args; next; next = next->next()) {
+                ASSERT (next->nodetype() == variable_declaration_node);
+                auto *arg = static_cast<ASTvariable_declaration*>(next.get());
+
+                if (arg->name() == field.name) {
+                    warning ("argument \"%s\" shadows a field with the same name",
+                             field.name);
+                    conflict = true;
+                    break;
+                }
+            }
+            if (! conflict) {
+                ustring qualname = ustring::format ("this.%s", field.name);
+                ASSERT (m_compiler->symtab().find (qualname) != nullptr);
+                ASSERT (!m_compiler->symtab().find (field.name) ||
+                         m_compiler->symtab().find (field.name)->scope() !=
+                         m_compiler->symtab().scopeid());
+
+                Symbol* qsym = m_compiler->symtab().find (qualname);
+                Symbol *fsym = new Symbol (field.name, field.type,
+                                           qsym->symtype (), this);
+                fsym->alias (qsym);
+                m_compiler->symtab().insert (fsym);
+            }
+        }
+    }
+}
+
+
+
 const char *
 ASTvariable_declaration::nodetypename () const
 {
@@ -675,10 +738,10 @@ ASTvariable_declaration::print (std::ostream &out, int indentlevel) const
 
 
 
-ASTvariable_ref::ASTvariable_ref (OSLCompilerImpl *comp, ustring name)
+ASTvariable_ref::ASTvariable_ref (OSLCompilerImpl *comp, ustring name, bool allowthis)
     : ASTnamed_symbol (variable_ref_node, comp, name)
 {
-    m_sym = validate (warn_function_exists);
+    m_sym = validate (warn_function_exists | (allowthis ? allow_this : 0));
     if (m_sym)
         m_typespec = m_sym->typespec();
 }
@@ -1195,10 +1258,11 @@ ASTtype_constructor::childname (size_t i) const
 ASTfunction_call::ASTfunction_call (OSLCompilerImpl *comp, ustring name,
                                     ASTNode *args, FunctionSymbol *funcsym)
     : ASTnamed_symbol (function_call_node, comp, name, args),
-      m_poly(funcsym),    // Default - resolved symbol or null
-      m_argread(~1),      // Default - all args are read except the first
-      m_argwrite(1),      // Default - first arg only is written by the op
-      m_argtakesderivs(0) // Default - doesn't take derivs
+      m_poly(funcsym),     // Default - resolved symbol or null
+      m_argread(~1),       // Default - all args are read except the first
+      m_argwrite(1),       // Default - first arg only is written by the op
+      m_argtakesderivs(0), // Default - doesn't take derivs
+      m_method(false)
 {
     m_sym = funcsym ? funcsym : validate (err_must_exist); // Look it up.
     if (! m_sym || is_struct_ctr())
@@ -1212,10 +1276,24 @@ ASTfunction_call::ASTfunction_call (OSLCompilerImpl *comp, ustring name,
 
 
 
+void
+ASTfunction_call::method_from_function (ASTNode* thisRef)
+{
+    ASSERT (m_method == false);
+    ASSERT (m_children.size() == 1);
+    m_method = true;
+    if (m_children[0])
+        thisRef->append(args().get());
+
+    m_children[0] = thisRef;
+}
+
+
+
 const char *
 ASTfunction_call::childname (size_t i) const
 {
-    return ustring::format ("param%d", (int)i).c_str();
+    return (i || !m_method ? ustring::format ("param%d", (int)i) : ustring("this")).c_str();
 }
 
 
diff --git a/src/liboslcomp/ast.h b/src/liboslcomp/ast.h
index 0c4bfbd5e..49d32b877 100644
--- a/src/liboslcomp/ast.h
+++ b/src/liboslcomp/ast.h
@@ -416,6 +416,7 @@ class ASTnamed_symbol : public ASTNode
         err_must_exist  = 0,  ///< Error when the named Symbol does not exist.
         check_clashes   = 1,  ///< If named Symbol exists error as a clash.
         warn_shadow     = 2,  ///< Warn if name shadows a known function.
+        allow_this      = 4,  ///< Allow m_name to be 'this'
 
         /// Convenient aliases of validation schemes.
         warn_function_exists = err_must_exist | warn_shadow,
@@ -423,19 +424,20 @@ class ASTnamed_symbol : public ASTNode
     };
 
     /// Validate that m_name is a legal name.
-    static bool check_reserved (ustring name, OSLCompilerImpl *comp);
+    static bool check_reserved (ustring name, OSLCompilerImpl *comp,
+                                int allowThis = 0);
 
     /// Validate that m_name is a legal name, and doesn't conflict with rules
     /// given in vflags allowing duplicate symbol if its SymType matches allowed.
     static Symbol* validate (ustring name, OSLCompilerImpl *comp,
-                             Validation vflags, int allowed = -1);
+                             int vflags, int allowed = -1);
 
 protected:
     bool check_reserved () {
         return ASTnamed_symbol::check_reserved (m_name, m_compiler);
     }
 
-    Symbol* validate (Validation vflags, int allowed = -1) {
+    Symbol* validate (int vflags, int allowed = -1) {
         return validate (m_name, m_compiler, vflags, allowed);
     }
 
@@ -504,7 +506,11 @@ class ASTvariable_declaration : public ASTnamed_symbol
     ASTvariable_declaration (OSLCompilerImpl *comp, const TypeSpec &type,
                              ustring name, ASTNode *init, bool isparam=false,
                              bool ismeta=false, bool isoutput=false,
-                             bool initlist=false);
+                             bool initlist=false, bool isthis=false);
+
+    ASTvariable_declaration (OSLCompilerImpl *comp, const TypeSpec &type,
+                             ASTNode *args);
+
     const char *nodetypename () const;
     const char *childname (size_t i) const;
     void print (std::ostream &out, int indentlevel=0) const;
@@ -551,7 +557,7 @@ class ASTvariable_declaration : public ASTnamed_symbol
 class ASTvariable_ref : public ASTnamed_symbol
 {
 public:
-    ASTvariable_ref (OSLCompilerImpl *comp, ustring name);
+    ASTvariable_ref (OSLCompilerImpl *comp, ustring name, bool allowthis);
     const char *nodetypename () const { return "variable_ref"; }
     const char *childname (size_t i) const { return ""; } // no children
     void print (std::ostream &out, int indentlevel=0) const;
@@ -954,6 +960,8 @@ class ASTfunction_call : public ASTnamed_symbol
         return (ASTfunction_declaration *) func()->node();
     }
 
+    void method_from_function(ASTNode* thisRef);
+
 private:
     /// Handle all the special cases for built-ins.  This includes
     /// irregular patterns of which args are read vs written, special
@@ -1027,10 +1035,13 @@ class ASTfunction_call : public ASTnamed_symbol
                                  ustring formal, ustring actual,
                                  Symbol *arrayindex = NULL);
 
+protected:
+
     FunctionSymbol *m_poly;         ///< The specific polymorphic variant
     unsigned int m_argread;         ///< Bit field - which args are read
     unsigned int m_argwrite;        ///< Bit field - which args are written
     unsigned int m_argtakesderivs;  ///< Bit field - which args take derivs
+    bool m_method;
 };
 
 
diff --git a/src/liboslcomp/oslgram.y b/src/liboslcomp/oslgram.y
index a8b61d61d..f15efebef 100644
--- a/src/liboslcomp/oslgram.y
+++ b/src/liboslcomp/oslgram.y
@@ -64,6 +64,7 @@ TypeDesc osllextype (int lex);
 OSL_NAMESPACE_EXIT
 
 static std::stack<TypeSpec> typespec_stack; // just for function_declaration
+bool allowthis;
 
 %}
 
@@ -102,7 +103,7 @@ static std::stack<TypeSpec> typespec_stack; // just for function_declaration
 %type <n> function_declaration
 %type <n> function_body_or_just_decl
 %type <n> struct_declaration
-%type <i> field_declarations field_declaration
+%type <n> field_declarations field_declaration
 %type <n> typed_field_list typed_field
 %type <n> variable_declaration def_expressions def_expression
 %type <n> initializer_opt initializer initializer_list_opt initializer_list 
@@ -324,22 +325,43 @@ function_body_or_just_decl
         ;
 
 function_declaration
-        : typespec IDENTIFIER 
+        : typespec IDENTIFIER
                 {
                     oslcompiler->symtab().push ();  // new scope
                     typespec_stack.push (oslcompiler->current_typespec());
+
+                    if (StructSpec *s = oslcompiler->symtab().current_struct()) {
+                        ustring name ($2);
+                        if (s->lookup_field (name) >= 0) {
+                            oslcompiler->error (oslcompiler->filename(),
+                                                oslcompiler->lineno(),
+                                                "Field \"%s\" already exists in struct \"%s\"",
+                                                name.c_str(), s->name().c_str());
+                        }
+                    }
                 }
-          '(' formal_params_opt ')' metadata_block_opt function_body_or_just_decl
+          '(' formal_params_opt ')' metadata_block_opt
                 {
+                    if (StructSpec *s = oslcompiler->symtab().current_struct()) {
+                        allowthis = true;
+                        TypeSpec t(s->name().c_str(), 0);
+                        auto* args = new ASTvariable_declaration(oslcompiler, t,
+                                                                 $5);
+                        $<n>$ = args;
+                    } else
+                        $<n>$ = $5;
+                }
+          function_body_or_just_decl
+                {
+                    allowthis = false;
                     oslcompiler->symtab().pop ();  // restore scope
                     ASTfunction_declaration *f;
                     f = new ASTfunction_declaration (oslcompiler,
                                                      typespec_stack.top(),
-                                                     ustring($2), $5, $8, NULL);
+                                                     ustring($2), $<n>8, $9, $7);
                     oslcompiler->remember_function_decl (f);
-                    f->add_meta ($7);
-                    $$ = f;
                     typespec_stack.pop ();
+                    $$ = f;
                 }
         ;
 
@@ -360,11 +382,12 @@ struct_declaration
 
 field_declarations
         : field_declaration
-        | field_declarations field_declaration 
+        | field_declarations field_declaration
         ;
 
 field_declaration
-        : typespec typed_field_list ';'
+        : typespec typed_field_list ';' { $$ = 0; }
+        | function_declaration
         ;
 
 typed_field_list
@@ -767,12 +790,27 @@ variable_lvalue
 id_or_field
         : IDENTIFIER 
                 {
-                    $$ = new ASTvariable_ref (oslcompiler, ustring($1));
+                    $$ = new ASTvariable_ref (oslcompiler, ustring($1), allowthis);
                 }
         | variable_lvalue '.' IDENTIFIER
                 {
                     $$ = new ASTstructselect (oslcompiler, $1, ustring($3));
                 }
+        | variable_lvalue '.' function_call
+                {
+                    static_cast<ASTfunction_call*>($3)->method_from_function($1);
+                    $$ = $3;
+                }
+        | function_call '.' function_call
+                {
+                    static_cast<ASTfunction_call*>($3)->method_from_function($1);
+                    $$ = $3;
+                }
+        | type_constructor '.' function_call
+                {
+                    static_cast<ASTfunction_call*>($3)->method_from_function($1);
+                    $$ = $3;
+                }
         ;
 
 variable_ref
diff --git a/src/liboslcomp/typecheck.cpp b/src/liboslcomp/typecheck.cpp
index 21419e4e1..6672ff8a8 100644
--- a/src/liboslcomp/typecheck.cpp
+++ b/src/liboslcomp/typecheck.cpp
@@ -1448,6 +1448,7 @@ class CandidateFunctions {
     size_t m_nargs;
     FunctionSymbol* m_called; // Function called by name (can be NULL!)
     bool m_had_initlist;
+    const bool m_method_call;
 
     const char* scoreWildcard(int& argscore, size_t& fargs, const char* args) const {
         while (fargs < m_nargs) {
@@ -1564,6 +1565,9 @@ class CandidateFunctions {
             int score = scoreType(formaltype, argtype);
             if (score == kNoMatch)
                 return kNoMatch;
+            // Implicit this cannot be coerced
+            if (score != kExactMatch && m_method_call && fargs == 0)
+                return kNoMatch;
 
             argscore += score;
         }
@@ -1607,10 +1611,9 @@ class CandidateFunctions {
 
 public:
     CandidateFunctions(OSLCompilerImpl* compiler, TypeSpec rval,
-                       ASTNode::ref args, FunctionSymbol* func)
+                       ASTNode::ref args, FunctionSymbol* func, bool methcall)
         : m_compiler(compiler), m_rval(rval), m_args(args), m_nargs(0),
-          m_called(func), m_had_initlist(false) {
-
+          m_called(func), m_had_initlist(false), m_method_call(methcall) {
         //std::cerr << "Matching " << func->name() << " formals='" << (rval.simpletype().basetype != TypeDesc::UNKNOWN ?  compiler->code_from_type (rval) : " ");
         for (ASTNode::ref arg = m_args; arg; arg = arg->next()) {
             //std::cerr << compiler->code_from_type (arg->typespec());
@@ -1943,13 +1946,14 @@ ASTfunction_call::typecheck (TypeSpec expected)
     // Save the currently choosen symbol for error reporting later
     FunctionSymbol* poly = func();
 
-    CandidateFunctions candidates(m_compiler, expected, args(), poly);
+    CandidateFunctions candidates(m_compiler, expected, args(), poly, m_method);
     std::tie(m_sym, m_typespec) = candidates.best(this, m_name);
 
     // Check resolution against prior versions of OSL.
-    // Skip the check if any arguments used initializer list syntax.
+    // Skip the check if any arguments used method or initializer list syntax.
     static const char* OSL_LEGACY = ::getenv("OSL_LEGACY_FUNCTION_RESOLUTION");
-    if (!candidates.hadinitlist() && OSL_LEGACY && strcmp(OSL_LEGACY, "0")) {
+    if (!m_method && !candidates.hadinitlist() &&
+        OSL_LEGACY && strcmp(OSL_LEGACY, "0")) {
         auto* legacy = LegacyOverload(m_compiler, this, poly,
                                    &ASTfunction_call::check_arglist)(expected);
         if (m_sym != legacy) {
diff --git a/testsuite/function-method/ref/out.txt b/testsuite/function-method/ref/out.txt
new file mode 100644
index 000000000..4ff3443de
--- /dev/null
+++ b/testsuite/function-method/ref/out.txt
@@ -0,0 +1,9 @@
+test.osl:12: warning: argument "f" shadows a field with the same name
+Compiled test.osl -> test.oso
+emethod::test: (42) (2)
+emethod::test: (42) (46)
+emethod::testchain: 40
+emethod::testchain: 36
+emethod::testchain: 28
+emethod::testchain: 12
+
diff --git a/testsuite/function-method/run.py b/testsuite/function-method/run.py
new file mode 100755
index 000000000..092a20007
--- /dev/null
+++ b/testsuite/function-method/run.py
@@ -0,0 +1,3 @@
+#!/usr/bin/env python
+
+command = testshade("-g 1 1 test")
diff --git a/testsuite/function-method/test.osl b/testsuite/function-method/test.osl
new file mode 100644
index 000000000..35c71e80e
--- /dev/null
+++ b/testsuite/function-method/test.osl
@@ -0,0 +1,26 @@
+struct emethod {
+    float f;
+
+    color test(int i) {
+        printf("emethod::test: (%g) (%d)\n", this.f, i);
+        return color(4,5,6);
+    }
+    float test2(int i) {
+        color c = this.test((int)(f + i*2));
+        return c[0];
+    }
+
+    emethod testchain(float f) {
+        this.f -= f;
+        printf("emethod::testchain: %g\n", this.f);
+        return this;
+    }
+};
+
+shader test ()
+{
+    emethod e = { 42 };
+    e.test(2);
+    e.test2(2);
+    emethod(2+4+8+16).testchain(2).testchain(4).testchain(8).testchain(16);
+}
diff --git a/testsuite/oslc-err-method/ref/out.txt b/testsuite/oslc-err-method/ref/out.txt
new file mode 100644
index 000000000..31f2daa6d
--- /dev/null
+++ b/testsuite/oslc-err-method/ref/out.txt
@@ -0,0 +1,4 @@
+test.osl:14: error: No matching function call to 'testmethod (vector)'
+  Candidates are:
+    test.osl:3	int testmethod (color)
+FAILED test.osl
diff --git a/testsuite/oslc-err-method/run.py b/testsuite/oslc-err-method/run.py
new file mode 100755
index 000000000..d901e097a
--- /dev/null
+++ b/testsuite/oslc-err-method/run.py
@@ -0,0 +1,5 @@
+#!/usr/bin/env python
+
+# command = oslc("test.osl")
+# don't even need that -- it's automatic
+failureok = 1     # this test is expected to have oslc errors
diff --git a/testsuite/oslc-err-method/test.osl b/testsuite/oslc-err-method/test.osl
new file mode 100644
index 000000000..73f814bc8
--- /dev/null
+++ b/testsuite/oslc-err-method/test.osl
@@ -0,0 +1,15 @@
+// Test that method invocation is not allowed with a coerced type
+
+int testmethod(color c) {
+    return 5;
+}
+
+shader
+test ()
+{
+    color c;
+    c.testmethod();
+
+    vector v0;
+    v0.testmethod();
+}

From ca277784810fd4f482f8e07e7010ed51705f2df0 Mon Sep 17 00:00:00 2001
From: Frederich Munch <marsupial@users.noreply.github.com>
Date: Thu, 28 Dec 2017 23:50:39 -0500
Subject: [PATCH 5/6] Push implicit this argument when calling methods from
 methods.

---
 src/liboslcomp/ast.cpp                | 20 ++++++++++++++++++--
 src/liboslcomp/ast.h                  |  3 ++-
 src/liboslcomp/oslgram.y              | 16 ++++++++--------
 testsuite/function-method/ref/out.txt | 15 +++++++++++----
 testsuite/function-method/test.osl    | 21 +++++++++++++++++++++
 5 files changed, 60 insertions(+), 15 deletions(-)

diff --git a/src/liboslcomp/ast.cpp b/src/liboslcomp/ast.cpp
index 8f049dd34..361a7d0c9 100644
--- a/src/liboslcomp/ast.cpp
+++ b/src/liboslcomp/ast.cpp
@@ -1256,7 +1256,8 @@ ASTtype_constructor::childname (size_t i) const
 
 
 ASTfunction_call::ASTfunction_call (OSLCompilerImpl *comp, ustring name,
-                                    ASTNode *args, FunctionSymbol *funcsym)
+                                    ASTNode *args, FunctionSymbol *funcsym,
+                                    ASTNode* implicit_this)
     : ASTnamed_symbol (function_call_node, comp, name, args),
       m_poly(funcsym),     // Default - resolved symbol or null
       m_argread(~1),       // Default - all args are read except the first
@@ -1272,6 +1273,16 @@ ASTfunction_call::ASTfunction_call (OSLCompilerImpl *comp, ustring name,
         m_sym = nullptr;
         return;
     }
+    if (implicit_this && m_sym->node()) {
+        int advance;
+        const char* form = static_cast<FunctionSymbol*>(m_sym)->argcodes().c_str();
+        /*TypeSpec rval = */ comp->type_from_code(form, &advance);
+        form += advance;
+        if (*form && comp->type_from_code(form) == implicit_this->typespec()) {
+            method_from_function(new ASTvariable_ref(m_compiler,
+                                                     ustring("this"), true));
+        }
+    }
 }
 
 
@@ -1279,7 +1290,12 @@ ASTfunction_call::ASTfunction_call (OSLCompilerImpl *comp, ustring name,
 void
 ASTfunction_call::method_from_function (ASTNode* thisRef)
 {
-    ASSERT (m_method == false);
+    // Only do this once!
+    if (m_method) {
+        delete thisRef;
+        return;
+    }
+
     ASSERT (m_children.size() == 1);
     m_method = true;
     if (m_children[0])
diff --git a/src/liboslcomp/ast.h b/src/liboslcomp/ast.h
index 49d32b877..a36577894 100644
--- a/src/liboslcomp/ast.h
+++ b/src/liboslcomp/ast.h
@@ -934,7 +934,8 @@ class ASTfunction_call : public ASTnamed_symbol
 {
 public:
     ASTfunction_call (OSLCompilerImpl *comp, ustring name, ASTNode *args,
-                      FunctionSymbol *funcsym = nullptr);
+                      FunctionSymbol *funcsym = nullptr,
+                      ASTNode* implicit_this = nullptr);
     const char *nodetypename () const { return "function_call"; }
     const char *childname (size_t i) const;
     const char *opname () const;
diff --git a/src/liboslcomp/oslgram.y b/src/liboslcomp/oslgram.y
index f15efebef..d3db576bd 100644
--- a/src/liboslcomp/oslgram.y
+++ b/src/liboslcomp/oslgram.y
@@ -64,7 +64,7 @@ TypeDesc osllextype (int lex);
 OSL_NAMESPACE_EXIT
 
 static std::stack<TypeSpec> typespec_stack; // just for function_declaration
-bool allowthis;
+static ASTNode::ref implicit_this;
 
 %}
 
@@ -343,22 +343,21 @@ function_declaration
           '(' formal_params_opt ')' metadata_block_opt
                 {
                     if (StructSpec *s = oslcompiler->symtab().current_struct()) {
-                        allowthis = true;
                         TypeSpec t(s->name().c_str(), 0);
-                        auto* args = new ASTvariable_declaration(oslcompiler, t,
-                                                                 $5);
-                        $<n>$ = args;
+                        implicit_this = new ASTvariable_declaration(oslcompiler, t,
+                                                                    $5);
+                        $<n>$ = implicit_this.get();
                     } else
                         $<n>$ = $5;
                 }
           function_body_or_just_decl
                 {
-                    allowthis = false;
                     oslcompiler->symtab().pop ();  // restore scope
                     ASTfunction_declaration *f;
                     f = new ASTfunction_declaration (oslcompiler,
                                                      typespec_stack.top(),
                                                      ustring($2), $<n>8, $9, $7);
+                    implicit_this = nullptr;
                     oslcompiler->remember_function_decl (f);
                     typespec_stack.pop ();
                     $$ = f;
@@ -790,7 +789,7 @@ variable_lvalue
 id_or_field
         : IDENTIFIER 
                 {
-                    $$ = new ASTvariable_ref (oslcompiler, ustring($1), allowthis);
+                    $$ = new ASTvariable_ref (oslcompiler, ustring($1), implicit_this);
                 }
         | variable_lvalue '.' IDENTIFIER
                 {
@@ -945,7 +944,8 @@ type_constructor
 function_call
         : IDENTIFIER '(' function_args_opt ')'
                 {
-                    $$ = new ASTfunction_call (oslcompiler, ustring($1), $3);
+                    $$ = new ASTfunction_call (oslcompiler, ustring($1), $3,
+                                               nullptr, implicit_this.get());
                 }
         ;
 
diff --git a/testsuite/function-method/ref/out.txt b/testsuite/function-method/ref/out.txt
index 4ff3443de..d72ab56a5 100644
--- a/testsuite/function-method/ref/out.txt
+++ b/testsuite/function-method/ref/out.txt
@@ -1,9 +1,16 @@
-test.osl:12: warning: argument "f" shadows a field with the same name
+test.osl:16: warning: argument "f" shadows a field with the same name
+test.osl:22: warning: argument "f" shadows a field with the same name
+test.osl:27: warning: "f" shadows a field with the same name
 Compiled test.osl -> test.oso
 emethod::test: (42) (2)
 emethod::test: (42) (46)
-emethod::testchain: 40
-emethod::testchain: 36
 emethod::testchain: 28
-emethod::testchain: 12
+emethod::testchain: 24
+emethod::testchain: 16
+emethod::testchain: 0
+emethod::shadowed: (40) (3.14)
+emethod::shadowed2: 40 22.4 6.28
+emethod::shadowed: (40) (6.28)
+unshadowed 5
+unshadowed *
 
diff --git a/testsuite/function-method/test.osl b/testsuite/function-method/test.osl
index 35c71e80e..b31fdfa0d 100644
--- a/testsuite/function-method/test.osl
+++ b/testsuite/function-method/test.osl
@@ -1,3 +1,6 @@
+void unshadowed(int i) { printf("unshadowed %d\n", i); }
+void unshadowed() { printf("unshadowed *\n"); }
+
 struct emethod {
     float f;
 
@@ -15,12 +18,30 @@ struct emethod {
         printf("emethod::testchain: %g\n", this.f);
         return this;
     }
+
+    void shadowed(float f) {
+        printf("emethod::shadowed: (%g) (%g)\n", this.f, f);
+    }
+
+    float shadowed2(float ff) {
+        float f = 32.4;
+        f -= 10;
+        printf("emethod::shadowed2: %g %g %g\n", this.f, f, ff);
+        shadowed(ff);
+        unshadowed(5);
+        unshadowed();
+        return f;
+    }
 };
 
+
 shader test ()
 {
     emethod e = { 42 };
     e.test(2);
     e.test2(2);
     emethod(2+4+8+16).testchain(2).testchain(4).testchain(8).testchain(16);
+    e.f -= 2;
+    e.shadowed(3.14);
+    e.shadowed2(6.28);
 }

From 0531579ed397949c468a34658a9246985daecb8b Mon Sep 17 00:00:00 2001
From: Frederich Munch <marsupial@users.noreply.github.com>
Date: Fri, 29 Dec 2017 01:44:03 -0500
Subject: [PATCH 6/6] Allow external method definitions in the style of C++.

---
 src/liboslcomp/oslgram.y              | 62 +++++++++++++++++++---
 testsuite/function-method/ref/out.txt | 16 ++++--
 testsuite/function-method/test.osl    | 75 ++++++++++++++++++++++-----
 3 files changed, 129 insertions(+), 24 deletions(-)

diff --git a/src/liboslcomp/oslgram.y b/src/liboslcomp/oslgram.y
index d3db576bd..cf86c13ef 100644
--- a/src/liboslcomp/oslgram.y
+++ b/src/liboslcomp/oslgram.y
@@ -97,7 +97,7 @@ static ASTNode::ref implicit_this;
 // Define the nonterminals 
 %type <n> shader_file 
 %type <n> global_declarations_opt global_declarations global_declaration
-%type <n> shader_or_function_declaration
+%type <n> shader_or_function_declaration method_declaration
 %type <n> formal_params_opt formal_params formal_param
 %type <n> metadata_block_opt metadata metadatum
 %type <n> function_declaration
@@ -166,6 +166,7 @@ global_declarations
 global_declaration
         : shader_or_function_declaration    { $$ = 0; }
         | struct_declaration                { $$ = 0; }
+        | method_declaration                { $$ = 0; }
         ;
 
 shader_or_function_declaration
@@ -173,14 +174,13 @@ shader_or_function_declaration
                 {
                     if ($1 == ShadTypeUnknown) {
                         // It's a function declaration, not a shader
+                        ASSERT (! typespec_stack.empty ());
                         oslcompiler->symtab().push ();  // new scope
-                        typespec_stack.push (oslcompiler->current_typespec());
                     }
                 }
           metadata_block_opt '(' 
                 {
-                    if ($1 != ShadTypeUnknown)
-                        oslcompiler->declaring_shader_formals (true);
+                    oslcompiler->declaring_shader_formals ($1 != ShadTypeUnknown);
                 }
           formal_params_opt ')'
                 {
@@ -220,6 +220,49 @@ shader_or_function_declaration
                 }
         ;
 
+method_declaration
+        : typespec_or_shadertype typespec ':' ':' IDENTIFIER
+                {
+                    // It's a method declaration, not a shader
+                    if ($1 != ShadTypeUnknown || typespec_stack.empty() ||
+                        (! oslcompiler->current_typespec().is_structure() &&
+                         ! oslcompiler->current_typespec().is_triple())) {
+                        oslcompiler->error (oslcompiler->filename(),
+                                            oslcompiler->lineno(),
+                                            "Cannot declare a method for this type");
+                    }
+
+                    oslcompiler->symtab().push ();  // new scope
+                    typespec_stack.push (oslcompiler->current_typespec());
+                }
+                '(' formal_params_opt ')' metadata_block_opt
+                {
+                    implicit_this = new ASTvariable_declaration(oslcompiler,
+                                                         typespec_stack.top(),
+                                                         $8);
+                    typespec_stack.pop ();
+                    $<n>$ = implicit_this.get();
+                }
+                function_body_or_just_decl
+                {
+                    // Method declaration
+                    ASSERT ($1 == ShadTypeUnknown);
+                    oslcompiler->symtab().pop ();  // restore scope
+                    ASTfunction_declaration *f;
+                    f = new ASTfunction_declaration (oslcompiler,
+                                                     typespec_stack.top(),
+                                                     ustring($5),
+                                                     $<n>11 /*arguments*/,
+                                                     $12 /*statements*/,
+                                                     $10 /*meta*/);
+                    implicit_this = nullptr;
+                    typespec_stack.pop ();
+                    oslcompiler->remember_function_decl (f);
+                    f->sourceline (@2.first_line);
+                    $$ = f;
+                }
+        ;
+
 formal_params_opt
         : formal_params
         | /* empty */                   { $$ = 0; }
@@ -586,12 +629,14 @@ typespec_or_shadertype
         : simple_typename
                 {
                     oslcompiler->current_typespec (TypeSpec (osllextype ($1)));
-                    $$ = 0;
+                    typespec_stack.push (oslcompiler->current_typespec ());
+                    $$ = ShadTypeUnknown;
                 }
         | CLOSURE simple_typename
                 {
                     oslcompiler->current_typespec (TypeSpec (osllextype ($2), true));
-                    $$ = 0;
+                    typespec_stack.push (oslcompiler->current_typespec ());
+                    $$ = ShadTypeUnknown;
                 }
         | IDENTIFIER /* struct name or shader type name */
                 {
@@ -606,9 +651,10 @@ typespec_or_shadertype
                         $$ = ShadTypeVolume;
                     else {
                         Symbol *s = oslcompiler->symtab().find (name);
-                        if (s && s->is_structure())
+                        if (s && s->is_structure()) {
                             oslcompiler->current_typespec (TypeSpec ("", s->typespec().structure()));
-                        else {
+                            typespec_stack.push (oslcompiler->current_typespec ());
+                        } else {
                             oslcompiler->current_typespec (TypeSpec (TypeDesc::UNKNOWN));
                             oslcompiler->error (oslcompiler->filename(),
                                                 oslcompiler->lineno(),
diff --git a/testsuite/function-method/ref/out.txt b/testsuite/function-method/ref/out.txt
index d72ab56a5..8623a16a2 100644
--- a/testsuite/function-method/ref/out.txt
+++ b/testsuite/function-method/ref/out.txt
@@ -1,6 +1,6 @@
-test.osl:16: warning: argument "f" shadows a field with the same name
-test.osl:22: warning: argument "f" shadows a field with the same name
-test.osl:27: warning: "f" shadows a field with the same name
+test.osl:13: warning: argument "f" shadows a field with the same name
+test.osl:19: warning: argument "f" shadows a field with the same name
+test.osl:36: warning: "f" shadows a field with the same name
 Compiled test.osl -> test.oso
 emethod::test: (42) (2)
 emethod::test: (42) (46)
@@ -8,9 +8,19 @@ emethod::testchain: 28
 emethod::testchain: 24
 emethod::testchain: 16
 emethod::testchain: 0
+emethod::predecl: (40) (40) (2)
 emethod::shadowed: (40) (3.14)
 emethod::shadowed2: 40 22.4 6.28
 emethod::shadowed: (40) (6.28)
 unshadowed 5
 unshadowed *
+custom_method0(c{1 1 1})
+custom_method0(v{2 2 2})
+custom_method1(c{1 1 1}, c{3 3 3})
+c.custom_method1 = 4 4 4
+custom_method1(v{2 2 2}, f2)
+vo = 4 4 4
+custom_method3: 101 2 3
+c1.custom_method3: 34 (101 2 3)
+custom_method3: 101 1 1
 
diff --git a/testsuite/function-method/test.osl b/testsuite/function-method/test.osl
index b31fdfa0d..83df5cb60 100644
--- a/testsuite/function-method/test.osl
+++ b/testsuite/function-method/test.osl
@@ -1,6 +1,3 @@
-void unshadowed(int i) { printf("unshadowed %d\n", i); }
-void unshadowed() { printf("unshadowed *\n"); }
-
 struct emethod {
     float f;
 
@@ -23,17 +20,53 @@ struct emethod {
         printf("emethod::shadowed: (%g) (%g)\n", this.f, f);
     }
 
-    float shadowed2(float ff) {
-        float f = 32.4;
-        f -= 10;
-        printf("emethod::shadowed2: %g %g %g\n", this.f, f, ff);
-        shadowed(ff);
-        unshadowed(5);
-        unshadowed();
-        return f;
-    }
+    vector predecl(float ff);
 };
 
+vector emethod::predecl(float ff) {
+    f -= ff;
+	printf("emethod::predecl: (%g) (%g) (%g)\n", this.f, f, ff);
+    return vector(6,7,8);
+}
+
+void unshadowed(int i) { printf("unshadowed %d\n", i); }
+void unshadowed() { printf("unshadowed *\n"); }
+
+float emethod::shadowed2(float ff) {
+    float f = 32.4;
+    f -= 10;
+    printf("emethod::shadowed2: %g %g %g\n", this.f, f, ff);
+    shadowed(ff);
+    unshadowed(5);
+    unshadowed();
+    return f;
+}
+
+float color::custom_method3(color c, color b) [[ string description = "MB" ]]  {
+    this[0] += 100;
+    printf("custom_method3: %g\n", this);
+    return this[1] + 32.0;
+}
+
+color custom_method0(color c) {
+    printf("custom_method0(c{%g})\n", c);
+    return c;
+}
+
+vector custom_method0(vector v) {
+    printf("custom_method0(v{%g})\n", v);
+    return v;
+}
+
+color custom_method1(color c, color b) {
+    printf("custom_method1(c{%g}, c{%g})\n", c, b);
+    return c + b;
+}
+
+vector custom_method1(vector v, float b) {
+    printf("custom_method1(v{%g}, f%g)\n", v, b);
+    return v + vector(b);
+}
 
 shader test ()
 {
@@ -41,7 +74,23 @@ shader test ()
     e.test(2);
     e.test2(2);
     emethod(2+4+8+16).testchain(2).testchain(4).testchain(8).testchain(16);
-    e.f -= 2;
+    e.predecl(2.0);
     e.shadowed(3.14);
     e.shadowed2(6.28);
+
+    color  c = 1;
+    vector vo = 2;
+
+    c.custom_method0();
+    vo.custom_method0();
+
+    printf("c.custom_method1 = %g\n", c.custom_method1(color(3)));
+
+    vo = vo.custom_method1(2.0);
+    printf("vo = %g\n", vo);
+
+    color c1 = color(1,2,3);
+    printf("c1.custom_method3: %g (%g)\n", c1.custom_method3(c,c), c1);
+
+    custom_method3(c,c,c);
 }