From afeae84340614763c8df8e6291dbe4912c3d47b1 Mon Sep 17 00:00:00 2001 From: David Anderson Date: Thu, 11 Dec 2014 22:25:46 -0800 Subject: [PATCH] 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 | 10 +- sourcepawn/compiler/sc1.cpp | 115 +++++++++++------- sourcepawn/compiler/sc2.cpp | 36 ++---- sourcepawn/compiler/sc3.cpp | 67 +++++----- sourcepawn/compiler/sc5-in.scp | 3 +- sourcepawn/compiler/sctracker.h | 8 +- .../compiler/tests/fail-bad-arg-decls.txt | 2 +- .../tests/fail-methodmap-as-rvalue.sp | 17 +++ .../tests/fail-methodmap-as-rvalue.txt | 1 + 9 files changed, 148 insertions(+), 111 deletions(-) create mode 100644 sourcepawn/compiler/tests/fail-methodmap-as-rvalue.sp create mode 100644 sourcepawn/compiler/tests/fail-methodmap-as-rvalue.txt diff --git a/sourcepawn/compiler/sc.h b/sourcepawn/compiler/sc.h index d22c1b54..164f64bd 100644 --- a/sourcepawn/compiler/sc.h +++ b/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); diff --git a/sourcepawn/compiler/sc1.cpp b/sourcepawn/compiler/sc1.cpp index 65875021..cc4fcc37 100644 --- a/sourcepawn/compiler/sc1.cpp +++ b/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); - if (!can_redef_layout_spec(spec, old_spec)) + LayoutSpec old_spec = deduce_layout_spec_by_name(mapname); + 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" */ - 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)); + if (vclass == sGLOBAL) { + if ((enumsym = findglb(enumname, vclass)) != NULL) { + // If we were previously defined as a methodmap, don't overwrite the + // symbol. Otherwise, flow into add_constant where we will error. + if (enumsym->ident != iMETHODMAP) + enumsym = nullptr; + } + } + + 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: - // Ignore usage on proxies. + case iMETHODMAP: + // Ignore usage on methodmaps. break; default: /* a variable */ diff --git a/sourcepawn/compiler/sc2.cpp b/sourcepawn/compiler/sc2.cpp index c660b6fb..b8cb9187 100644 --- a/sourcepawn/compiler/sc2.cpp +++ b/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: - // Original loop determined it was okay to keep. - mustdelete=FALSE; + case iMETHODMAP: + // We delete methodmap symbols at the end, but since methodmaps + // 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; } diff --git a/sourcepawn/compiler/sc3.cpp b/sourcepawn/compiler/sc3.cpp index ca163ada..8a8c0314 100644 --- a/sourcepawn/compiler/sc3.cpp +++ b/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, &alias)) != 0) { + if ((sym = findglb(st, sSTATEVAR)) != 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) { - address(sym,sPRI); /* get starting address in primary register */ - return FALSE; /* return 0 for array (not lvalue) */ - } else { - return TRUE; /* return 1 if lvalue (not function or array) */ - } /* if */ + switch (sym->ident) { + case iARRAY: + case iREFARRAY: + address(sym,sPRI); /* get starting address in primary register */ + return FALSE; /* return 0 for array (not lvalue) */ + 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; diff --git a/sourcepawn/compiler/sc5-in.scp b/sourcepawn/compiler/sc5-in.scp index f9b3f0e0..a91e3928 100644 --- a/sourcepawn/compiler/sc5-in.scp +++ b/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", diff --git a/sourcepawn/compiler/sctracker.h b/sourcepawn/compiler/sctracker.h index 0e45cbfa..8ffee95d 100644 --- a/sourcepawn/compiler/sctracker.h +++ b/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; - struct methodmap_s *parent; + methodmap_t *next; + 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 diff --git a/sourcepawn/compiler/tests/fail-bad-arg-decls.txt b/sourcepawn/compiler/tests/fail-bad-arg-decls.txt index d92248fe..f6404967 100644 --- a/sourcepawn/compiler/tests/fail-bad-arg-decls.txt +++ b/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(expression) diff --git a/sourcepawn/compiler/tests/fail-methodmap-as-rvalue.sp b/sourcepawn/compiler/tests/fail-methodmap-as-rvalue.sp new file mode 100644 index 00000000..1218fca6 --- /dev/null +++ b/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(0); + } +}; + +public main() +{ + X x = X(5); + return X; +} diff --git a/sourcepawn/compiler/tests/fail-methodmap-as-rvalue.txt b/sourcepawn/compiler/tests/fail-methodmap-as-rvalue.txt new file mode 100644 index 00000000..7dfa5277 --- /dev/null +++ b/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