Re: [PATCH 13/43] Move piece_closure and its support to expr.c

Zoran Zaric via Gdb-patches
2021-04-28T11:40:51Z

On 4/28/21 2:56 AM, Simon Marchi wrote:
> [CAUTION: External Email]
>
> On 2021-03-01 9:45 a.m., Zoran Zaric via Gdb-patches wrote:
>> @@ -300,4 +301,15 @@ extern const gdb_byte *safe_read_sleb128 (const gdb_byte *buf,
>> extern const gdb_byte *safe_skip_leb128 (const gdb_byte *buf,
>> const gdb_byte *buf_end);
>>
>> +extern const struct lval_funcs pieced_value_funcs;
>> +
>> +/* Allocate a closure for a value formed from separately-described
>> + PIECES. */
>> +
>> +struct piece_closure *allocate_piece_closure
>> + (dwarf2_per_cu_data *per_cu,
>> + dwarf2_per_objfile *per_objfile,
>> + std::vector<dwarf_expr_piece> &&pieces,
>> + struct frame_info *frame);
>
> Indent with two columns:
>
> struct piece_closure *allocate_piece_closure
> (dwarf2_per_cu_data *per_cu,
> dwarf2_per_objfile *per_objfile,
> std::vector<dwarf_expr_piece> &&pieces,
> struct frame_info *frame);

Thanks, I will fix it in the next iteration.
>
>> @@ -96,8 +89,7 @@ enum debug_loc_kind
>> DEBUG_LOC_INVALID_ENTRY = -2
>> };
>>
>> -/* Helper function which throws an error if a synthetic pointer is
>> - invalid. */
>> +/* See loc.h. */
>>
>> static void
>> invalid_synthetic_pointer (void)
>
> I don't think you wanted to change this comment.
>

Thanks, I will fix it in the next iteration.

>> @@ -286,4 +278,13 @@ extern int dwarf_reg_to_regnum (struct gdbarch *arch, int dwarf_reg);
>> extern int dwarf_reg_to_regnum_or_error (struct gdbarch *arch,
>> ULONGEST dwarf_reg);
>>
>> +/* Fetch the value pointed to by a synthetic pointer. */
>> +
>> +extern struct value *indirect_synthetic_pointer
>> + (sect_offset die, LONGEST byte_offset,
>> + dwarf2_per_cu_data *per_cu,
>> + dwarf2_per_objfile *per_objfile,
>> + struct frame_info *frame,
>> + struct type *type, bool resolve_abstract_p = false);
>
> This would typically have two columns of indent:
>
> extern struct value *indirect_synthetic_pointer
> (sect_offset die, LONGEST byte_offset,
> dwarf2_per_cu_data *per_cu,
> dwarf2_per_objfile *per_objfile,
> struct frame_info *frame,
> struct type *type, bool resolve_abstract_p = false);
>
> Although I found one instance in this file of a declaration formatted
> like you did, it can be confusing.
>
> Simon
>

Thanks, I will fix it in the next iteration.

Zoran