Refactor ASTNodes who have both a Symbol and name into common base class ASTnamed_symbol.

Author: marsupial

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

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,20 +449,17 @@ 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
     // start of the declaration, substitute that.
     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;
     }
 }

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