From afeae84340614763c8df8e6291dbe4912c3d47b1 Mon Sep 17 00:00:00 2001
From: David Anderson <dvander@alliedmods.net>
Date: Thu, 11 Dec 2014 22:25:46 -0800
Subject: [PATCH 1/3] 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<int>(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<X>(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
From 5b69efe5d4a113c453b8706a65353363707b23e0 Mon Sep 17 00:00:00 2001
From: David Anderson <dvander@alliedmods.net>
Date: Thu, 11 Dec 2014 23:18:32 -0800
Subject: [PATCH 2/3] Add static method support to methodmaps.
---
sourcepawn/compiler/sc1.cpp | 32 +++-
sourcepawn/compiler/sc3.cpp | 150 ++++++++++++------
sourcepawn/compiler/sc5-in.scp | 3 +
sourcepawn/compiler/sctracker.h | 1 +
.../tests/fail-call-non-static-on-type.sp | 12 ++
.../tests/fail-call-non-static-on-type.txt | 1 +
.../tests/fail-call-static-via-instance.sp | 13 ++
.../tests/fail-call-static-via-instance.txt | 1 +
.../compiler/tests/ok-call-static-method.sp | 12 ++
9 files changed, 167 insertions(+), 58 deletions(-)
create mode 100644 sourcepawn/compiler/tests/fail-call-non-static-on-type.sp
create mode 100644 sourcepawn/compiler/tests/fail-call-non-static-on-type.txt
create mode 100644 sourcepawn/compiler/tests/fail-call-static-via-instance.sp
create mode 100644 sourcepawn/compiler/tests/fail-call-static-via-instance.txt
create mode 100644 sourcepawn/compiler/tests/ok-call-static-method.sp
diff --git a/sourcepawn/compiler/sc1.cpp b/sourcepawn/compiler/sc1.cpp
index cc4fcc37..892cd32b 100644
--- a/sourcepawn/compiler/sc1.cpp
+++ b/sourcepawn/compiler/sc1.cpp
@@ -3625,7 +3625,13 @@ static void make_primitive(typeinfo_t *type, int tag)
type->ident = iVARIABLE;
}
-symbol *parse_inline_function(methodmap_t *map, const typeinfo_t *type, const char *name, int is_native, int is_ctor, int is_dtor)
+symbol *parse_inline_function(methodmap_t *map,
+ const typeinfo_t *type,
+ const char *name,
+ int is_native,
+ int is_ctor,
+ int is_dtor,
+ bool is_static)
{
declinfo_t decl;
memset(&decl, 0, sizeof(decl));
@@ -3640,7 +3646,7 @@ symbol *parse_inline_function(methodmap_t *map, const typeinfo_t *type, const ch
decl.type.is_new = TRUE;
const int *thistag = NULL;
- if (!is_ctor)
+ if (!is_ctor && !is_static)
thistag = &map->tag;
// Build a new symbol. Construct a temporary name including the class.
@@ -3752,7 +3758,7 @@ int parse_property_accessor(const typeinfo_t *type, methodmap_t *map, methodmap_
ret_type = &voidtype;
}
- target = parse_inline_function(map, ret_type, tmpname, is_native, FALSE, FALSE);
+ target = parse_inline_function(map, ret_type, tmpname, is_native, FALSE, FALSE, false);
}
if (!target)
@@ -3851,8 +3857,12 @@ methodmap_method_t *parse_method(methodmap_t *map)
int is_dtor = 0;
int is_bind = 0;
int is_native = 0;
+ bool is_static = false;
const char *spectype = layout_spec_name(map->spec);
+ if (matchtoken(tSTATIC))
+ is_static = true;
+
// This stores the name of the method (for destructors, we add a ~).
token_ident_t ident;
strcpy(ident.name, "__unknown__");
@@ -3864,7 +3874,8 @@ methodmap_method_t *parse_method(methodmap_t *map)
typeinfo_t type;
memset(&type, 0, sizeof(type));
- if (matchtoken('~')) {
+ // Destructors cannot be static.
+ if (!is_static && matchtoken('~')) {
// We got something like "public ~Blah = X"
is_bind = TRUE;
is_dtor = TRUE;
@@ -3951,6 +3962,11 @@ methodmap_method_t *parse_method(methodmap_t *map)
error(114, "constructor", spectype, map->name);
}
+ if (is_ctor && is_static) {
+ // Constructors may not be static.
+ error(175);
+ }
+
symbol *target = NULL;
if (is_bind) {
// Find an existing symbol.
@@ -3960,7 +3976,7 @@ methodmap_method_t *parse_method(methodmap_t *map)
else if (target->ident != iFUNCTN)
error(10);
} else {
- target = parse_inline_function(map, &type, ident.name, is_native, is_ctor, is_dtor);
+ target = parse_inline_function(map, &type, ident.name, is_native, is_ctor, is_dtor, is_static);
}
if (!target)
@@ -3998,13 +4014,15 @@ methodmap_method_t *parse_method(methodmap_t *map)
method->target = target;
method->getter = NULL;
method->setter = NULL;
+ method->is_static = is_static;
// If the symbol is a constructor, we bypass the initial argument checks.
if (is_ctor) {
if (map->ctor)
error(113, map->name);
- } else if (!check_this_tag(map, target)) {
- error(108, spectype, map->name);
+ } else if (!is_static) {
+ if (!check_this_tag(map, target))
+ error(108, spectype, map->name);
}
if (is_dtor)
diff --git a/sourcepawn/compiler/sc3.cpp b/sourcepawn/compiler/sc3.cpp
index 8a8c0314..e7c5dd5b 100644
--- a/sourcepawn/compiler/sc3.cpp
+++ b/sourcepawn/compiler/sc3.cpp
@@ -2001,6 +2001,96 @@ static int hier2(value *lval)
} /* switch */
}
+static symbol *
+fake_function_for_method(methodmap_t *map, const char *lexstr)
+{
+ // Fetch a fake function so errors aren't as crazy.
+ char tmpname[METHOD_NAMEMAX + 1];
+ strcpy(tmpname, map->name);
+ strcat(tmpname, ".");
+ strcat(tmpname, lexstr);
+ tmpname[sNAMEMAX] = '\0';
+ return fetchfunc(tmpname);
+}
+
+enum FieldExprResult
+{
+ FER_Fail,
+ FER_Accessor,
+ FER_CallFunction,
+ FER_CallMethod
+};
+
+static FieldExprResult
+field_expression(svalue &thisval, value *lval, symbol **target)
+{
+ // Catch invalid calls early so we don't compile with a tag mismatch.
+ switch (thisval.val.ident) {
+ case iARRAY:
+ case iREFARRAY:
+ error(106);
+ break;
+
+ case iFUNCTN:
+ case iREFFUNC:
+ error(107);
+ break;
+ }
+
+ cell lexval;
+ char *lexstr;
+ if (!needtoken(tSYMBOL))
+ return FER_Fail;
+ tokeninfo(&lexval, &lexstr);
+
+ if (thisval.val.ident == iMETHODMAP) {
+ methodmap_t *map = thisval.val.sym->methodmap;
+ methodmap_method_t *method = methodmap_find_method(map, lexstr);
+ if (!method) {
+ error(105, map->name, lexstr);
+ *target = fake_function_for_method(map, lexstr);
+ return FER_CallFunction;
+ }
+
+ if (!method->is_static)
+ error(176, method->name, map->name);
+ *target = method->target;
+ return FER_CallFunction;
+ }
+
+ methodmap_t *map;
+ if ((map = methodmap_find_by_tag(thisval.val.tag)) == NULL) {
+ error(104, pc_tagname(thisval.val.tag));
+ return FER_Fail;
+ }
+
+ methodmap_method_t *method;
+ if ((method = methodmap_find_method(map, lexstr)) == NULL) {
+ error(105, map->name, lexstr);
+ *target = fake_function_for_method(map, lexstr);
+ return FER_CallFunction;
+ }
+
+ if (method && (method->getter || method->setter)) {
+ if (thisval.lvalue)
+ rvalue(lval);
+ clear_value(lval);
+ lval->ident = iACCESSOR;
+ lval->tag = method->property_tag();
+ lval->accessor = method;
+ return FER_Accessor;
+ }
+
+ *target = method->target;
+
+ if (method->is_static) {
+ error(177, method->name, map->name, method->name);
+ return FER_CallFunction;
+ }
+ return FER_CallMethod;
+}
+
+
/* hier1
*
* The highest hierarchy level: it looks for pointer and array indices
@@ -2223,59 +2313,17 @@ restart:
svalue *implicitthis = NULL;
if (tok == '.') {
- methodmap_t *map;
-
- /* Catch invalid calls early so we don't compile with a tag mismatch. */
- switch (thisval.val.ident) {
- case iARRAY:
- case iREFARRAY:
- error(106);
+ switch (field_expression(thisval, lval1, &sym)) {
+ case FER_Fail:
+ case FER_CallFunction:
break;
-
- case iFUNCTN:
- case iREFFUNC:
- error(107);
- break;
- }
-
- if ((map = methodmap_find_by_tag(thisval.val.tag)) == NULL) {
- error(104, pc_tagname(thisval.val.tag));
- }
-
- if (needtoken(tSYMBOL) && map) {
- cell lexval;
- char *lexstr;
- methodmap_method_t *method;
-
- tokeninfo(&lexval, &lexstr);
- if ((method = methodmap_find_method(map, lexstr)) == NULL)
- error(105, map->name, lexstr);
-
- if (method && (method->getter || method->setter)) {
- if (lvalue)
- rvalue(lval1);
- clear_value(lval1);
- lval1->ident = iACCESSOR;
- lval1->tag = method->property_tag();
- lval1->accessor = method;
- lvalue = TRUE;
- goto restart;
- }
-
- if (!method || !method->target) {
- error(105, map->name, lexstr);
-
- // Fetch a fake function so errors aren't as crazy.
- char tmpname[METHOD_NAMEMAX + 1];
- strcpy(tmpname, map->name);
- strcat(tmpname, ".");
- strcat(tmpname, lexstr);
- tmpname[sNAMEMAX] = '\0';
- sym = fetchfunc(tmpname);
- } else {
+ case FER_CallMethod:
implicitthis = &thisval;
- sym = method->target;
- }
+ break;
+ case FER_Accessor:
+ goto restart;
+ default:
+ assert(false);
}
// If we don't find a '(' next, just fail to compile for now -- and
diff --git a/sourcepawn/compiler/sc5-in.scp b/sourcepawn/compiler/sc5-in.scp
index a91e3928..7968cf5b 100644
--- a/sourcepawn/compiler/sc5-in.scp
+++ b/sourcepawn/compiler/sc5-in.scp
@@ -218,6 +218,9 @@ static const char *errmsg[] = {
/*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",
+/*175*/ "constructors cannot be static\n",
+/*176*/ "non-static method or property '%s' must be called with a value of type '%s'\n",
+/*177*/ "static method '%s' must be invoked via its type (try '%s.%s')\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 8ffee95d..27f44ecc 100644
--- a/sourcepawn/compiler/sctracker.h
+++ b/sourcepawn/compiler/sctracker.h
@@ -85,6 +85,7 @@ typedef struct methodmap_method_s
symbol *target;
symbol *getter;
symbol *setter;
+ bool is_static;
int property_tag() const {
assert(getter || setter);
diff --git a/sourcepawn/compiler/tests/fail-call-non-static-on-type.sp b/sourcepawn/compiler/tests/fail-call-non-static-on-type.sp
new file mode 100644
index 00000000..ac6df284
--- /dev/null
+++ b/sourcepawn/compiler/tests/fail-call-non-static-on-type.sp
@@ -0,0 +1,12 @@
+native printnum(t);
+
+methodmap X
+{
+ public int GetThing() {
+ return 10;
+ }
+}
+
+public main() {
+ printnum(X.GetThing());
+}
diff --git a/sourcepawn/compiler/tests/fail-call-non-static-on-type.txt b/sourcepawn/compiler/tests/fail-call-non-static-on-type.txt
new file mode 100644
index 00000000..42c312db
--- /dev/null
+++ b/sourcepawn/compiler/tests/fail-call-non-static-on-type.txt
@@ -0,0 +1 @@
+(11) : error 176: non-static method or property 'GetThing' must be called with a value of type 'X'
diff --git a/sourcepawn/compiler/tests/fail-call-static-via-instance.sp b/sourcepawn/compiler/tests/fail-call-static-via-instance.sp
new file mode 100644
index 00000000..24a16591
--- /dev/null
+++ b/sourcepawn/compiler/tests/fail-call-static-via-instance.sp
@@ -0,0 +1,13 @@
+native printnum(t);
+
+methodmap X
+{
+ public static int GetThing() {
+ return 10;
+ }
+}
+
+public main() {
+ X x;
+ printnum(x.GetThing());
+}
diff --git a/sourcepawn/compiler/tests/fail-call-static-via-instance.txt b/sourcepawn/compiler/tests/fail-call-static-via-instance.txt
new file mode 100644
index 00000000..9a7dbc95
--- /dev/null
+++ b/sourcepawn/compiler/tests/fail-call-static-via-instance.txt
@@ -0,0 +1 @@
+(12) : error 177: static method 'GetThing' must be invoked via its type (try 'X.GetThing')
diff --git a/sourcepawn/compiler/tests/ok-call-static-method.sp b/sourcepawn/compiler/tests/ok-call-static-method.sp
new file mode 100644
index 00000000..19a887ea
--- /dev/null
+++ b/sourcepawn/compiler/tests/ok-call-static-method.sp
@@ -0,0 +1,12 @@
+native printnum(t);
+
+methodmap X
+{
+ public static int GetThing() {
+ return 10;
+ }
+}
+
+public main() {
+ printnum(X.GetThing());
+}
From 61bf7de1015a8a3450765bf1305514fc470aea36 Mon Sep 17 00:00:00 2001
From: David Anderson <dvander@alliedmods.net>
Date: Thu, 11 Dec 2014 23:34:00 -0800
Subject: [PATCH 3/3] Revert adminmenu API changes.
---
extensions/topmenus/smn_topmenus.cpp | 16 ++++++++++++++++
plugins/basebans.sp | 4 +++-
plugins/basecomm.sp | 4 +++-
plugins/basecommands.sp | 4 +++-
plugins/basevotes.sp | 4 +++-
plugins/funcommands.sp | 4 +++-
plugins/funvotes.sp | 4 +++-
plugins/include/adminmenu.inc | 4 ++--
plugins/include/topmenus.inc | 5 +++++
plugins/playercommands.sp | 4 +++-
10 files changed, 44 insertions(+), 9 deletions(-)
diff --git a/extensions/topmenus/smn_topmenus.cpp b/extensions/topmenus/smn_topmenus.cpp
index 247c0db9..3dc0e380 100644
--- a/extensions/topmenus/smn_topmenus.cpp
+++ b/extensions/topmenus/smn_topmenus.cpp
@@ -449,6 +449,21 @@ static cell_t TopMenu_AddCategory(IPluginContext *pContext, const cell_t *params
return AddToTopMenu(pContext, new_params);
}
+static cell_t TopMenu_FromHandle(IPluginContext *pContext, const cell_t *params)
+{
+ HandleError err;
+ TopMenu *pMenu;
+ HandleSecurity sec(pContext->GetIdentity(), myself->GetIdentity());
+
+ if ((err = handlesys->ReadHandle(params[1], hTopMenuType, &sec, (void **)&pMenu))
+ != HandleError_None)
+ {
+ return pContext->ThrowNativeError("Invalid Handle %x (error: %d)", params[1], err);
+ }
+
+ return params[1];
+}
+
sp_nativeinfo_t g_TopMenuNatives[] =
{
{"AddToTopMenu", AddToTopMenu},
@@ -474,6 +489,7 @@ sp_nativeinfo_t g_TopMenuNatives[] =
{"TopMenu.GetInfoString", GetTopMenuInfoString},
{"TopMenu.GetObjName", GetTopMenuName},
{"TopMenu.CacheTitles.set", SetTopMenuTitleCaching},
+ {"TopMenu.FromHandle", TopMenu_FromHandle},
{NULL, NULL},
};
diff --git a/plugins/basebans.sp b/plugins/basebans.sp
index 0ecb2bd0..99498c0d 100644
--- a/plugins/basebans.sp
+++ b/plugins/basebans.sp
@@ -124,8 +124,10 @@ LoadBanReasons()
}
}
-public OnAdminMenuReady(TopMenu topmenu)
+public OnAdminMenuReady(Handle aTopMenu)
{
+ TopMenu topmenu = TopMenu.FromHandle(aTopMenu);
+
/* Block us from being called twice */
if (topmenu == hTopMenu)
{
diff --git a/plugins/basecomm.sp b/plugins/basecomm.sp
index 6c2993e7..0b176198 100644
--- a/plugins/basecomm.sp
+++ b/plugins/basecomm.sp
@@ -100,8 +100,10 @@ public OnPluginStart()
}
}
-public OnAdminMenuReady(TopMenu topmenu)
+public OnAdminMenuReady(Handle aTopMenu)
{
+ TopMenu topmenu = TopMenu.FromHandle(aTopMenu);
+
/* Block us from being called twice */
if (topmenu == hTopMenu)
{
diff --git a/plugins/basecommands.sp b/plugins/basecommands.sp
index 5bc80f6d..24593358 100644
--- a/plugins/basecommands.sp
+++ b/plugins/basecommands.sp
@@ -146,8 +146,10 @@ bool:IsClientAllowedToChangeCvar(client, const String:cvarname[])
return allowed;
}
-public OnAdminMenuReady(TopMenu topmenu)
+public OnAdminMenuReady(Handle aTopMenu)
{
+ TopMenu topmenu = TopMenu.FromHandle(aTopMenu);
+
/* Block us from being called twice */
if (topmenu == hTopMenu)
{
diff --git a/plugins/basevotes.sp b/plugins/basevotes.sp
index 943a5e5d..5aea62a8 100644
--- a/plugins/basevotes.sp
+++ b/plugins/basevotes.sp
@@ -132,8 +132,10 @@ public OnConfigsExecuted()
g_mapCount = LoadMapList(g_MapList);
}
-public OnAdminMenuReady(TopMenu topmenu)
+public OnAdminMenuReady(Handle aTopMenu)
{
+ TopMenu topmenu = TopMenu.FromHandle(aTopMenu);
+
/* Block us from being called twice */
if (topmenu == hTopMenu)
{
diff --git a/plugins/funcommands.sp b/plugins/funcommands.sp
index 78dcb295..8b92efbe 100644
--- a/plugins/funcommands.sp
+++ b/plugins/funcommands.sp
@@ -258,8 +258,10 @@ public Action:Event_RoundEnd(Handle:event,const String:name[],bool:dontBroadcast
KillAllDrugs();
}
-public OnAdminMenuReady(TopMenu topmenu)
+public OnAdminMenuReady(Handle aTopMenu)
{
+ TopMenu topmenu = TopMenu.FromHandle(aTopMenu);
+
/* Block us from being called twice */
if (topmenu == hTopMenu)
{
diff --git a/plugins/funvotes.sp b/plugins/funvotes.sp
index 2d2c6e2e..d975ab2c 100644
--- a/plugins/funvotes.sp
+++ b/plugins/funvotes.sp
@@ -135,8 +135,10 @@ public OnPluginStart()
}
}
-public OnAdminMenuReady(TopMenu topmenu)
+public OnAdminMenuReady(Handle aTopMenu)
{
+ TopMenu topmenu = TopMenu.FromHandle(aTopMenu);
+
/* Block us from being called twice */
if (topmenu == hTopMenu)
{
diff --git a/plugins/include/adminmenu.inc b/plugins/include/adminmenu.inc
index db5cacdb..b3662c04 100644
--- a/plugins/include/adminmenu.inc
+++ b/plugins/include/adminmenu.inc
@@ -65,7 +65,7 @@
* @param topmenu Handle to the admin menu's TopMenu.
* @noreturn
*/
-forward OnAdminMenuCreated(TopMenu topmenu);
+forward OnAdminMenuCreated(Handle topmenu);
/**
* Called when the admin menu is ready to have items added.
@@ -73,7 +73,7 @@ forward OnAdminMenuCreated(TopMenu topmenu);
* @param topmenu Handle to the admin menu's TopMenu.
* @noreturn
*/
-forward OnAdminMenuReady(TopMenu topmenu);
+forward OnAdminMenuReady(Handle topmenu);
/**
* Retrieves the Handle to the admin top menu.
diff --git a/plugins/include/topmenus.inc b/plugins/include/topmenus.inc
index 83f99ac1..817dec51 100644
--- a/plugins/include/topmenus.inc
+++ b/plugins/include/topmenus.inc
@@ -146,6 +146,11 @@ methodmap TopMenu < Handle
// @return A new TopMenu.
public native TopMenu(TopMenuHandler handler);
+ // Returns a TopMenu handle from a generic handle. If the given handle is
+ // a TopMenu, the handle is simply casted back. Otherwise, an error is
+ // raised.
+ public static native TopMenu FromHandle(Handle handle);
+
// Re-sorts the items in a TopMenu via a configuration file.
//
// The format of the configuration file should be a Valve Key-Values
diff --git a/plugins/playercommands.sp b/plugins/playercommands.sp
index f0ab772e..7ddcee54 100644
--- a/plugins/playercommands.sp
+++ b/plugins/playercommands.sp
@@ -76,8 +76,10 @@ public OnPluginStart()
}
}
-public OnAdminMenuReady(TopMenu topmenu)
+public OnAdminMenuReady(Handle aTopMenu)
{
+ TopMenu topmenu = TopMenu.FromHandle(aTopMenu);
+
/* Block us from being called twice */
if (topmenu == hTopMenu)
{