* [RFA] C++-ify parser_state @ 2017-11-26 17:40 Tom Tromey 2017-11-27 14:17 ` Sergio Durigan Junior 2017-11-27 16:41 ` Simon Marchi 0 siblings, 2 replies; 9+ messages in thread From: Tom Tromey @ 2017-11-26 17:40 UTC (permalink / raw) To: gdb-patches; +Cc: Tom Tromey This mildly C++-ifies parser_state and stap_parse_info -- just enough to remove some cleanups. Regression tested by the buildbot. ChangeLog 2017-11-26 Tom Tromey <tom@tromey.com> * stap-probe.h (struct stap_parse_info): Add constructor, destructor. * stap-probe.c (stap_parse_argument): Update. * rust-exp.y (rust_lex_tests): Update. * parser-defs.h (struct parser_state): Add constructor, destructor, release method. (initialize_expout, reallocate_expout): Remove. * parse.c (parser_state::parser_state): Rename from initialize_expout. (parser_state::~parser_state): New. (parser_state::release): Rename from reallocate_expout. (parse_exp_in_context_1): Update. * dtrace-probe.c (dtrace_probe::build_arg_exprs): Update. --- gdb/ChangeLog | 16 ++++++++++++++ gdb/dtrace-probe.c | 11 ++-------- gdb/parse.c | 63 ++++++++++++++++++++++++++++++------------------------ gdb/parser-defs.h | 32 +++++++++++++-------------- gdb/rust-exp.y | 3 +-- gdb/stap-probe.c | 15 ++----------- gdb/stap-probe.h | 17 +++++++++++++++ 7 files changed, 88 insertions(+), 69 deletions(-) diff --git a/gdb/dtrace-probe.c b/gdb/dtrace-probe.c index 1c88f89054..c0134dcac5 100644 --- a/gdb/dtrace-probe.c +++ b/gdb/dtrace-probe.c @@ -626,21 +626,15 @@ dtrace_probe::build_arg_exprs (struct gdbarch *gdbarch) value of the argument when executed at the PC of the probe. */ for (dtrace_probe_arg &arg : m_args) { - struct cleanup *back_to; - struct parser_state pstate; - /* Initialize the expression buffer in the parser state. The language does not matter, since we are using our own parser. */ - initialize_expout (&pstate, 10, current_language, gdbarch); - back_to = make_cleanup (free_current_contents, &pstate.expout); + parser_state pstate (10, current_language, gdbarch); /* The argument value, which is ABI dependent and casted to `long int'. */ gdbarch_dtrace_parse_probe_argument (gdbarch, &pstate, argc); - discard_cleanups (back_to); - /* Casting to the expected type, but only if the type was recognized at probe load time. Otherwise the argument will be evaluated as the long integer passed to the probe. */ @@ -651,8 +645,7 @@ dtrace_probe::build_arg_exprs (struct gdbarch *gdbarch) write_exp_elt_opcode (&pstate, UNOP_CAST); } - reallocate_expout (&pstate); - arg.expr = expression_up (pstate.expout); + arg.expr = pstate.release (); prefixify_expression (arg.expr.get ()); ++argc; } diff --git a/gdb/parse.c b/gdb/parse.c index 506efa38fb..3a7b7af4e1 100644 --- a/gdb/parse.c +++ b/gdb/parse.c @@ -152,34 +152,43 @@ end_arglist (void) /* See definition in parser-defs.h. */ -void -initialize_expout (struct parser_state *ps, size_t initial_size, - const struct language_defn *lang, - struct gdbarch *gdbarch) +parser_state::parser_state (size_t initial_size, + const struct language_defn *lang, + struct gdbarch *gdbarch) + : expout_size (initial_size), + expout_ptr (0) { - ps->expout_size = initial_size; - ps->expout_ptr = 0; - ps->expout + expout = (struct expression *) xmalloc (sizeof (struct expression) - + EXP_ELEM_TO_BYTES (ps->expout_size)); - ps->expout->language_defn = lang; - ps->expout->gdbarch = gdbarch; + + EXP_ELEM_TO_BYTES (expout_size)); + expout->language_defn = lang; + expout->gdbarch = gdbarch; } /* See definition in parser-defs.h. */ -void -reallocate_expout (struct parser_state *ps) +parser_state::~parser_state () +{ + xfree (expout); +} + +expression_up +parser_state::release () { /* Record the actual number of expression elements, and then reallocate the expression memory so that we free up any excess elements. */ - ps->expout->nelts = ps->expout_ptr; - ps->expout = (struct expression *) - xrealloc (ps->expout, + expout->nelts = expout_ptr; + expout = (struct expression *) + xrealloc (expout, sizeof (struct expression) - + EXP_ELEM_TO_BYTES (ps->expout_ptr)); + + EXP_ELEM_TO_BYTES (expout_ptr)); + + expression_up result (expout); + /* Ensure that we don't free it in the destructor. */ + expout = nullptr; + return result; } /* This page contains the functions for adding data to the struct expression @@ -1118,7 +1127,6 @@ parse_exp_in_context_1 (const char **stringptr, CORE_ADDR pc, int comma, int void_context_p, int *out_subexp) { const struct language_defn *lang = NULL; - struct parser_state ps; int subexp; lexptr = *stringptr; @@ -1194,7 +1202,7 @@ parse_exp_in_context_1 (const char **stringptr, CORE_ADDR pc, and others called from *.y) ensure CURRENT_LANGUAGE gets restored to the value matching SELECTED_FRAME as set by get_current_arch. */ - initialize_expout (&ps, 10, lang, get_current_arch ()); + parser_state ps (10, lang, get_current_arch ()); scoped_restore_current_language lang_saver; set_language (lang->la_language); @@ -1207,33 +1215,32 @@ parse_exp_in_context_1 (const char **stringptr, CORE_ADDR pc, CATCH (except, RETURN_MASK_ALL) { if (! parse_completion) - { - xfree (ps.expout); - throw_exception (except); - } + throw_exception (except); } END_CATCH - reallocate_expout (&ps); + /* We have to operate on an "expression *", due to la_post_parser, + which explains this funny-looking double release. */ + struct expression *result = ps.release ().release (); /* Convert expression from postfix form as generated by yacc parser, to a prefix form. */ if (expressiondebug) - dump_raw_expression (ps.expout, gdb_stdlog, + dump_raw_expression (result, gdb_stdlog, "before conversion to prefix form"); - subexp = prefixify_expression (ps.expout); + subexp = prefixify_expression (result); if (out_subexp) *out_subexp = subexp; - lang->la_post_parser (&ps.expout, void_context_p); + lang->la_post_parser (&result, void_context_p); if (expressiondebug) - dump_prefix_expression (ps.expout, gdb_stdlog); + dump_prefix_expression (result, gdb_stdlog); *stringptr = lexptr; - return expression_up (ps.expout); + return expression_up (result); } /* Parse STRING as an expression, and complain if this fails diff --git a/gdb/parser-defs.h b/gdb/parser-defs.h index f43fb75df2..262b81b056 100644 --- a/gdb/parser-defs.h +++ b/gdb/parser-defs.h @@ -37,6 +37,21 @@ extern int parser_debug; struct parser_state { + /* Constructor. INITIAL_SIZE is the initial size of the expout + array. LANG is the language used to parse the expression. And + GDBARCH is the gdbarch to use during parsing. */ + + parser_state (size_t initial_size, const struct language_defn *lang, + struct gdbarch *gdbarch); + ~parser_state (); + + DISABLE_COPY_AND_ASSIGN (parser_state); + + /* Resize the allocated expression to the correct size, and return + it as an expression_up -- passing ownership to the caller. */ + expression_up release (); + + /* The expression related to this parser state. */ struct expression *expout; @@ -156,23 +171,6 @@ struct type_stack int size; }; -/* Helper function to initialize the expout, expout_size, expout_ptr - trio inside PS before it is used to store expression elements created - during the parsing of an expression. INITIAL_SIZE is the initial size of - the expout array. LANG is the language used to parse the expression. - And GDBARCH is the gdbarch to use during parsing. */ - -extern void initialize_expout (struct parser_state *ps, - size_t initial_size, - const struct language_defn *lang, - struct gdbarch *gdbarch); - -/* Helper function that reallocates the EXPOUT inside PS in order to - eliminate any unused space. It is generally used when the expression - has just been parsed and created. */ - -extern void reallocate_expout (struct parser_state *ps); - /* Reverse an expression from suffix form (in which it is constructed) to prefix form (in which we can conveniently print or execute it). Ordinarily this always returns -1. However, if EXPOUT_LAST_STRUCT diff --git a/gdb/rust-exp.y b/gdb/rust-exp.y index e372a6ea1e..8f1455bbf0 100644 --- a/gdb/rust-exp.y +++ b/gdb/rust-exp.y @@ -2670,8 +2670,7 @@ rust_lex_tests (void) &test_obstack); // Set up dummy "parser", so that rust_type works. - struct parser_state ps; - initialize_expout (&ps, 0, &rust_language_defn, target_gdbarch ()); + struct parser_state ps (0, &rust_language_defn, target_gdbarch ()); rust_parser parser (&ps); rust_lex_test_one ("", 0); diff --git a/gdb/stap-probe.c b/gdb/stap-probe.c index 9f4e00845a..d8a67c750a 100644 --- a/gdb/stap-probe.c +++ b/gdb/stap-probe.c @@ -1146,25 +1146,17 @@ static expression_up stap_parse_argument (const char **arg, struct type *atype, struct gdbarch *gdbarch) { - struct stap_parse_info p; - struct cleanup *back_to; - /* We need to initialize the expression buffer, in order to begin our parsing efforts. We use language_c here because we may need to do pointer arithmetics. */ - initialize_expout (&p.pstate, 10, language_def (language_c), gdbarch); - back_to = make_cleanup (free_current_contents, &p.pstate.expout); + struct stap_parse_info p (10, language_def (language_c), gdbarch); p.saved_arg = *arg; p.arg = *arg; p.arg_type = atype; - p.gdbarch = gdbarch; - p.inside_paren_p = 0; stap_parse_argument_1 (&p, 0, STAP_OPERAND_PREC_NONE); - discard_cleanups (back_to); - gdb_assert (p.inside_paren_p == 0); /* Casting the final expression to the appropriate type. */ @@ -1172,13 +1164,10 @@ stap_parse_argument (const char **arg, struct type *atype, write_exp_elt_type (&p.pstate, atype); write_exp_elt_opcode (&p.pstate, UNOP_CAST); - reallocate_expout (&p.pstate); - p.arg = skip_spaces (p.arg); *arg = p.arg; - /* We can safely return EXPOUT here. */ - return expression_up (p.pstate.expout); + return p.pstate.release (); } /* Implementation of 'parse_arguments' method. */ diff --git a/gdb/stap-probe.h b/gdb/stap-probe.h index c3327e6999..af24a380e0 100644 --- a/gdb/stap-probe.h +++ b/gdb/stap-probe.h @@ -28,6 +28,23 @@ struct stap_parse_info { + /* Constructor. Arguments are passed to the parser_state + constructor. */ + stap_parse_info (size_t initial_size, const struct language_defn *lang, + struct gdbarch *gdbarch) + : arg (nullptr), + pstate (initial_size, lang, gdbarch), + saved_arg (nullptr), + arg_type (nullptr), + gdbarch (gdbarch), + inside_paren_p (0) + { + } + + ~stap_parse_info () = default; + + DISABLE_COPY_AND_ASSIGN (stap_parse_info); + /* The probe's argument in a string format. */ const char *arg; -- 2.13.6 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [RFA] C++-ify parser_state 2017-11-26 17:40 [RFA] C++-ify parser_state Tom Tromey @ 2017-11-27 14:17 ` Sergio Durigan Junior 2017-11-27 16:41 ` Simon Marchi 1 sibling, 0 replies; 9+ messages in thread From: Sergio Durigan Junior @ 2017-11-27 14:17 UTC (permalink / raw) To: Tom Tromey; +Cc: gdb-patches On Sunday, November 26 2017, Tom Tromey wrote: > This mildly C++-ifies parser_state and stap_parse_info -- just enough > to remove some cleanups. Thanks for the patch, Tom. This was on my TODO list for a while now. I looked it over and it seems good to me. > Regression tested by the buildbot. > > ChangeLog > 2017-11-26 Tom Tromey <tom@tromey.com> > > * stap-probe.h (struct stap_parse_info): Add constructor, > destructor. > * stap-probe.c (stap_parse_argument): Update. > * rust-exp.y (rust_lex_tests): Update. > * parser-defs.h (struct parser_state): Add constructor, > destructor, release method. > (initialize_expout, reallocate_expout): Remove. > * parse.c (parser_state::parser_state): Rename from > initialize_expout. > (parser_state::~parser_state): New. > (parser_state::release): Rename from reallocate_expout. > (parse_exp_in_context_1): Update. > * dtrace-probe.c (dtrace_probe::build_arg_exprs): Update. > --- > gdb/ChangeLog | 16 ++++++++++++++ > gdb/dtrace-probe.c | 11 ++-------- > gdb/parse.c | 63 ++++++++++++++++++++++++++++++------------------------ > gdb/parser-defs.h | 32 +++++++++++++-------------- > gdb/rust-exp.y | 3 +-- > gdb/stap-probe.c | 15 ++----------- > gdb/stap-probe.h | 17 +++++++++++++++ > 7 files changed, 88 insertions(+), 69 deletions(-) > > diff --git a/gdb/dtrace-probe.c b/gdb/dtrace-probe.c > index 1c88f89054..c0134dcac5 100644 > --- a/gdb/dtrace-probe.c > +++ b/gdb/dtrace-probe.c > @@ -626,21 +626,15 @@ dtrace_probe::build_arg_exprs (struct gdbarch *gdbarch) > value of the argument when executed at the PC of the probe. */ > for (dtrace_probe_arg &arg : m_args) > { > - struct cleanup *back_to; > - struct parser_state pstate; > - > /* Initialize the expression buffer in the parser state. The > language does not matter, since we are using our own > parser. */ > - initialize_expout (&pstate, 10, current_language, gdbarch); > - back_to = make_cleanup (free_current_contents, &pstate.expout); > + parser_state pstate (10, current_language, gdbarch); > > /* The argument value, which is ABI dependent and casted to > `long int'. */ > gdbarch_dtrace_parse_probe_argument (gdbarch, &pstate, argc); > > - discard_cleanups (back_to); > - > /* Casting to the expected type, but only if the type was > recognized at probe load time. Otherwise the argument will > be evaluated as the long integer passed to the probe. */ > @@ -651,8 +645,7 @@ dtrace_probe::build_arg_exprs (struct gdbarch *gdbarch) > write_exp_elt_opcode (&pstate, UNOP_CAST); > } > > - reallocate_expout (&pstate); > - arg.expr = expression_up (pstate.expout); > + arg.expr = pstate.release (); > prefixify_expression (arg.expr.get ()); > ++argc; > } > diff --git a/gdb/parse.c b/gdb/parse.c > index 506efa38fb..3a7b7af4e1 100644 > --- a/gdb/parse.c > +++ b/gdb/parse.c > @@ -152,34 +152,43 @@ end_arglist (void) > > /* See definition in parser-defs.h. */ > > -void > -initialize_expout (struct parser_state *ps, size_t initial_size, > - const struct language_defn *lang, > - struct gdbarch *gdbarch) > +parser_state::parser_state (size_t initial_size, > + const struct language_defn *lang, > + struct gdbarch *gdbarch) > + : expout_size (initial_size), > + expout_ptr (0) > { > - ps->expout_size = initial_size; > - ps->expout_ptr = 0; > - ps->expout > + expout > = (struct expression *) xmalloc (sizeof (struct expression) > - + EXP_ELEM_TO_BYTES (ps->expout_size)); > - ps->expout->language_defn = lang; > - ps->expout->gdbarch = gdbarch; > + + EXP_ELEM_TO_BYTES (expout_size)); > + expout->language_defn = lang; > + expout->gdbarch = gdbarch; > } > > /* See definition in parser-defs.h. */ > > -void > -reallocate_expout (struct parser_state *ps) > +parser_state::~parser_state () > +{ > + xfree (expout); > +} > + > +expression_up > +parser_state::release () > { > /* Record the actual number of expression elements, and then > reallocate the expression memory so that we free up any > excess elements. */ > > - ps->expout->nelts = ps->expout_ptr; > - ps->expout = (struct expression *) > - xrealloc (ps->expout, > + expout->nelts = expout_ptr; > + expout = (struct expression *) > + xrealloc (expout, > sizeof (struct expression) > - + EXP_ELEM_TO_BYTES (ps->expout_ptr)); > + + EXP_ELEM_TO_BYTES (expout_ptr)); > + > + expression_up result (expout); > + /* Ensure that we don't free it in the destructor. */ > + expout = nullptr; > + return result; > } > > /* This page contains the functions for adding data to the struct expression > @@ -1118,7 +1127,6 @@ parse_exp_in_context_1 (const char **stringptr, CORE_ADDR pc, > int comma, int void_context_p, int *out_subexp) > { > const struct language_defn *lang = NULL; > - struct parser_state ps; > int subexp; > > lexptr = *stringptr; > @@ -1194,7 +1202,7 @@ parse_exp_in_context_1 (const char **stringptr, CORE_ADDR pc, > and others called from *.y) ensure CURRENT_LANGUAGE gets restored > to the value matching SELECTED_FRAME as set by get_current_arch. */ > > - initialize_expout (&ps, 10, lang, get_current_arch ()); > + parser_state ps (10, lang, get_current_arch ()); > > scoped_restore_current_language lang_saver; > set_language (lang->la_language); > @@ -1207,33 +1215,32 @@ parse_exp_in_context_1 (const char **stringptr, CORE_ADDR pc, > CATCH (except, RETURN_MASK_ALL) > { > if (! parse_completion) > - { > - xfree (ps.expout); > - throw_exception (except); > - } > + throw_exception (except); > } > END_CATCH > > - reallocate_expout (&ps); > + /* We have to operate on an "expression *", due to la_post_parser, > + which explains this funny-looking double release. */ > + struct expression *result = ps.release ().release (); > > /* Convert expression from postfix form as generated by yacc > parser, to a prefix form. */ > > if (expressiondebug) > - dump_raw_expression (ps.expout, gdb_stdlog, > + dump_raw_expression (result, gdb_stdlog, > "before conversion to prefix form"); > > - subexp = prefixify_expression (ps.expout); > + subexp = prefixify_expression (result); > if (out_subexp) > *out_subexp = subexp; > > - lang->la_post_parser (&ps.expout, void_context_p); > + lang->la_post_parser (&result, void_context_p); > > if (expressiondebug) > - dump_prefix_expression (ps.expout, gdb_stdlog); > + dump_prefix_expression (result, gdb_stdlog); > > *stringptr = lexptr; > - return expression_up (ps.expout); > + return expression_up (result); > } > > /* Parse STRING as an expression, and complain if this fails > diff --git a/gdb/parser-defs.h b/gdb/parser-defs.h > index f43fb75df2..262b81b056 100644 > --- a/gdb/parser-defs.h > +++ b/gdb/parser-defs.h > @@ -37,6 +37,21 @@ extern int parser_debug; > > struct parser_state > { > + /* Constructor. INITIAL_SIZE is the initial size of the expout > + array. LANG is the language used to parse the expression. And > + GDBARCH is the gdbarch to use during parsing. */ > + > + parser_state (size_t initial_size, const struct language_defn *lang, > + struct gdbarch *gdbarch); > + ~parser_state (); > + > + DISABLE_COPY_AND_ASSIGN (parser_state); > + > + /* Resize the allocated expression to the correct size, and return > + it as an expression_up -- passing ownership to the caller. */ > + expression_up release (); > + > + > /* The expression related to this parser state. */ > > struct expression *expout; > @@ -156,23 +171,6 @@ struct type_stack > int size; > }; > > -/* Helper function to initialize the expout, expout_size, expout_ptr > - trio inside PS before it is used to store expression elements created > - during the parsing of an expression. INITIAL_SIZE is the initial size of > - the expout array. LANG is the language used to parse the expression. > - And GDBARCH is the gdbarch to use during parsing. */ > - > -extern void initialize_expout (struct parser_state *ps, > - size_t initial_size, > - const struct language_defn *lang, > - struct gdbarch *gdbarch); > - > -/* Helper function that reallocates the EXPOUT inside PS in order to > - eliminate any unused space. It is generally used when the expression > - has just been parsed and created. */ > - > -extern void reallocate_expout (struct parser_state *ps); > - > /* Reverse an expression from suffix form (in which it is constructed) > to prefix form (in which we can conveniently print or execute it). > Ordinarily this always returns -1. However, if EXPOUT_LAST_STRUCT > diff --git a/gdb/rust-exp.y b/gdb/rust-exp.y > index e372a6ea1e..8f1455bbf0 100644 > --- a/gdb/rust-exp.y > +++ b/gdb/rust-exp.y > @@ -2670,8 +2670,7 @@ rust_lex_tests (void) > &test_obstack); > > // Set up dummy "parser", so that rust_type works. > - struct parser_state ps; > - initialize_expout (&ps, 0, &rust_language_defn, target_gdbarch ()); > + struct parser_state ps (0, &rust_language_defn, target_gdbarch ()); > rust_parser parser (&ps); > > rust_lex_test_one ("", 0); > diff --git a/gdb/stap-probe.c b/gdb/stap-probe.c > index 9f4e00845a..d8a67c750a 100644 > --- a/gdb/stap-probe.c > +++ b/gdb/stap-probe.c > @@ -1146,25 +1146,17 @@ static expression_up > stap_parse_argument (const char **arg, struct type *atype, > struct gdbarch *gdbarch) > { > - struct stap_parse_info p; > - struct cleanup *back_to; > - > /* We need to initialize the expression buffer, in order to begin > our parsing efforts. We use language_c here because we may need > to do pointer arithmetics. */ > - initialize_expout (&p.pstate, 10, language_def (language_c), gdbarch); > - back_to = make_cleanup (free_current_contents, &p.pstate.expout); > + struct stap_parse_info p (10, language_def (language_c), gdbarch); > > p.saved_arg = *arg; > p.arg = *arg; > p.arg_type = atype; > - p.gdbarch = gdbarch; > - p.inside_paren_p = 0; > > stap_parse_argument_1 (&p, 0, STAP_OPERAND_PREC_NONE); > > - discard_cleanups (back_to); > - > gdb_assert (p.inside_paren_p == 0); > > /* Casting the final expression to the appropriate type. */ > @@ -1172,13 +1164,10 @@ stap_parse_argument (const char **arg, struct type *atype, > write_exp_elt_type (&p.pstate, atype); > write_exp_elt_opcode (&p.pstate, UNOP_CAST); > > - reallocate_expout (&p.pstate); > - > p.arg = skip_spaces (p.arg); > *arg = p.arg; > > - /* We can safely return EXPOUT here. */ > - return expression_up (p.pstate.expout); > + return p.pstate.release (); > } > > /* Implementation of 'parse_arguments' method. */ > diff --git a/gdb/stap-probe.h b/gdb/stap-probe.h > index c3327e6999..af24a380e0 100644 > --- a/gdb/stap-probe.h > +++ b/gdb/stap-probe.h > @@ -28,6 +28,23 @@ > > struct stap_parse_info > { > + /* Constructor. Arguments are passed to the parser_state > + constructor. */ > + stap_parse_info (size_t initial_size, const struct language_defn *lang, > + struct gdbarch *gdbarch) > + : arg (nullptr), > + pstate (initial_size, lang, gdbarch), > + saved_arg (nullptr), > + arg_type (nullptr), > + gdbarch (gdbarch), > + inside_paren_p (0) > + { > + } > + > + ~stap_parse_info () = default; > + > + DISABLE_COPY_AND_ASSIGN (stap_parse_info); > + > /* The probe's argument in a string format. */ > const char *arg; > > -- > 2.13.6 -- Sergio GPG key ID: 237A 54B1 0287 28BF 00EF 31F4 D0EB 7628 65FC 5E36 Please send encrypted e-mail if possible http://sergiodj.net/ ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [RFA] C++-ify parser_state 2017-11-26 17:40 [RFA] C++-ify parser_state Tom Tromey 2017-11-27 14:17 ` Sergio Durigan Junior @ 2017-11-27 16:41 ` Simon Marchi 2017-11-30 3:29 ` Tom Tromey 1 sibling, 1 reply; 9+ messages in thread From: Simon Marchi @ 2017-11-27 16:41 UTC (permalink / raw) To: Tom Tromey, gdb-patches Hi Tom, I think this looks good. I have some suggestions to clean things a further bit, you are free to take them or leave them On 2017-11-26 12:40 PM, Tom Tromey wrote: > +expression_up > +parser_state::release () > { > /* Record the actual number of expression elements, and then > reallocate the expression memory so that we free up any > excess elements. */ > > - ps->expout->nelts = ps->expout_ptr; > - ps->expout = (struct expression *) > - xrealloc (ps->expout, > + expout->nelts = expout_ptr; > + expout = (struct expression *) > + xrealloc (expout, > sizeof (struct expression) > - + EXP_ELEM_TO_BYTES (ps->expout_ptr)); > + + EXP_ELEM_TO_BYTES (expout_ptr)); > + > + expression_up result (expout); > + /* Ensure that we don't free it in the destructor. */ > + expout = nullptr; > + return result; If expout was an expression_up, we could just std::move it here, and wouldn't need an explicit destructor. > } > > /* This page contains the functions for adding data to the struct expression > @@ -1118,7 +1127,6 @@ parse_exp_in_context_1 (const char **stringptr, CORE_ADDR pc, > int comma, int void_context_p, int *out_subexp) > { > const struct language_defn *lang = NULL; > - struct parser_state ps; > int subexp; > > lexptr = *stringptr; > @@ -1194,7 +1202,7 @@ parse_exp_in_context_1 (const char **stringptr, CORE_ADDR pc, > and others called from *.y) ensure CURRENT_LANGUAGE gets restored > to the value matching SELECTED_FRAME as set by get_current_arch. */ > > - initialize_expout (&ps, 10, lang, get_current_arch ()); > + parser_state ps (10, lang, get_current_arch ()); > > scoped_restore_current_language lang_saver; > set_language (lang->la_language); > @@ -1207,33 +1215,32 @@ parse_exp_in_context_1 (const char **stringptr, CORE_ADDR pc, > CATCH (except, RETURN_MASK_ALL) > { > if (! parse_completion) > - { > - xfree (ps.expout); > - throw_exception (except); > - } > + throw_exception (except); > } > END_CATCH > > - reallocate_expout (&ps); > + /* We have to operate on an "expression *", due to la_post_parser, > + which explains this funny-looking double release. */ > + struct expression *result = ps.release ().release (); > > /* Convert expression from postfix form as generated by yacc > parser, to a prefix form. */ > > if (expressiondebug) > - dump_raw_expression (ps.expout, gdb_stdlog, > + dump_raw_expression (result, gdb_stdlog, > "before conversion to prefix form"); > > - subexp = prefixify_expression (ps.expout); > + subexp = prefixify_expression (result); > if (out_subexp) > *out_subexp = subexp; > > - lang->la_post_parser (&ps.expout, void_context_p); > + lang->la_post_parser (&result, void_context_p); Passing a pointer or reference to the unique_ptr would allow the implementations of la_post_parser to modify it directly, and avoid the .release ().release (). > --- a/gdb/stap-probe.c > +++ b/gdb/stap-probe.c > @@ -1146,25 +1146,17 @@ static expression_up > stap_parse_argument (const char **arg, struct type *atype, > struct gdbarch *gdbarch) > { > - struct stap_parse_info p; > - struct cleanup *back_to; > - > /* We need to initialize the expression buffer, in order to begin > our parsing efforts. We use language_c here because we may need > to do pointer arithmetics. */ > - initialize_expout (&p.pstate, 10, language_def (language_c), gdbarch); > - back_to = make_cleanup (free_current_contents, &p.pstate.expout); > + struct stap_parse_info p (10, language_def (language_c), gdbarch); > > p.saved_arg = *arg; > p.arg = *arg; > p.arg_type = atype; > - p.gdbarch = gdbarch; > - p.inside_paren_p = 0; Why not pass the other arguments to the constructor (*arg and atype)? Simon ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [RFA] C++-ify parser_state 2017-11-27 16:41 ` Simon Marchi @ 2017-11-30 3:29 ` Tom Tromey 2017-11-30 3:54 ` Simon Marchi 0 siblings, 1 reply; 9+ messages in thread From: Tom Tromey @ 2017-11-30 3:29 UTC (permalink / raw) To: Simon Marchi; +Cc: Tom Tromey, gdb-patches >>>>> "Simon" == Simon Marchi <simon.marchi@ericsson.com> writes: >> + /* Ensure that we don't free it in the destructor. */ >> + expout = nullptr; >> + return result; Simon> If expout was an expression_up, we could just std::move it here, and Simon> wouldn't need an explicit destructor. I thought that looked somewhat difficult due to the use of xrealloc when growing the expression. Really, of course, the whole expression structure needs to be redone. That's a big task though. >> - lang->la_post_parser (&ps.expout, void_context_p); >> + lang->la_post_parser (&result, void_context_p); Simon> Passing a pointer or reference to the unique_ptr would allow Simon> the implementations of la_post_parser to modify it directly, Simon> and avoid the .release ().release (). I'll look into it. The realloc thing may be an issue here as well. >> p.saved_arg = *arg; >> p.arg = *arg; >> p.arg_type = atype; >> - p.gdbarch = gdbarch; >> - p.inside_paren_p = 0; Simon> Why not pass the other arguments to the constructor (*arg and atype)? Yeah, this one I'll do for sure. Tom ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [RFA] C++-ify parser_state 2017-11-30 3:29 ` Tom Tromey @ 2017-11-30 3:54 ` Simon Marchi 2017-11-30 5:49 ` Simon Marchi 0 siblings, 1 reply; 9+ messages in thread From: Simon Marchi @ 2017-11-30 3:54 UTC (permalink / raw) To: Tom Tromey; +Cc: Simon Marchi, gdb-patches On 2017-11-29 22:29, Tom Tromey wrote: >>>>>> "Simon" == Simon Marchi <simon.marchi@ericsson.com> writes: > >>> + /* Ensure that we don't free it in the destructor. */ >>> + expout = nullptr; >>> + return result; > > Simon> If expout was an expression_up, we could just std::move it here, > and > Simon> wouldn't need an explicit destructor. > > I thought that looked somewhat difficult due to the use of xrealloc > when > growing the expression. > > Really, of course, the whole expression structure needs to be redone. > That's a big task though. Agreed. >>> - lang->la_post_parser (&ps.expout, void_context_p); >>> + lang->la_post_parser (&result, void_context_p); > > Simon> Passing a pointer or reference to the unique_ptr would allow > Simon> the implementations of la_post_parser to modify it directly, > Simon> and avoid the .release ().release (). > > I'll look into it. > The realloc thing may be an issue here as well. I think this should be enough (the commit on top): https://github.com/simark/binutils-gdb/commits/parser_state Simon ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [RFA] C++-ify parser_state 2017-11-30 3:54 ` Simon Marchi @ 2017-11-30 5:49 ` Simon Marchi 2017-12-09 4:41 ` Tom Tromey 0 siblings, 1 reply; 9+ messages in thread From: Simon Marchi @ 2017-11-30 5:49 UTC (permalink / raw) To: Tom Tromey; +Cc: Simon Marchi, gdb-patches On 2017-11-29 22:54, Simon Marchi wrote: > On 2017-11-29 22:29, Tom Tromey wrote: >> Simon> Passing a pointer or reference to the unique_ptr would allow >> Simon> the implementations of la_post_parser to modify it directly, >> Simon> and avoid the .release ().release (). >> >> I'll look into it. >> The realloc thing may be an issue here as well. > > I think this should be enough (the commit on top): > > https://github.com/simark/binutils-gdb/commits/parser_state > > Simon Oops, I realized I only answered half the question. I added a second patch on top. Simon ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [RFA] C++-ify parser_state 2017-11-30 5:49 ` Simon Marchi @ 2017-12-09 4:41 ` Tom Tromey 2017-12-10 21:23 ` Simon Marchi 0 siblings, 1 reply; 9+ messages in thread From: Tom Tromey @ 2017-12-09 4:41 UTC (permalink / raw) To: Simon Marchi; +Cc: Tom Tromey, Simon Marchi, gdb-patches >>>>> "Simon" == Simon Marchi <simon.marchi@polymtl.ca> writes: Simon> Oops, I realized I only answered half the question. I added a second Simon> patch on top. No problem. How's this? Tom commit ee14e980825b5eabd6753a52c75b73dda0762bf2 Author: Tom Tromey <tom@tromey.com> Date: Wed Nov 22 21:45:53 2017 -0700 C++-ify parser_state This mildly C++-ifies parser_state and stap_parse_info -- just enough to remove some cleanups. This version includes the changes implemented by Simon. Regression tested by the buildbot. ChangeLog 2017-11-26 Tom Tromey <tom@tromey.com> Simon Marchi <simon.marchi@ericsson.com> * stap-probe.h (struct stap_parse_info): Add constructor, destructor. * stap-probe.c (stap_parse_argument): Update. * rust-exp.y (rust_lex_tests): Update. * parser-defs.h (struct parser_state): Add constructor, destructor, release method. (null_post_parser): Change type. <expout>: Change type to expression_up. (initialize_expout, reallocate_expout): Remove. * parse.c (parser_state::parser_state): Rename from initialize_expout. (parser_state::release): Rename from reallocate_expout. (parse_exp_in_context_1): Update. (null_post_parser): Change type of "exp". * dtrace-probe.c (dtrace_probe::build_arg_exprs): Update. * ada-lang.c (resolve, resolve_subexp) (replace_operator_with_call): Change type of "expp". * language.h (struct language_defn) <la_post_parser>: Change type of "expp". diff --git a/gdb/ChangeLog b/gdb/ChangeLog index 1d381135e7..edfcd98dbe 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,26 @@ +2017-11-26 Tom Tromey <tom@tromey.com> + Simon Marchi <simon.marchi@ericsson.com> + + * stap-probe.h (struct stap_parse_info): Add constructor, + destructor. + * stap-probe.c (stap_parse_argument): Update. + * rust-exp.y (rust_lex_tests): Update. + * parser-defs.h (struct parser_state): Add constructor, + destructor, release method. + (null_post_parser): Change type. + <expout>: Change type to expression_up. + (initialize_expout, reallocate_expout): Remove. + * parse.c (parser_state::parser_state): Rename from + initialize_expout. + (parser_state::release): Rename from reallocate_expout. + (parse_exp_in_context_1): Update. + (null_post_parser): Change type of "exp". + * dtrace-probe.c (dtrace_probe::build_arg_exprs): Update. + * ada-lang.c (resolve, resolve_subexp) + (replace_operator_with_call): Change type of "expp". + * language.h (struct language_defn) <la_post_parser>: Change type + of "expp". + 2017-12-08 Pedro Alves <palves@redhat.com> * dwarf2read.c (mock_mapped_index): Reimplement as an extension of diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c index d6f7ef4ce8..9bb047626d 100644 --- a/gdb/ada-lang.c +++ b/gdb/ada-lang.c @@ -124,10 +124,10 @@ static int num_defns_collected (struct obstack *); static struct block_symbol *defns_collected (struct obstack *, int); -static struct value *resolve_subexp (struct expression **, int *, int, +static struct value *resolve_subexp (expression_up *, int *, int, struct type *); -static void replace_operator_with_call (struct expression **, int, int, int, +static void replace_operator_with_call (expression_up *, int, int, int, struct symbol *, const struct block *); static int possible_user_operator_p (enum exp_opcode, struct value **); @@ -3235,7 +3235,7 @@ ada_decoded_op_name (enum exp_opcode op) return type is preferred. May change (expand) *EXP. */ static void -resolve (struct expression **expp, int void_context_p) +resolve (expression_up *expp, int void_context_p) { struct type *context_type = NULL; int pc = 0; @@ -3256,7 +3256,7 @@ resolve (struct expression **expp, int void_context_p) are as in ada_resolve, above. */ static struct value * -resolve_subexp (struct expression **expp, int *pos, int deprocedure_p, +resolve_subexp (expression_up *expp, int *pos, int deprocedure_p, struct type *context_type) { int pc = *pos; @@ -3270,7 +3270,7 @@ resolve_subexp (struct expression **expp, int *pos, int deprocedure_p, argvec = NULL; nargs = 0; - exp = *expp; + exp = expp->get (); /* Pass one: resolve operands, saving their types and updating *pos, if needed. */ @@ -3420,7 +3420,7 @@ resolve_subexp (struct expression **expp, int *pos, int deprocedure_p, for (i = 0; i < nargs; i += 1) argvec[i] = resolve_subexp (expp, pos, 1, NULL); argvec[i] = NULL; - exp = *expp; + exp = expp->get (); /* Pass two: perform any resolution on principal operator. */ switch (op) @@ -3515,7 +3515,7 @@ resolve_subexp (struct expression **expp, int *pos, int deprocedure_p, replace_operator_with_call (expp, pc, 0, 0, exp->elts[pc + 2].symbol, exp->elts[pc + 1].block); - exp = *expp; + exp = expp->get (); } break; @@ -3596,7 +3596,7 @@ resolve_subexp (struct expression **expp, int *pos, int deprocedure_p, replace_operator_with_call (expp, pc, nargs, 1, candidates[i].symbol, candidates[i].block); - exp = *expp; + exp = expp->get (); } break; @@ -4115,7 +4115,7 @@ get_selections (int *choices, int n_choices, int max_results, arguments. Update *EXPP as needed to hold more space. */ static void -replace_operator_with_call (struct expression **expp, int pc, int nargs, +replace_operator_with_call (expression_up *expp, int pc, int nargs, int oplen, struct symbol *sym, const struct block *block) { @@ -4124,7 +4124,7 @@ replace_operator_with_call (struct expression **expp, int pc, int nargs, struct expression *newexp = (struct expression *) xzalloc (sizeof (struct expression) + EXP_ELEM_TO_BYTES ((*expp)->nelts + 7 - oplen)); - struct expression *exp = *expp; + struct expression *exp = expp->get (); newexp->nelts = exp->nelts + 7 - oplen; newexp->language_defn = exp->language_defn; @@ -4140,8 +4140,7 @@ replace_operator_with_call (struct expression **expp, int pc, int nargs, newexp->elts[pc + 4].block = block; newexp->elts[pc + 5].symbol = sym; - *expp = newexp; - xfree (exp); + expp->reset (newexp); } /* Type-class predicates */ diff --git a/gdb/dtrace-probe.c b/gdb/dtrace-probe.c index 3314445f98..40553d3245 100644 --- a/gdb/dtrace-probe.c +++ b/gdb/dtrace-probe.c @@ -625,21 +625,15 @@ dtrace_probe::build_arg_exprs (struct gdbarch *gdbarch) value of the argument when executed at the PC of the probe. */ for (dtrace_probe_arg &arg : m_args) { - struct cleanup *back_to; - struct parser_state pstate; - /* Initialize the expression buffer in the parser state. The language does not matter, since we are using our own parser. */ - initialize_expout (&pstate, 10, current_language, gdbarch); - back_to = make_cleanup (free_current_contents, &pstate.expout); + parser_state pstate (10, current_language, gdbarch); /* The argument value, which is ABI dependent and casted to `long int'. */ gdbarch_dtrace_parse_probe_argument (gdbarch, &pstate, argc); - discard_cleanups (back_to); - /* Casting to the expected type, but only if the type was recognized at probe load time. Otherwise the argument will be evaluated as the long integer passed to the probe. */ @@ -650,8 +644,7 @@ dtrace_probe::build_arg_exprs (struct gdbarch *gdbarch) write_exp_elt_opcode (&pstate, UNOP_CAST); } - reallocate_expout (&pstate); - arg.expr = expression_up (pstate.expout); + arg.expr = pstate.release (); prefixify_expression (arg.expr.get ()); ++argc; } diff --git a/gdb/language.h b/gdb/language.h index 47ad8da05d..7e300dadcb 100644 --- a/gdb/language.h +++ b/gdb/language.h @@ -25,12 +25,12 @@ #include "symtab.h" #include "common/function-view.h" +#include "expression.h" /* Forward decls for prototypes. */ struct value; struct objfile; struct frame_info; -struct expression; struct ui_file; struct value_print_options; struct type_print_options; @@ -182,7 +182,7 @@ struct language_defn for releasing its previous contents, if necessary. If VOID_CONTEXT_P, then no value is expected from the expression. */ - void (*la_post_parser) (struct expression ** expp, int void_context_p); + void (*la_post_parser) (expression_up *expp, int void_context_p); void (*la_printchar) (int ch, struct type *chtype, struct ui_file * stream); diff --git a/gdb/parse.c b/gdb/parse.c index dff519ba63..127ce7ff29 100644 --- a/gdb/parse.c +++ b/gdb/parse.c @@ -152,34 +152,32 @@ end_arglist (void) /* See definition in parser-defs.h. */ -void -initialize_expout (struct parser_state *ps, size_t initial_size, - const struct language_defn *lang, - struct gdbarch *gdbarch) +parser_state::parser_state (size_t initial_size, + const struct language_defn *lang, + struct gdbarch *gdbarch) + : expout_size (initial_size), + expout (XNEWVAR (expression, + (sizeof (expression) + + EXP_ELEM_TO_BYTES (expout_size)))), + expout_ptr (0) { - ps->expout_size = initial_size; - ps->expout_ptr = 0; - ps->expout - = (struct expression *) xmalloc (sizeof (struct expression) - + EXP_ELEM_TO_BYTES (ps->expout_size)); - ps->expout->language_defn = lang; - ps->expout->gdbarch = gdbarch; + expout->language_defn = lang; + expout->gdbarch = gdbarch; } -/* See definition in parser-defs.h. */ - -void -reallocate_expout (struct parser_state *ps) +expression_up +parser_state::release () { /* Record the actual number of expression elements, and then reallocate the expression memory so that we free up any excess elements. */ - ps->expout->nelts = ps->expout_ptr; - ps->expout = (struct expression *) - xrealloc (ps->expout, - sizeof (struct expression) - + EXP_ELEM_TO_BYTES (ps->expout_ptr)); + expout->nelts = expout_ptr; + expout.reset (XRESIZEVAR (expression, expout.release (), + (sizeof (expression) + + EXP_ELEM_TO_BYTES (expout_ptr)))); + + return std::move (expout); } /* This page contains the functions for adding data to the struct expression @@ -196,9 +194,9 @@ write_exp_elt (struct parser_state *ps, const union exp_element *expelt) if (ps->expout_ptr >= ps->expout_size) { ps->expout_size *= 2; - ps->expout = (struct expression *) - xrealloc (ps->expout, sizeof (struct expression) - + EXP_ELEM_TO_BYTES (ps->expout_size)); + ps->expout.reset (XRESIZEVAR (expression, ps->expout.release (), + (sizeof (expression) + + EXP_ELEM_TO_BYTES (ps->expout_size)))); } ps->expout->elts[ps->expout_ptr++] = *expelt; } @@ -1116,7 +1114,6 @@ parse_exp_in_context_1 (const char **stringptr, CORE_ADDR pc, int comma, int void_context_p, int *out_subexp) { const struct language_defn *lang = NULL; - struct parser_state ps; int subexp; lexptr = *stringptr; @@ -1192,7 +1189,7 @@ parse_exp_in_context_1 (const char **stringptr, CORE_ADDR pc, and others called from *.y) ensure CURRENT_LANGUAGE gets restored to the value matching SELECTED_FRAME as set by get_current_arch. */ - initialize_expout (&ps, 10, lang, get_current_arch ()); + parser_state ps (10, lang, get_current_arch ()); scoped_restore_current_language lang_saver; set_language (lang->la_language); @@ -1205,33 +1202,32 @@ parse_exp_in_context_1 (const char **stringptr, CORE_ADDR pc, CATCH (except, RETURN_MASK_ALL) { if (! parse_completion) - { - xfree (ps.expout); - throw_exception (except); - } + throw_exception (except); } END_CATCH - reallocate_expout (&ps); + /* We have to operate on an "expression *", due to la_post_parser, + which explains this funny-looking double release. */ + expression_up result = ps.release (); /* Convert expression from postfix form as generated by yacc parser, to a prefix form. */ if (expressiondebug) - dump_raw_expression (ps.expout, gdb_stdlog, + dump_raw_expression (result.get (), gdb_stdlog, "before conversion to prefix form"); - subexp = prefixify_expression (ps.expout); + subexp = prefixify_expression (result.get ()); if (out_subexp) *out_subexp = subexp; - lang->la_post_parser (&ps.expout, void_context_p); + lang->la_post_parser (&result, void_context_p); if (expressiondebug) - dump_prefix_expression (ps.expout, gdb_stdlog); + dump_prefix_expression (result.get (), gdb_stdlog); *stringptr = lexptr; - return expression_up (ps.expout); + return result; } /* Parse STRING as an expression, and complain if this fails @@ -1320,7 +1316,7 @@ parse_expression_for_completion (const char *string, char **name, /* A post-parser that does nothing. */ void -null_post_parser (struct expression **exp, int void_context_p) +null_post_parser (expression_up *exp, int void_context_p) { } @@ -1866,10 +1862,11 @@ increase_expout_size (struct parser_state *ps, size_t lenelt) if ((ps->expout_ptr + lenelt) >= ps->expout_size) { ps->expout_size = std::max (ps->expout_size * 2, - ps->expout_ptr + lenelt + 10); - ps->expout = (struct expression *) - xrealloc (ps->expout, (sizeof (struct expression) - + EXP_ELEM_TO_BYTES (ps->expout_size))); + ps->expout_ptr + lenelt + 10); + ps->expout.reset (XRESIZEVAR (expression, + ps->expout.release (), + (sizeof (struct expression) + + EXP_ELEM_TO_BYTES (ps->expout_size)))); } } diff --git a/gdb/parser-defs.h b/gdb/parser-defs.h index f43fb75df2..907a79811a 100644 --- a/gdb/parser-defs.h +++ b/gdb/parser-defs.h @@ -37,14 +37,27 @@ extern int parser_debug; struct parser_state { - /* The expression related to this parser state. */ + /* Constructor. INITIAL_SIZE is the initial size of the expout + array. LANG is the language used to parse the expression. And + GDBARCH is the gdbarch to use during parsing. */ + + parser_state (size_t initial_size, const struct language_defn *lang, + struct gdbarch *gdbarch); + + DISABLE_COPY_AND_ASSIGN (parser_state); - struct expression *expout; + /* Resize the allocated expression to the correct size, and return + it as an expression_up -- passing ownership to the caller. */ + expression_up release (); /* The size of the expression above. */ size_t expout_size; + /* The expression related to this parser state. */ + + expression_up expout; + /* The number of elements already in the expression. This is used to know where to put new elements. */ @@ -156,23 +169,6 @@ struct type_stack int size; }; -/* Helper function to initialize the expout, expout_size, expout_ptr - trio inside PS before it is used to store expression elements created - during the parsing of an expression. INITIAL_SIZE is the initial size of - the expout array. LANG is the language used to parse the expression. - And GDBARCH is the gdbarch to use during parsing. */ - -extern void initialize_expout (struct parser_state *ps, - size_t initial_size, - const struct language_defn *lang, - struct gdbarch *gdbarch); - -/* Helper function that reallocates the EXPOUT inside PS in order to - eliminate any unused space. It is generally used when the expression - has just been parsed and created. */ - -extern void reallocate_expout (struct parser_state *ps); - /* Reverse an expression from suffix form (in which it is constructed) to prefix form (in which we can conveniently print or execute it). Ordinarily this always returns -1. However, if EXPOUT_LAST_STRUCT @@ -265,7 +261,7 @@ extern struct type *follow_types (struct type *); extern type_instance_flags follow_type_instance_flags (); -extern void null_post_parser (struct expression **, int); +extern void null_post_parser (expression_up *, int); extern bool parse_float (const char *p, int len, const struct type *type, gdb_byte *data); diff --git a/gdb/rust-exp.y b/gdb/rust-exp.y index 731a0391a6..9f4ec9dfd2 100644 --- a/gdb/rust-exp.y +++ b/gdb/rust-exp.y @@ -2666,8 +2666,7 @@ rust_lex_tests (void) &test_obstack); // Set up dummy "parser", so that rust_type works. - struct parser_state ps; - initialize_expout (&ps, 0, &rust_language_defn, target_gdbarch ()); + struct parser_state ps (0, &rust_language_defn, target_gdbarch ()); rust_parser parser (&ps); rust_lex_test_one ("", 0); diff --git a/gdb/stap-probe.c b/gdb/stap-probe.c index 9f4e00845a..47e370c0ac 100644 --- a/gdb/stap-probe.c +++ b/gdb/stap-probe.c @@ -1146,25 +1146,14 @@ static expression_up stap_parse_argument (const char **arg, struct type *atype, struct gdbarch *gdbarch) { - struct stap_parse_info p; - struct cleanup *back_to; - /* We need to initialize the expression buffer, in order to begin our parsing efforts. We use language_c here because we may need to do pointer arithmetics. */ - initialize_expout (&p.pstate, 10, language_def (language_c), gdbarch); - back_to = make_cleanup (free_current_contents, &p.pstate.expout); - - p.saved_arg = *arg; - p.arg = *arg; - p.arg_type = atype; - p.gdbarch = gdbarch; - p.inside_paren_p = 0; + struct stap_parse_info p (*arg, atype, 10, language_def (language_c), + gdbarch); stap_parse_argument_1 (&p, 0, STAP_OPERAND_PREC_NONE); - discard_cleanups (back_to); - gdb_assert (p.inside_paren_p == 0); /* Casting the final expression to the appropriate type. */ @@ -1172,13 +1161,10 @@ stap_parse_argument (const char **arg, struct type *atype, write_exp_elt_type (&p.pstate, atype); write_exp_elt_opcode (&p.pstate, UNOP_CAST); - reallocate_expout (&p.pstate); - p.arg = skip_spaces (p.arg); *arg = p.arg; - /* We can safely return EXPOUT here. */ - return expression_up (p.pstate.expout); + return p.pstate.release (); } /* Implementation of 'parse_arguments' method. */ diff --git a/gdb/stap-probe.h b/gdb/stap-probe.h index c3327e6999..a277aefcf5 100644 --- a/gdb/stap-probe.h +++ b/gdb/stap-probe.h @@ -28,6 +28,22 @@ struct stap_parse_info { + stap_parse_info (const char *arg_, struct type *arg_type_, + size_t initial_size, const struct language_defn *lang, + struct gdbarch *gdbarch) + : arg (arg_), + pstate (initial_size, lang, gdbarch), + saved_arg (arg_), + arg_type (arg_type_), + gdbarch (gdbarch), + inside_paren_p (0) + { + } + + ~stap_parse_info () = default; + + DISABLE_COPY_AND_ASSIGN (stap_parse_info); + /* The probe's argument in a string format. */ const char *arg; ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [RFA] C++-ify parser_state 2017-12-09 4:41 ` Tom Tromey @ 2017-12-10 21:23 ` Simon Marchi 2017-12-31 0:04 ` Tom Tromey 0 siblings, 1 reply; 9+ messages in thread From: Simon Marchi @ 2017-12-10 21:23 UTC (permalink / raw) To: Tom Tromey; +Cc: Simon Marchi, gdb-patches On 2017-12-08 23:40, Tom Tromey wrote: >>>>>> "Simon" == Simon Marchi <simon.marchi@polymtl.ca> writes: > > Simon> Oops, I realized I only answered half the question. I added a > second > Simon> patch on top. > > No problem. How's this? LGTM, with some nits. > commit ee14e980825b5eabd6753a52c75b73dda0762bf2 > Author: Tom Tromey <tom@tromey.com> > Date: Wed Nov 22 21:45:53 2017 -0700 > > C++-ify parser_state > > This mildly C++-ifies parser_state and stap_parse_info -- just > enough > to remove some cleanups. > > This version includes the changes implemented by Simon. > > Regression tested by the buildbot. > > ChangeLog > 2017-11-26 Tom Tromey <tom@tromey.com> > Simon Marchi <simon.marchi@ericsson.com> > > * stap-probe.h (struct stap_parse_info): Add constructor, > destructor. > * stap-probe.c (stap_parse_argument): Update. > * rust-exp.y (rust_lex_tests): Update. > * parser-defs.h (struct parser_state): Add constructor, > destructor, release method. > (null_post_parser): Change type. > <expout>: Change type to expression_up. This last line should be before "null_post_parser" I think. > (initialize_expout, reallocate_expout): Remove. > * parse.c (parser_state::parser_state): Rename from > initialize_expout. > (parser_state::release): Rename from reallocate_expout. write_exp_elt, increase_expout_size should be mentioned. > (parse_exp_in_context_1): Update. > (null_post_parser): Change type of "exp". > * dtrace-probe.c (dtrace_probe::build_arg_exprs): Update. > * ada-lang.c (resolve, resolve_subexp) > (replace_operator_with_call): Change type of "expp". > * language.h (struct language_defn) <la_post_parser>: > Change type > of "expp". ... > --- a/gdb/stap-probe.h > +++ b/gdb/stap-probe.h > @@ -28,6 +28,22 @@ > > struct stap_parse_info > { > + stap_parse_info (const char *arg_, struct type *arg_type_, > + size_t initial_size, const struct language_defn *lang, > + struct gdbarch *gdbarch) > + : arg (arg_), > + pstate (initial_size, lang, gdbarch), > + saved_arg (arg_), > + arg_type (arg_type_), > + gdbarch (gdbarch), > + inside_paren_p (0) > + { > + } > + > + ~stap_parse_info () = default; This can be removed. Thanks, Simon ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [RFA] C++-ify parser_state 2017-12-10 21:23 ` Simon Marchi @ 2017-12-31 0:04 ` Tom Tromey 0 siblings, 0 replies; 9+ messages in thread From: Tom Tromey @ 2017-12-31 0:04 UTC (permalink / raw) To: Simon Marchi; +Cc: Tom Tromey, Simon Marchi, gdb-patches >>>>> "Simon" == Simon Marchi <simon.marchi@polymtl.ca> writes: >> No problem. How's this? Simon> LGTM, with some nits. I'm finally pushing this Tom ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2017-12-31 0:04 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-11-26 17:40 [RFA] C++-ify parser_state Tom Tromey 2017-11-27 14:17 ` Sergio Durigan Junior 2017-11-27 16:41 ` Simon Marchi 2017-11-30 3:29 ` Tom Tromey 2017-11-30 3:54 ` Simon Marchi 2017-11-30 5:49 ` Simon Marchi 2017-12-09 4:41 ` Tom Tromey 2017-12-10 21:23 ` Simon Marchi 2017-12-31 0:04 ` Tom Tromey
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).