Alice Carlotti <[email protected]> writes:
> When a check-function-bodies test fails, it can sometimes be hard to
> identify which part of a large function regexp is causing the failure.
> To aid debugging these failures, find the largest initial set of lines
> that matches the function, and log this separately from the remainder of
> the regexp.
>
> Additionally, add a newline before logging the function bodies, so that
> the first line has the same indentation as the rest of that function.
>
>
> gcc/testsuite/ChangeLog:
>
> * lib/scanasm.exp (check_function_body): Log matching portion
> of body_regexp separately.
>
> ---
>
> I've tested this by taking working tests and deleting a line from the start,
> end, or middle of the regexp, including a line from the middle of a group of
> alternative sequences. In all cases the output was as expected (although less
> precise when alternatives are involved).
>
> Is this, or something similar, ok for master?
>
> Alice
Sorry for the slow reply. I was hoping that someone more "current" would
review.
> diff --git a/gcc/testsuite/lib/scanasm.exp b/gcc/testsuite/lib/scanasm.exp
> index
> c414d44f0f0e614fe96cdd64d51eb06758c47992..be26f16dd7dc0f0a7925d39d486e3c5b137fc8f8
> 100644
> --- a/gcc/testsuite/lib/scanasm.exp
> +++ b/gcc/testsuite/lib/scanasm.exp
> @@ -983,8 +983,31 @@ proc check_function_body { functions name body_regexp } {
> }
> set fn_res [regexp "^$body_regexp\$" $up_functions($name)]
> if { !$fn_res } {
> - verbose -log "body: $body_regexp"
> - verbose -log "against: $up_functions($name)"
> + # Find the longest initial set of lines in body_regexp that matches the
> + # function. Unfortunately this makes runtime quadratic in the number
> of
> + # lines, but failures in large check-function-body regexps are
> hopefully
> + # rare.
It might be worth expanding the comment to say that the possibility of
regexp errors means that a binary search wouldn't work. It would then
be more obvious that the linear search is used for correctness,
rather than just simplicity.
> + set ok_index [string length $body_regexp]
> + incr ok_index -1
> + while {$ok_index >= 0} {
> + set short_regexp [string range $body_regexp 0 $ok_index]
> + set match_found 0
> + # Not all initial sets of lines are valid. Ignore any broken regexps.
> + catch {
> + if { [regexp "^$short_regexp" $up_functions($name)] } {
> + set match_found 1
> + }
> + }
> + if {$match_found} {
> + break
> + }
Minor, but since $up_functions($name) is now used multiple times,
it might be worth storing it in a "body" variable:
set body $up_functions($name)
and replacing existing uses. Then the catch above could be written:
if { ![catch { regexp "^$short_regexp" $body } res] && $res } {
break
}
without the need for match_found.
> + set ok_index [string last "\n" $body_regexp $ok_index-1]
> + }
> + set head [string range $body_regexp 0 $ok_index]
> + set tail [string range $body_regexp $ok_index+1 end]
> + verbose -log "initial matched regexp:\n$head"
> + verbose -log "mismatch in remaining regexp:\n$tail"
> + verbose -log "against:\n$up_functions($name)"
I suppose there are two cases in which one of these regexps could
be empty:
(a) $head could be empty when even the smallest valid slice of
$body_regexp doesn't match. The original message might then
be clearer, but maybe not.
(b) $tail could be empty when the whole of $body_regexp matches,
so that it's the final "$" that causes the mismatch. It might
then be clearer to say that the regexp matches but is incomplete.
OK with the comment and catch changes, and with whatever you feel is
best for the new messages (including keeping the ones in the patch).
Thanks,
Richard
> }
> return $fn_res
> }
