This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH v2 03/11] [PR gdb/14441] gdb: valops: add ability to return rvalue reference values from value_ref()
- From: Keith Seitz <keiths at redhat dot com>
- To: Artemiy Volkov <artemiyv at acm dot org>
- Cc: gdb-patches at sourceware dot org
- Date: Fri, 19 Feb 2016 10:52:41 -0800
- Subject: Re: [PATCH v2 03/11] [PR gdb/14441] gdb: valops: add ability to return rvalue reference values from value_ref()
- Authentication-results: sourceware.org; auth=none
- References: <1450661481-31178-1-git-send-email-artemiyv at acm dot org> <1453229609-20159-1-git-send-email-artemiyv at acm dot org> <1453229609-20159-4-git-send-email-artemiyv at acm dot org>
Just "nits" in this review.
On 01/19/2016 10:53 AM, Artemiy Volkov wrote:
> 2016-01-19 Artemiy Volkov <artemiyv@acm.org>
>
> * gdb/ada-lang.c (ada_evaluate_subexp): Adhere to the new
> value_ref() interface.
> * gdb/c-valprint.c (c_value_print): Likewise.
> * gdb/infcall.c (value_arg_coerce): Likewise.
> * gdb/python/py-value.c (valpy_reference_value): Likewise.
> * gdb/valops.c (value_cast): Likewise.
> (value_reinterpret_cast): Likewise.
> (value_dynamic_cast): Likewise.
> (value_ref): Parameterize by kind of return value reference type.
> (typecmp): Likewise.
Another place that could be simplified, removing "likewise."
> * gdb/value.h: New interface.
This isn't a particularly useful comment. ChangeLogs explain what changed:
* gdb/value.h (value_ref): Add new parameter "refcode".
> diff --git a/gdb/c-valprint.c b/gdb/c-valprint.c
> index 62552ec..e210e99 100644
> --- a/gdb/c-valprint.c
> +++ b/gdb/c-valprint.c
> @@ -602,10 +602,13 @@ c_value_print (struct value *val, struct ui_file *stream,
> else if (options->objectprint
> && (TYPE_CODE (TYPE_TARGET_TYPE (type)) == TYPE_CODE_STRUCT))
> {
> - int is_ref = TYPE_CODE (type) == TYPE_CODE_REF;
> + int is_ref = TYPE_REFERENCE (type);
> + enum type_code refcode = TYPE_CODE_UNDEF;
Indentation?
>
> - if (is_ref)
> + if (is_ref) {
> val = value_addr (val);
> + refcode = TYPE_CODE (type);
> + }
>
> /* Pointer to class, check real type of object. */
> fprintf_filtered (stream, "(");
> diff --git a/gdb/valops.c b/gdb/valops.c
> index 1aafb5a..cc01689 100644
> --- a/gdb/valops.c
> +++ b/gdb/valops.c
> @@ -828,7 +829,7 @@ value_dynamic_cast (struct type *type, struct value *arg)
> value_address (tem), tem,
> rtti_type, &result) == 1)
> return value_cast (type,
> - is_ref ? value_ref (result) : value_addr (result));
> + is_ref ? value_ref (result, TYPE_CODE (resolved_type)) : value_addr (result));
>
> if (TYPE_CODE (resolved_type) == TYPE_CODE_PTR)
> return value_zero (type, not_lval);
The two changed lines in the above hunk are too long.
> @@ -1499,16 +1500,19 @@ value_addr (struct value *arg1)
> contents. */
>
> struct value *
> -value_ref (struct value *arg1)
> +value_ref (struct value *arg1, enum type_code refcode)
> {
> struct value *arg2;
> struct type *type = check_typedef (value_type (arg1));
>
> - if (TYPE_CODE (type) == TYPE_CODE_REF)
> + gdb_assert (refcode == TYPE_CODE_REF || refcode == TYPE_CODE_RVALUE_REF);
> +
> + if ((TYPE_CODE (type) == TYPE_CODE_REF || TYPE_CODE (type) == TYPE_CODE_RVALUE_REF)
> + && TYPE_CODE (type) == refcode)
> return arg1;
>
The "if ((TYPE_CODE...)" line is too long.
> arg2 = value_addr (arg1);
> - deprecated_set_value_type (arg2, lookup_lvalue_reference_type (type));
> + deprecated_set_value_type (arg2, lookup_reference_type (type, refcode));
> return arg2;
> }
>
Keith