[PATCH v2 0/4] MR5906: widl: Respect u and l modifiers in expressions.
-- v2: widl: Respect u and l modifiers in expressions. widl: Use struct integer for the aNUM and aHEXNUM tokens. widl: Store the hexadecimal flag inside of the expr_t union. widl: Do not allow "lu" as an integer suffix. https://gitlab.winehq.org/wine/wine/-/merge_requests/5906
On Mon Jun 24 08:05:59 2024 +0000, Rémi Bernon wrote:
oaidl.idl, oledb.idl for instance. Thanks. I had to search through those files for a specific broken enum, but I found it in CALLCONV. I've fixed that bug in the latest revision.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/5906#note_74245
From: Elizabeth Figura <zfigura(a)codeweavers.com> MIDL does not. --- tools/widl/parser.l | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tools/widl/parser.l b/tools/widl/parser.l index 7a1ae18abdc..75cdae22550 100644 --- a/tools/widl/parser.l +++ b/tools/widl/parser.l @@ -431,8 +431,8 @@ SAFEARRAY{ws}*/\( return tSAFEARRAY; [a-zA-Z_][0-9a-zA-Z_]* { return token_ident( yytext, yylval ); } - 0[xX]{hd}+([lL][uU]?|[uU][lL]?)? { return token_num( aHEXNUM, yytext, yylval ); } - [0-9]+([lL][uU]?|[uU][lL]?)? { return token_num( aNUM, yytext, yylval ); } + 0[xX]{hd}+[uU]?[lL]? { return token_num( aHEXNUM, yytext, yylval ); } + [0-9]+[uU]?[lL]? { return token_num( aNUM, yytext, yylval ); } L\"(\\.|[^"\\])*\" { return token_str( aWSTRING, yytext + 1, yylval ); } \"(\\.|[^"\\])*\" { return token_str( aSTRING, yytext, yylval ); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5906
From: Elizabeth Figura <zfigura(a)codeweavers.com> --- tools/widl/expr.c | 35 +++++++++++--------------- tools/widl/expr.h | 2 +- tools/widl/header.c | 4 +-- tools/widl/parser.y | 56 +++++++++++++++++++++++++---------------- tools/widl/widltypes.h | 9 +++++-- tools/widl/write_msft.c | 7 +++--- tools/widl/write_sltg.c | 2 +- 7 files changed, 63 insertions(+), 52 deletions(-) diff --git a/tools/widl/expr.c b/tools/widl/expr.c index e0c56b81089..e51ed6c94c4 100644 --- a/tools/widl/expr.c +++ b/tools/widl/expr.c @@ -112,28 +112,24 @@ static int is_float_type(const type_t *type) expr_t *make_expr(enum expr_type type) { expr_t *e = xmalloc(sizeof(expr_t)); + memset(e, 0, sizeof(*e)); e->type = type; - e->ref = NULL; - e->u.lval = 0; - e->is_const = FALSE; - e->cval = 0; return e; } -expr_t *make_exprl(enum expr_type type, int val) +expr_t *make_exprl(enum expr_type type, const struct integer *integer) { expr_t *e = xmalloc(sizeof(expr_t)); + memset(e, 0, sizeof(*e)); e->type = type; - e->ref = NULL; - e->u.lval = val; - e->is_const = FALSE; + e->u.integer = *integer; /* check for numeric constant */ - if (type == EXPR_NUM || type == EXPR_HEXNUM || type == EXPR_TRUEFALSE) + if (type == EXPR_NUM || type == EXPR_TRUEFALSE) { /* make sure true/false value is valid */ - assert(type != EXPR_TRUEFALSE || val == 0 || val == 1); + assert(type != EXPR_TRUEFALSE || integer->value == 0 || integer->value == 1); e->is_const = TRUE; - e->cval = val; + e->cval = integer->value; } return e; } @@ -252,10 +248,9 @@ expr_t *make_expr1(enum expr_type type, expr_t *expr) { expr_t *e; e = xmalloc(sizeof(expr_t)); + memset(e, 0, sizeof(*e)); e->type = type; e->ref = expr; - e->u.lval = 0; - e->is_const = FALSE; /* check for compile-time optimization */ if (expr->is_const) { @@ -514,7 +509,6 @@ static struct expression_type resolve_expression(const struct expr_loc *expr_loc { case EXPR_VOID: break; - case EXPR_HEXNUM: case EXPR_NUM: case EXPR_TRUEFALSE: result.is_temporary = FALSE; @@ -692,16 +686,16 @@ void write_expr(FILE *h, const expr_t *e, int brackets, case EXPR_VOID: break; case EXPR_NUM: - fprintf(h, "%u", e->u.lval); - break; - case EXPR_HEXNUM: - fprintf(h, "0x%x", e->u.lval); + if (e->u.integer.is_hex) + fprintf(h, "0x%x", e->u.integer.value); + else + fprintf(h, "%u", e->u.integer.value); break; case EXPR_DOUBLE: fprintf(h, "%#.15g", e->u.dval); break; case EXPR_TRUEFALSE: - if (e->u.lval == 0) + if (e->u.integer.value == 0) fprintf(h, "FALSE"); else fprintf(h, "TRUE"); @@ -869,9 +863,8 @@ int compare_expr(const expr_t *a, const expr_t *b) switch (a->type) { case EXPR_NUM: - case EXPR_HEXNUM: case EXPR_TRUEFALSE: - return a->u.lval - b->u.lval; + return a->u.integer.value - b->u.integer.value; case EXPR_DOUBLE: return a->u.dval - b->u.dval; case EXPR_IDENTIFIER: diff --git a/tools/widl/expr.h b/tools/widl/expr.h index 7b0e1b2c7ba..89c0acf5491 100644 --- a/tools/widl/expr.h +++ b/tools/widl/expr.h @@ -26,7 +26,7 @@ struct expr_loc }; extern expr_t *make_expr(enum expr_type type); -extern expr_t *make_exprl(enum expr_type type, int val); +extern expr_t *make_exprl(enum expr_type type, const struct integer *integer); extern expr_t *make_exprd(enum expr_type type, double val); extern expr_t *make_exprs(enum expr_type type, char *val); extern expr_t *make_exprt(enum expr_type type, var_t *var, expr_t *expr); diff --git a/tools/widl/header.c b/tools/widl/header.c index bb30156b8a3..413354d5b56 100644 --- a/tools/widl/header.c +++ b/tools/widl/header.c @@ -1583,7 +1583,7 @@ static void write_apicontract_guard_start(FILE *header, const expr_t *expr) int ver; if (!winrt_mode) return; type = expr->u.tref.type; - ver = expr->ref->u.lval; + ver = expr->ref->u.integer.value; name = format_apicontract_macro(type); fprintf(header, "#if %s_VERSION >= %#x\n", name, ver); free(name); @@ -1596,7 +1596,7 @@ static void write_apicontract_guard_end(FILE *header, const expr_t *expr) int ver; if (!winrt_mode) return; type = expr->u.tref.type; - ver = expr->ref->u.lval; + ver = expr->ref->u.integer.value; name = format_apicontract_macro(type); fprintf(header, "#endif /* %s_VERSION >= %#x */\n", name, ver); free(name); diff --git a/tools/widl/parser.y b/tools/widl/parser.y index f5b82f29db3..3d64587d526 100644 --- a/tools/widl/parser.y +++ b/tools/widl/parser.y @@ -596,12 +596,14 @@ contract_ver: ; contract_req - : decl_spec ',' contract_ver { if ($1->type->type_type != TYPE_APICONTRACT) - error_loc("type %s is not an apicontract\n", $1->type->name); - $$ = make_exprl(EXPR_NUM, $3); - $$ = make_exprt(EXPR_GTREQL, declare_var(NULL, $1, make_declarator(NULL), 0), $$); - } - ; + : decl_spec ',' contract_ver { + struct integer integer = {.value = $3}; + if ($1->type->type_type != TYPE_APICONTRACT) + error_loc("type %s is not an apicontract\n", $1->type->name); + $$ = make_exprl(EXPR_NUM, &integer); + $$ = make_exprt(EXPR_GTREQL, declare_var(NULL, $1, make_declarator(NULL), 0), $$); + } + ; static_attr : decl_spec ',' contract_req { if ($1->type->type_type != TYPE_INTERFACE) @@ -793,21 +795,28 @@ enums | enum_list ; -enum_list: enum { if (!$1->eval) - $1->eval = make_exprl(EXPR_NUM, 0 /* default for first enum entry */); +enum_list: enum { + struct integer integer = {.value = 0}; + if (!$1->eval) + $1->eval = make_exprl(EXPR_NUM, &integer); $$ = append_var( NULL, $1 ); - } - | enum_list ',' enum { if (!$3->eval) + } + | enum_list ',' enum { + if (!$3->eval) { var_t *last = LIST_ENTRY( list_tail($$), var_t, entry ); - enum expr_type type = EXPR_NUM; - if (last->eval->type == EXPR_HEXNUM) type = EXPR_HEXNUM; - if (last->eval->cval + 1 < 0) type = EXPR_HEXNUM; - $3->eval = make_exprl(type, last->eval->cval + 1); + struct integer integer; + + if (last->eval->type == EXPR_NUM) + integer.is_hex = last->eval->u.integer.is_hex; + integer.value = last->eval->cval + 1; + if (integer.value < 0) + integer.is_hex = TRUE; + $3->eval = make_exprl(EXPR_NUM, &integer); } $$ = append_var( $1, $3 ); - } - ; + } + ; enum_member: m_attributes ident { $$ = $2; $$->attrs = check_enum_member_attrs($1); @@ -835,12 +844,17 @@ m_expr | expr ; -expr: aNUM { $$ = make_exprl(EXPR_NUM, $1); } - | aHEXNUM { $$ = make_exprl(EXPR_HEXNUM, $1); } +expr: aNUM { struct integer integer = {.value = $1}; + $$ = make_exprl(EXPR_NUM, &integer); } + | aHEXNUM { struct integer integer = {.value = $1, .is_hex = TRUE}; + $$ = make_exprl(EXPR_NUM, &integer); } | aDOUBLE { $$ = make_exprd(EXPR_DOUBLE, $1); } - | tFALSE { $$ = make_exprl(EXPR_TRUEFALSE, 0); } - | tNULL { $$ = make_exprl(EXPR_NUM, 0); } - | tTRUE { $$ = make_exprl(EXPR_TRUEFALSE, 1); } + | tFALSE { struct integer integer = {.value = 0}; + $$ = make_exprl(EXPR_TRUEFALSE, &integer); } + | tNULL { struct integer integer = {.value = 0}; + $$ = make_exprl(EXPR_NUM, &integer); } + | tTRUE { struct integer integer = {.value = 1}; + $$ = make_exprl(EXPR_TRUEFALSE, &integer); } | aSTRING { $$ = make_exprs(EXPR_STRLIT, $1); } | aWSTRING { $$ = make_exprs(EXPR_WSTRLIT, $1); } | aSQSTRING { $$ = make_exprs(EXPR_CHARCONST, $1); } diff --git a/tools/widl/widltypes.h b/tools/widl/widltypes.h index 7bc24fad0d3..44559e9f2ef 100644 --- a/tools/widl/widltypes.h +++ b/tools/widl/widltypes.h @@ -191,7 +191,6 @@ enum expr_type { EXPR_VOID, EXPR_NUM, - EXPR_HEXNUM, EXPR_DOUBLE, EXPR_IDENTIFIER, EXPR_NEG, @@ -349,11 +348,17 @@ struct _attr_t { struct location where; }; +struct integer +{ + int value; + int is_hex; +}; + struct _expr_t { enum expr_type type; const expr_t *ref; union { - int lval; + struct integer integer; double dval; const char *sval; const expr_t *ext; diff --git a/tools/widl/write_msft.c b/tools/widl/write_msft.c index b9a3d6664f2..66401fd09ed 100644 --- a/tools/widl/write_msft.c +++ b/tools/widl/write_msft.c @@ -1299,9 +1299,8 @@ static void set_custdata_attr(msft_typelib_t *typelib, attr_custdata_t *custdata case EXPR_WSTRLIT: set_custdata(typelib, &custdata->id, VT_BSTR, custdata->pval->u.sval, offset); break; - case EXPR_HEXNUM: case EXPR_NUM: - set_custdata(typelib, &custdata->id, VT_I4, &custdata->pval->u.lval, offset); + set_custdata(typelib, &custdata->id, VT_I4, &custdata->pval->u.integer.value, offset); break; default: error("custom() attribute with unknown type\n"); @@ -1395,7 +1394,7 @@ static int add_func_desc(msft_typeinfo_t* typeinfo, var_t *func, int index) break; case ATTR_HELPCONTEXT: extra_attr = max(extra_attr, 1); - help_context = expr->u.lval; + help_context = expr->u.integer.value; break; case ATTR_HELPSTRING: extra_attr = max(extra_attr, 2); @@ -1403,7 +1402,7 @@ static int add_func_desc(msft_typeinfo_t* typeinfo, var_t *func, int index) break; case ATTR_HELPSTRINGCONTEXT: extra_attr = max(extra_attr, 6); - help_string_context = expr->u.lval; + help_string_context = expr->u.integer.value; break; case ATTR_HIDDEN: funcflags |= 0x40; /* FUNCFLAG_FHIDDEN */ diff --git a/tools/widl/write_sltg.c b/tools/widl/write_sltg.c index d09fdb37c5c..5429b60c607 100644 --- a/tools/widl/write_sltg.c +++ b/tools/widl/write_sltg.c @@ -1060,7 +1060,7 @@ static int get_func_flags(const var_t *func, int *dispid, int *invokekind, int * flags |= 0x10; /* FUNCFLAG_FDISPLAYBIND */ break; case ATTR_HELPCONTEXT: - *helpcontext = expr->u.lval; + *helpcontext = expr->u.integer.value; break; case ATTR_HELPSTRING: *helpstring = attr->u.pval; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5906
From: Elizabeth Figura <zfigura(a)codeweavers.com> --- tools/widl/parser.l | 11 ++++++----- tools/widl/parser.y | 23 +++++++++++------------ 2 files changed, 17 insertions(+), 17 deletions(-) diff --git a/tools/widl/parser.l b/tools/widl/parser.l index 75cdae22550..1e7c543e8af 100644 --- a/tools/widl/parser.l +++ b/tools/widl/parser.l @@ -154,10 +154,11 @@ static int token_str( int token, const char *str, YYSTYPE *yylval ) return token; } -static int token_num( int token, const char *yytext, YYSTYPE *yylval ) +static int token_num( const char *yytext, YYSTYPE *yylval, int is_hex ) { - yylval->num = xstrtoul( yytext, NULL, 0 ); - return token; + yylval->integer.value = xstrtoul( yytext, NULL, 0 ); + yylval->integer.is_hex = is_hex; + return is_hex ? aHEXNUM : aNUM; } static int token_ident( const char *str, YYSTYPE *yylval ) @@ -431,8 +432,8 @@ SAFEARRAY{ws}*/\( return tSAFEARRAY; [a-zA-Z_][0-9a-zA-Z_]* { return token_ident( yytext, yylval ); } - 0[xX]{hd}+[uU]?[lL]? { return token_num( aHEXNUM, yytext, yylval ); } - [0-9]+[uU]?[lL]? { return token_num( aNUM, yytext, yylval ); } + 0[xX]{hd}+[uU]?[lL]? { return token_num( yytext, yylval, TRUE ); } + [0-9]+[uU]?[lL]? { return token_num( yytext, yylval, FALSE ); } L\"(\\.|[^"\\])*\" { return token_str( aWSTRING, yytext + 1, yylval ); } \"(\\.|[^"\\])*\" { return token_str( aSTRING, yytext, yylval ); } diff --git a/tools/widl/parser.y b/tools/widl/parser.y index 3d64587d526..062edfd2079 100644 --- a/tools/widl/parser.y +++ b/tools/widl/parser.y @@ -151,6 +151,7 @@ PARSER_LTYPE pop_import(void); char *str; struct uuid *uuid; unsigned int num; + struct integer integer; double dbl; typelib_t *typelib; struct _import_t *import; @@ -163,7 +164,7 @@ PARSER_LTYPE pop_import(void); %token <str> aIDENTIFIER aPRAGMA %token <str> aKNOWNTYPE -%token <num> aNUM aHEXNUM +%token <integer> aNUM aHEXNUM %token <dbl> aDOUBLE %token <str> aSTRING aWSTRING aSQSTRING %token <str> tCDECL @@ -494,8 +495,8 @@ pragma_warning: tPRAGMA_WARNING '(' aIDENTIFIER ':' warnings ')' ; warnings: - aNUM { $$ = append_warning(NULL, $1); } - | warnings aNUM { $$ = append_warning($1, $2); } + aNUM { $$ = append_warning(NULL, $1.value); } + | warnings aNUM { $$ = append_warning($1, $2.value); } ; typedecl: @@ -591,8 +592,8 @@ marshaling_behavior: ; contract_ver: - aNUM { $$ = MAKEVERSION(0, $1); } - | aNUM '.' aNUM { $$ = MAKEVERSION($3, $1); } + aNUM { $$ = MAKEVERSION(0, $1.value); } + | aNUM '.' aNUM { $$ = MAKEVERSION($3.value, $1.value); } ; contract_req @@ -844,10 +845,8 @@ m_expr | expr ; -expr: aNUM { struct integer integer = {.value = $1}; - $$ = make_exprl(EXPR_NUM, &integer); } - | aHEXNUM { struct integer integer = {.value = $1, .is_hex = TRUE}; - $$ = make_exprl(EXPR_NUM, &integer); } +expr: aNUM { $$ = make_exprl(EXPR_NUM, &$1); } + | aHEXNUM { $$ = make_exprl(EXPR_NUM, &$1); } | aDOUBLE { $$ = make_exprd(EXPR_DOUBLE, $1); } | tFALSE { struct integer integer = {.value = 0}; $$ = make_exprl(EXPR_TRUEFALSE, &integer); } @@ -1385,9 +1384,9 @@ uniondef: tUNION m_typename '{' ne_union_fields '}' ; version: - aNUM { $$ = MAKEVERSION($1, 0); } - | aNUM '.' aNUM { $$ = MAKEVERSION($1, $3); } - | aHEXNUM { $$ = $1; } + aNUM { $$ = MAKEVERSION($1.value, 0); } + | aNUM '.' aNUM { $$ = MAKEVERSION($1.value, $3.value); } + | aHEXNUM { $$ = $1.value; } ; acf_statements -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5906
From: Elizabeth Figura <zfigura(a)codeweavers.com> --- tools/widl/expr.c | 6 +++++- tools/widl/parser.l | 2 ++ tools/widl/widltypes.h | 2 ++ 3 files changed, 9 insertions(+), 1 deletion(-) diff --git a/tools/widl/expr.c b/tools/widl/expr.c index e51ed6c94c4..be818de2e83 100644 --- a/tools/widl/expr.c +++ b/tools/widl/expr.c @@ -512,7 +512,7 @@ static struct expression_type resolve_expression(const struct expr_loc *expr_loc case EXPR_NUM: case EXPR_TRUEFALSE: result.is_temporary = FALSE; - result.type = type_new_int(TYPE_BASIC_INT, 0); + result.type = type_new_int(e->u.integer.is_long ? TYPE_BASIC_LONG : TYPE_BASIC_INT, e->u.integer.is_unsigned); break; case EXPR_STRLIT: result.is_temporary = TRUE; @@ -690,6 +690,10 @@ void write_expr(FILE *h, const expr_t *e, int brackets, fprintf(h, "0x%x", e->u.integer.value); else fprintf(h, "%u", e->u.integer.value); + if (e->u.integer.is_unsigned) + fprintf(h, "u"); + if (e->u.integer.is_long) + fprintf(h, "l"); break; case EXPR_DOUBLE: fprintf(h, "%#.15g", e->u.dval); diff --git a/tools/widl/parser.l b/tools/widl/parser.l index 1e7c543e8af..87a9fc55f07 100644 --- a/tools/widl/parser.l +++ b/tools/widl/parser.l @@ -158,6 +158,8 @@ static int token_num( const char *yytext, YYSTYPE *yylval, int is_hex ) { yylval->integer.value = xstrtoul( yytext, NULL, 0 ); yylval->integer.is_hex = is_hex; + yylval->integer.is_long = !!strchr(yytext, 'l'); + yylval->integer.is_unsigned = !!strchr(yytext, 'u'); return is_hex ? aHEXNUM : aNUM; } diff --git a/tools/widl/widltypes.h b/tools/widl/widltypes.h index 44559e9f2ef..5ee1323793d 100644 --- a/tools/widl/widltypes.h +++ b/tools/widl/widltypes.h @@ -351,6 +351,8 @@ struct _attr_t { struct integer { int value; + int is_unsigned; + int is_long; int is_hex; }; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5906
Perhaps, but it still looks confusing at first, and so IMO not a good idea. I don't see what benefit it has over a local variable,
Conciseness, mostly.
or, lets say, separate parameters
The main reason I didn't do this was 3/4 and 4/4. Anyway, I've replaced it with a local variable. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5906#note_74246
This merge request was approved by Rémi Bernon. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5906
This merge request was approved by Huw Davies. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5906
participants (4)
-
Elizabeth Figura -
Elizabeth Figura (＠zfigura)
-
Huw Davies (＠huw)
-
Rémi Bernon