Replace symbol proxies with type symbols.

Proxies were rather hacky and complicated, and only existed as a
workaround for oddities with constructors. This patch replaces them with
actual type symbols, a very tiny step to semantically getting rid of tags.

This greatly simplifies how we implement constructors, and paves the way
for using methodmap symbols in field expressions.

Since non-value symbols are new to spcomp1, we place a rather
pigeonholed check into primary() to make sure non-value symbols don't
escape into expressions.

sourcepawn/compiler/sc.h

@@ -103,6 +103,8 @@ typedef struct s_constvalue {
                          * tag for enumeration lists */
 } constvalue;
 
+struct methodmap_t;
+
 /*  Symbol table format
  *
  *  The symbol name read from the input file is stored in "name", the
@@ -116,7 +118,6 @@ typedef struct s_constvalue {
 typedef struct s_symbol {
   struct s_symbol *next;
   struct s_symbol *parent;  /* hierarchical types */
-  struct s_symbol *target;  /* proxy target */
   char name[sNAMEMAX+1];
   uint32_t hash;        /* value derived from name, for quicker searching */
   cell addr;            /* address or offset (or value for constant, index for native function) */
@@ -151,6 +152,7 @@ typedef struct s_symbol {
   struct s_symbol **refer;  /* referrer list, functions that "use" this symbol */
   int numrefers;        /* number of entries in the referrer list */
   char *documentation;  /* optional documentation string */
+  methodmap_t *methodmap; /* if ident == iMETHODMAP */
 } symbol;
 
 /*  Possible entries for "ident". These are used in the "symbol", "value"
@@ -170,8 +172,8 @@ typedef struct s_symbol {
 #define iFUNCTN     9
 #define iREFFUNC    10
 #define iVARARGS    11  /* function specified ... as argument(s) */
-#define iPROXY      12  /* proxies to another symbol. */
 #define iACCESSOR   13  /* property accessor via a methodmap_method_t */
+#define iMETHODMAP  14  /* symbol defining a methodmap */
 
 /*  Possible entries for "usage"
  *
@@ -227,7 +229,6 @@ typedef struct s_symbol {
 #define uRETNONE  0x10
 
 #define flgDEPRECATED 0x01  /* symbol is deprecated (avoid use) */
-#define flgPROXIED    0x02  /* symbol has incoming proxy */
 
 #define uCOUNTOF  0x20  /* set in the "hasdefault" field of the arginfo struct */
 #define uTAGOF    0x40  /* set in the "hasdefault" field of the arginfo struct */
@@ -246,7 +247,6 @@ struct methodmap_method_s;
 
 typedef struct value_s {
   symbol *sym;          /* symbol in symbol table, NULL for (constant) expression */
-  symbol *proxy;        /* original symbol if resolved via a proxy */
   cell constval;        /* value of the constant expression (if ident==iCONSTEXPR)
                          * also used for the size of a literal array */
   int tag;              /* tag (of the expression) */
@@ -678,7 +678,7 @@ void delete_symbol(symbol *root,symbol *sym);
 void delete_symbols(symbol *root,int level,int del_labels,int delete_functions);
 int refer_symbol(symbol *entry,symbol *bywhom);
 void markusage(symbol *sym,int usage);
-symbol *findglb(const char *name,int filter,symbol **alias = NULL);
+symbol *findglb(const char *name,int filter);
 symbol *findloc(const char *name);
 symbol *findconst(const char *name,int *matchtag);
 symbol *finddepend(const symbol *parent);

sourcepawn/compiler/sc1.cpp

@@ -3570,32 +3570,6 @@ static void check_void_decl(const declinfo_t *decl, int variable)
   }
 }
 
-static void define_constructor(methodmap_t *map, methodmap_method_t *method)
-{
-  symbol *sym = findglb(map->name, sGLOBAL);
-
-  if (sym) {
-    const char *type = "__unknown__";
-    switch (sym->ident) {
-      case iVARIABLE:
-      case iARRAY:
-      case iARRAYCELL:
-      case iARRAYCHAR:
-        type = "variable";
-        break;
-      case iFUNCTN:
-        type = "function";
-        break;
-    }
-    error(113, map->name, type);
-    return;
-  }
-
-  sym = addsym(map->name, 0, iPROXY, sGLOBAL, 0, 0);
-  sym->target = method->target;
-  method->target->flags |= flgPROXIED;
-}
-
 // Current lexer position is, we've parsed "public", an optional "native", and
 // a type expression.
 //
@@ -4027,7 +4001,8 @@ methodmap_method_t *parse_method(methodmap_t *map)
 
   // If the symbol is a constructor, we bypass the initial argument checks.
   if (is_ctor) {
-    define_constructor(map, method);
+    if (map->ctor)
+      error(113, map->name);
   } else if (!check_this_tag(map, target)) {
     error(108, spectype, map->name);
   }
@@ -4049,7 +4024,6 @@ static void domethodmap(LayoutSpec spec)
 {
   int val;
   char *str;
-  LayoutSpec old_spec;
   methodmap_t *parent = NULL;
   const char *spectype = layout_spec_name(spec);
 
@@ -4062,8 +4036,9 @@ static void domethodmap(LayoutSpec spec)
   if (!isupper(*mapname))
     error(109, spectype);
 
-  old_spec = deduce_layout_spec_by_name(mapname);
+  LayoutSpec old_spec = deduce_layout_spec_by_name(mapname);
-  if (!can_redef_layout_spec(spec, old_spec))
+  int can_redef = can_redef_layout_spec(spec, old_spec);
+  if (!can_redef)
     error(110, mapname, layout_spec_name(old_spec));
 
   if (matchtoken('<')) {
@@ -4093,6 +4068,46 @@ static void domethodmap(LayoutSpec spec)
   }
   methodmap_add(map);
 
+  if (can_redef) {
+    symbol *sym = findglb(mapname, sGLOBAL);
+    if (sym && sym->ident != iMETHODMAP) {
+      // We should only hit this on the first pass. Assert really hard that
+      // we're about to kill an enum definition and not something random.
+      assert(sc_status == statFIRST);
+      assert(sym->ident == iCONSTEXPR);
+      assert(TAGID(map->tag) == TAGID(sym->tag));
+
+      sym->ident = iMETHODMAP;
+
+      // Kill previous enumstruct properties, if any.
+      if (sym->usage & uENUMROOT) {
+        for (constvalue *cv = sym->dim.enumlist; cv; cv = cv->next) {
+          symbol *csym = findglb(cv->name, sGLOBAL);
+          if (csym &&
+              csym->ident == iCONSTEXPR &&
+              csym->parent == sym &&
+              (csym->usage & uENUMFIELD))
+          {
+            csym->usage &= ~uENUMFIELD;
+            csym->parent = NULL;
+          }
+        }
+        delete_consttable(sym->dim.enumlist);
+        free(sym->dim.enumlist);
+        sym->dim.enumlist = NULL;
+      }
+    } else if (!sym) {
+      sym = addsym(
+        mapname,      // name
+        0,            // addr
+        iMETHODMAP,   // ident
+        sGLOBAL,      // vclass
+        map->tag,     // tag
+        uDEFINE);     // usage
+    }
+    sym->methodmap = map;
+  }
+
   needtoken('{');
   while (!matchtoken('}')) {
     token_t tok;
@@ -4587,8 +4602,8 @@ static void decl_enum(int vclass)
   int tag,explicittag;
   cell increment,multiplier;
   constvalue *enumroot;
-  symbol *enumsym;
   LayoutSpec spec;
+  symbol *enumsym = nullptr;
 
   /* get an explicit tag, if any (we need to remember whether an explicit
    * tag was passed, even if that explicit tag was "_:", so we cannot call
@@ -4655,19 +4670,35 @@ static void decl_enum(int vclass)
   } /* if */
 
   if (strlen(enumname)>0) {
-    /* already create the root symbol, so the fields can have it as their "parent" */
+    if (vclass == sGLOBAL) {
-    enumsym=add_constant(enumname,0,vclass,tag);
+      if ((enumsym = findglb(enumname, vclass)) != NULL) {
-    if (enumsym!=NULL)
+        // If we were previously defined as a methodmap, don't overwrite the
-      enumsym->usage |= uENUMROOT;
+        // symbol. Otherwise, flow into add_constant where we will error.
-    /* start a new list for the element names */
+        if (enumsym->ident != iMETHODMAP)
-    if ((enumroot=(constvalue*)malloc(sizeof(constvalue)))==NULL)
+          enumsym = nullptr;
-      error(FATAL_ERROR_OOM);                       /* insufficient memory (fatal error) */
+      }
-    memset(enumroot,0,sizeof(constvalue));
+    }
+
+    if (!enumsym) {
+      /* create the root symbol, so the fields can have it as their "parent" */
+      enumsym=add_constant(enumname,0,vclass,tag);
+      if (enumsym!=NULL)
+        enumsym->usage |= uENUMROOT;
+      /* start a new list for the element names */
+      if ((enumroot=(constvalue*)malloc(sizeof(constvalue)))==NULL)
+        error(FATAL_ERROR_OOM);                       /* insufficient memory (fatal error) */
+      memset(enumroot,0,sizeof(constvalue));
+    }
   } else {
     enumsym=NULL;
     enumroot=NULL;
   } /* if */
 
+  // If this enum is for a methodmap, forget the symbol so code below doesn't
+  // build an enum struct.
+  if (enumsym && enumsym->ident == iMETHODMAP)
+    enumsym = NULL;
+
   needtoken('{');
   /* go through all constants */
   value=0;                              /* default starting value */
@@ -4721,7 +4752,7 @@ static void decl_enum(int vclass)
   matchtoken(';');      /* eat an optional ; */
 
   /* set the enum name to the "next" value (typically the last value plus one) */
-  if (enumsym!=NULL) {
+  if (enumsym) {
     assert((enumsym->usage & uENUMROOT)!=0);
     enumsym->addr=value;
     /* assign the constant list */
@@ -6510,8 +6541,8 @@ static int testsymbols(symbol *root,int level,int testlabs,int testconst)
         error(203,sym->name);       /* symbol isn't used: ... */
       } /* if */
       break;
-    case iPROXY:
+    case iMETHODMAP:
-      // Ignore usage on proxies.
+      // Ignore usage on methodmaps.
       break;
     default:
       /* a variable */

sourcepawn/compiler/sc2.cpp

@@ -2924,25 +2924,6 @@ void delete_symbols(symbol *root,int level,int delete_labels,int delete_function
   constvalue *stateptr;
   int mustdelete;
 
-  // Hack - proxies have a "target" pointer, but the target could be deleted
-  // already if done inside the main loop below. To get around this we do a
-  // precursor pass. Note that proxies can only be at the global scope.
-  if (origRoot == &glbtab) {
-    symbol *iter = root;
-    while (iter->next) {
-      sym = iter->next;
-
-      if (sym->ident != iPROXY) {
-        iter = sym;
-	continue;
-      }
-
-      RemoveFromHashTable(sp_Globals, sym);
-      iter->next = sym->next;
-      free_symbol(sym);
-    }
-  }
-
   /* erase only the symbols with a deeper nesting level than the
    * specified nesting level */
   while (root->next!=NULL) {
@@ -2985,9 +2966,12 @@ void delete_symbols(symbol *root,int level,int delete_labels,int delete_function
       mustdelete=delete_functions || (sym->usage & uNATIVE)!=0;
       assert(sym->parent==NULL);
       break;
-    case iPROXY:
+    case iMETHODMAP:
-      // Original loop determined it was okay to keep.
+      // We delete methodmap symbols at the end, but since methodmaps
-      mustdelete=FALSE;
+      // themselves get wiped, we null the pointer.
+      sym->methodmap = nullptr;
+      mustdelete = delete_functions;
+      assert(!sym->parent);
       break;
     case iARRAYCELL:
     case iARRAYCHAR:
@@ -3143,7 +3127,7 @@ void markusage(symbol *sym,int usage)
  *
  *  Returns a pointer to the global symbol (if found) or NULL (if not found)
  */
-symbol *findglb(const char *name, int filter, symbol **alias)
+symbol *findglb(const char *name, int filter)
 {
   /* find a symbol with a matching automaton first */
   symbol *sym=NULL;
@@ -3168,12 +3152,6 @@ symbol *findglb(const char *name, int filter, symbol **alias)
   if (sym==NULL)
     sym=FindInHashTable(sp_Globals,name,fcurrent);
 
-  if (sym && sym->ident == iPROXY) {
-    if (alias)
-      *alias = sym;
-    return sym->target;
-  }
-
   return sym;
 }
 

sourcepawn/compiler/sc3.cpp

@@ -2025,14 +2025,27 @@ static int hier1(value *lval1)
   lvalue=primary(lval1);
   symtok=tokeninfo(&val,&st);   /* get token read by primary() */
   cursym=lval1->sym;
+
 restart:
   sym=cursym;
+
+  if (lval1->ident == iMETHODMAP &&
+      !(lexpeek('.') || lexpeek('(')))
+  {
+    // Cannot use methodmap as an rvalue/lvalue.
+    error(174, sym ? sym->name : "(unknown)");
+
+    lval1->ident = iCONSTEXPR;
+    lval1->tag = 0;
+    lval1->constval = 0;
+  }
+
   if (matchtoken('[') || matchtoken('{') || matchtoken('(') || matchtoken('.')) {
+    tok=tokeninfo(&val,&st);    /* get token read by matchtoken() */
     if (lvalue && lval1->ident == iACCESSOR) {
       rvalue(lval1);
       lvalue = FALSE;
     }
-    tok=tokeninfo(&val,&st);    /* get token read by matchtoken() */
     magic_string = (sym && (sym->tag == pc_tag_string && sym->dim.array.level == 0));
     if (sym==NULL && symtok!=tSYMBOL) {
       /* we do not have a valid symbol and we appear not to have read a valid
@@ -2275,7 +2288,18 @@ restart:
 
       assert(tok=='(');
       if (sym==NULL || (sym->ident!=iFUNCTN && sym->ident!=iREFFUNC)) {
-        if (sym==NULL && sc_status==statFIRST) {
+        if (sym && sym->ident == iMETHODMAP && sym->methodmap) {
+          if (!sym->methodmap->ctor) {
+            // Immediately fatal - no function to call.
+            return error(172, sym->name);
+          }
+          if (sym->methodmap->nullable) {
+            // Keep going, this is basically a style thing.
+            error(170, sym->methodmap->name);
+          }
+
+          sym = sym->methodmap->ctor->target;
+        } else if (sym==NULL && sc_status==statFIRST) {
           /* could be a "use before declaration"; in that case, create a stub
            * function so that the usage can be marked.
            */
@@ -2285,27 +2309,13 @@ restart:
           markusage(sym,uREAD);
         } else {
           return error(12);           /* invalid function call */
-        } /* if */
+        }
       } else if ((sym->usage & uMISSING)!=0) {
         char symname[2*sNAMEMAX+16];  /* allow space for user defined operators */
         funcdisplayname(symname,sym->name);
         error(4,symname);             /* function not defined */
       } /* if */
 
-      // Check whether we're calling a constructor. This is a bit hacky, since
-      // we're relying on whatever the lval state is.
-      if ((sym->flags & flgPROXIED) &&
-          lval1->proxy &&
-          lval1->proxy->target == sym)
-      {
-        // Only constructors should be proxied, but we check anyway.
-        assert(!implicitthis);
-        if (methodmap_t *methodmap = methodmap_find_by_tag(sym->tag)) {
-          if (sym == methodmap->ctor->target && methodmap->nullable)
-            error(170, methodmap->name);
-        }
-      }
-
       callfunction(sym,implicitthis,lval1,TRUE);
       if (lexpeek('.')) {
         lvalue = FALSE;
@@ -2342,7 +2352,6 @@ restart:
 
     funcenum_t *fe = funcenum_for_symbol(target);
     lval1->sym = NULL;
-    lval1->proxy = NULL;
     lval1->ident = iCONSTEXPR;
     lval1->constval = (public_index << 1) | 1;
     lval1->tag = fe->tag;
@@ -2429,8 +2438,7 @@ static int primary(value *lval)
       } /* if */
     } /* if */
     /* now try a global variable */
-    symbol *alias = NULL;
+    if ((sym = findglb(st, sSTATEVAR)) != 0) {
-    if ((sym = findglb(st, sSTATEVAR, &alias)) != 0) {
       if (sym->ident==iFUNCTN || sym->ident==iREFFUNC) {
         /* if the function is only in the table because it was inserted as a
          * stub in the first pass (i.e. it was "used" but never declared or
@@ -2442,15 +2450,18 @@ static int primary(value *lval)
         if ((sym->usage & uDEFINE)==0)
           error(17,st);
         lval->sym=sym;
-        lval->proxy=alias;
         lval->ident=sym->ident;
         lval->tag=sym->tag;
-        if (sym->ident==iARRAY || sym->ident==iREFARRAY) {
+        switch (sym->ident) {
-          address(sym,sPRI);    /* get starting address in primary register */
+          case iARRAY:
-          return FALSE;         /* return 0 for array (not lvalue) */
+          case iREFARRAY:
-        } else {
+            address(sym,sPRI);    /* get starting address in primary register */
-          return TRUE;          /* return 1 if lvalue (not function or array) */
+            return FALSE;         /* return 0 for array (not lvalue) */
-        } /* if */
+          case iMETHODMAP:
+            return FALSE;
+          default:
+            return TRUE;          /* return 1 if lvalue (not function or array) */
+        } /* switch */
       } /* if */
     } else {
       if (!sc_allowproccall)
@@ -2466,7 +2477,6 @@ static int primary(value *lval)
     assert(sym!=NULL);
     assert(sym->ident==iFUNCTN || sym->ident==iREFFUNC);
     lval->sym=sym;
-    lval->proxy=alias;
     lval->ident=sym->ident;
     lval->tag=sym->tag;
     return FALSE;       /* return 0 for function (not an lvalue) */
@@ -2482,7 +2492,6 @@ static int primary(value *lval)
 static void clear_value(value *lval)
 {
   lval->sym=NULL;
-  lval->proxy=NULL;
   lval->constval=0L;
   lval->tag=0;
   lval->ident=0;

sourcepawn/compiler/sc5-in.scp

@@ -156,7 +156,7 @@ static const char *errmsg[] = {
 /*110*/  "%s has already been defined (previously seen as %s)\n",
 /*111*/  "expected identifier - did you forget a type?\n",
 /*112*/  "constructor function must return tag %s\n",
-/*113*/  "cannot define constructor for \"%s\"; already exists as a %s\n",
+/*113*/  "constructor for \"%s\" already exists\n",
 /*114*/  "missing type, or %s must have the same name as %s \"%s\"\n",
 /*115*/  "cannot use delete, %s %s has no destructor\n",
 /*116*/  "no methodmap or class was found for %s\n",
@@ -217,6 +217,7 @@ static const char *errmsg[] = {
 /*171*/  "cannot use 'new' with non-object-like methodmap '%s'\n",
 /*172*/  "methodmap '%s' does not have a constructor\n",
 /*173*/  "'%s' is a newly reserved keyword that may be used in the future; use a different name as an identifier\n",
+/*174*/  "symbol '%s' is a type and cannot be used as a value\n",
 #else
   "\315e\306\227\266k\217:\235\277bu\201fo\220\204\223\012",
   "\202l\224\250s\205g\346\356e\233\201(\243\315\214\267\202) \253 f\255low ea\305 \042c\353e\042\012",

sourcepawn/compiler/sctracker.h

@@ -100,10 +100,10 @@ typedef struct methodmap_method_s
   }
 } methodmap_method_t;
 
-typedef struct methodmap_s
+struct methodmap_t
 {
-  struct methodmap_s *next;
+  methodmap_t *next;
-  struct methodmap_s *parent;
+  methodmap_t *parent;
   int tag;
   int nullable;
   LayoutSpec spec;
@@ -114,7 +114,7 @@ typedef struct methodmap_s
   // Shortcut.
   methodmap_method_t *dtor;
   methodmap_method_t *ctor;
-} methodmap_t;
+};
 
 /**
  * Pawn Structs

sourcepawn/compiler/tests/fail-bad-arg-decls.txt

@@ -1,3 +1,3 @@
 (1) : error 001: expected token: "-identifier-", but found ")"
 (5) : error 140: new-style array types cannot specify dimension sizes as part of their type
-(9) : error 001: expected token: "-identifier-", but found ":"
+(9) : warning 238: 'int:' is an illegal cast; use view_as<int>(expression)

sourcepawn/compiler/tests/fail-methodmap-as-rvalue.sp

@@ -0,0 +1,17 @@
+native printnum(x);
+
+enum X {
+};
+
+methodmap X {
+	public X(int y) {
+		printnum(y);
+		return view_as<X>(0);
+	}
+};
+
+public main()
+{
+	X x = X(5);
+	return X;
+}

sourcepawn/compiler/tests/fail-methodmap-as-rvalue.txt

@@ -0,0 +1 @@
+(16) : error 174: symbol 'X' is a type and cannot be used as a value