On 8/28/21 11:28 PM, Simon Marchi wrote:
> 
> 
> On 2021-08-28 4:29 p.m., Simon Marchi via Gdb-patches wrote:
>>
>>
>> On 2021-08-28 11:31 a.m., Tom de Vries wrote:
>>> On 8/27/21 5:09 PM, Simon Marchi via Gdb-patches wrote:
>>>>
>>>>
>>>> On 2021-08-27 9:35 a.m., Tom Tromey wrote:
>>>>>>>>>> "Tom" == Tom de Vries via Gdb-patches <gdb-patches@sourceware.org> writes:
>>>>>
>>>>> Tom> +    #   arange [-c <comment>] [<segment selector>] <start> <length>
>>>>> Tom> +    #     -- adds an address range.
>>>>>
>>>>> I wonder if there's a way to make this more tcl-ish, say by rearranging
>>>>> the order of arguments so that things can be defaulted.  I think the
>>>>> "args"-parsing style should normally be a last resort.
>>>>
>>>> I personally don't like this style
>>>>
>>>>     proc arange { arange_start arange_length {comment ""} {seg_sel ""} }
>>>>
>>>> ... because if you want to specify the last parameter, you need to give
>>>> all the other optional ones before.
>>>>
>>>> I also agree that just having:
>>>>
>>>>     proc arange { args }
>>>>
>>>> is not great, since we have to do the argument parsing by hand, and it's
>>>> a bit opaque what the proc accepts.  Could we consistently use the
>>>> "options" pattern, such as the one used by aranges and cu?
>>>>
>>>>    proc arange { options arange_start arange_length }
>>>>
>>>> The callers would look like:
>>>>
>>>>     arange {} $start $length
>>>>     arange {
>>>>        comment $comment
>>>>        seg_sel $seg_sel
>>>>     } $start $length
>>>>
>>>> I think that's a good compromise.  I could re-do the rnglists procs this
>>>> way, if you'd like.
>>>>
>>>
>>> This patch implements that approach, using a new proc parse_options
>>> similar to parse_args.
>>>
>>> WDYT?
>>
>> Here:
>>
>> @@ -2354,9 +2350,9 @@ namespace eval Dwarf {
>>  	# Terminator tuple.
>>  	set comment "Terminator"
>>  	if { $_seg_size == 0 } {
>> -	    arange 0 0 $comment
>> +	    arange [list comment $comment] 0 0
>>  	} else {
>> -	    arange 0 0 $comment 0
>> +	    arange [list comment $comment seg_sel 0] 0 0
>>  	}
>>  
>>
>> Could we apply some magic so that we are able to use { } instead of
>> list?
>>
>>   arange {
>>     comment $comment
>>     set_seg 0
>>   } { ... }
>>
>> ... instead of having to use [list ...]?  I suppose doing an "eval" or
>> something of the option value in the caller's context?
> 
> Here's a patch that does it using subst (as well as changes rnglists and
> loclists to use parse_options, but that should be in a separate patch):
> 

OK, I've integrated the subst bit (and made it conditional, that was
still missing).  Also added the subst part for the default arguments,
and added a proper log message.  Any further comments?

[ FWIW, a bit of bike-shedding.  I could imagine a pattern where you
pair a proc <name> and <name>_ like this:
...
  proc arange_ { options start len } {
    ...
  }
  proc arange { start len } {
    arange_ {} start len
  }
...
to be able to do:
...
  arange $start $len
...
and:
...
  arange_ { comment "bla" } $start $len
...
which would be slightly less annoying that having to specify empty
options. ]

Thanks,
- Tom