Re: [PATCH] D: Fix crash when expression debugging

[Iain Buclaw <ibuclaw@gdcproject.org>]

On 5 January 2017 at 17:13, Simon Marchi <simon.marchi@polymtl.ca> wrote:
>>>>>>> +if { [skip_d_tests] } { continue }
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>> We should output a message:
>>>>>>
>>>>>> untested "skipping d language tests"
>>>>>>
>>>>>> It may be more reasonable to just return instead of continuing? The
>>>>>> effect
>>>>>> will probably be the same, but it is a bit confusing to read
>>>>>> "continue"
>>>>>> without a visible loop.
>>>>>>
>>>>>
>>>>> I did a quick grep, and it seems like everyone is skippingtests in
>>>>> this way except for gdb.ada and gdb.btrace which are doing { return -1
>>>>> }
>>>>>
>>>>>
>>>>
>>>> That's a bit of a stretch. Take, for example, a few of the examples in
>>>> gdb.base. You will see a number of them returning.
>>>>
>>>> The problem here is inheriting past confusing practices when we use some
>>>> existing files to create new ones, which is not your fault really. I'm
>>>> guilty myself. :-)
>>>>
>>>
>>> Yes indeed. I wasn't disagreeing, just questioning the two competing
>>> ways of returning.
>>>
>>> I will update to use return and push this in then if there's no
>>> disagreement. :-)
>>>
>>
>> Hopefully someone will chime in for a second opinion. :-)
>
>
> +1 :)


FYI, committed patched as-is except for the following adjustment as discussed:

-if { [skip_d_tests] } { continue }
+if { [skip_d_tests] } { return -1 }


Thanks for the review.