Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(2202)

Issue 4800046: [pph] Ignore line number in diff checksum (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
13 years, 9 months ago by Gabriel Charette
Modified:
13 years, 9 months ago
CC:
gcc-patches_gcc.gnu.org
Visibility:
Public.

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+16 lines, -4 lines) Patch
M gcc/testsuite/ChangeLog.pph View 1 chunk +7 lines, -0 lines 0 comments Download
M gcc/testsuite/g++.dg/pph/c1eabi1.cc View 1 chunk +1 line, -1 line 0 comments Download
M gcc/testsuite/g++.dg/pph/p4eabi1.cc View 1 chunk +1 line, -1 line 0 comments Download
M gcc/testsuite/lib/dg-pph.exp View 1 chunk +7 lines, -2 lines 0 comments Download

Messages

Total messages: 2
Gabriel Charette
The recent implementation of comparing expected diff checksum had a problem: different configs generate different ...
13 years, 9 months ago (2011-07-22 23:24:28 UTC) #1
Lawrence Crowl
13 years, 9 months ago (2011-07-23 00:36:12 UTC) #2
On 7/22/11, Gabriel Charette <gchare@google.com> wrote:
> The recent implementation of comparing expected diff checksum had a problem:
> different configs generate different assembly (and although the diff itself
> was still the same in both case, the actual differences were on different
> lines; thus the checksum of the diff was different).
>
> Added logic to only diff on the actual differences.
>
> Couldn't find a good TCL solution... had to revert to using 'exec grep', if
> anyone has a good solution, I'm willing to change it...
>
> I also made a small modification to break down a line that was accidently
> longer than 80 columns in a previous checkin.
>
> Tested with pph regression testing.
>
> Gab
>
> diff --git a/gcc/testsuite/ChangeLog.pph b/gcc/testsuite/ChangeLog.pph
> index 7ffeaf2..c90ab86 100644
> --- a/gcc/testsuite/ChangeLog.pph
> +++ b/gcc/testsuite/ChangeLog.pph
> @@ -1,3 +1,10 @@
> +2011-07-22  Gabriel Charette  <gchare@google.com>
> +
> +	* g++.dg/pph/c1eabi1.cc: Change expected diff checksum.
> +	* g++.dg/pph/p4eabi1.cc: Change expected diff checksum.
> +	* lib/dg-pph.exp (dg-pph-pos): Ignore line numbers in diff checksum.
> +	Split long output message, using sumMessage, to respect 80 cols limit.
> +
>  2011-07-18  Gabriel Charette  <gchare@google.com>
>
>  	* lib/dg-pph.exp (dg-pph-pos): Output actualSum on unexpected diff.
> diff --git a/gcc/testsuite/g++.dg/pph/c1eabi1.cc
> b/gcc/testsuite/g++.dg/pph/c1eabi1.cc
> index 3321870..d26900f 100644
> --- a/gcc/testsuite/g++.dg/pph/c1eabi1.cc
> +++ b/gcc/testsuite/g++.dg/pph/c1eabi1.cc
> @@ -1,5 +1,5 @@
>  // { dg-options "-w -fpermissive" }
> -// pph asm xdiff 36206
> +// pph asm xdiff 60950
>
>  #include "c0eabi1.h"
>
> diff --git a/gcc/testsuite/g++.dg/pph/p4eabi1.cc
> b/gcc/testsuite/g++.dg/pph/p4eabi1.cc
> index 2f0715f..9b4739e 100644
> --- a/gcc/testsuite/g++.dg/pph/p4eabi1.cc
> +++ b/gcc/testsuite/g++.dg/pph/p4eabi1.cc
> @@ -1,5 +1,5 @@
>  // { dg-options "-w -fpermissive" }
> -// pph asm xdiff 15662
> +// pph asm xdiff 06317
>
>  #include "p4eabi1.h"
>
> diff --git a/gcc/testsuite/lib/dg-pph.exp b/gcc/testsuite/lib/dg-pph.exp
> index 013ccfe..677fcd7 100644
> --- a/gcc/testsuite/lib/dg-pph.exp
> +++ b/gcc/testsuite/lib/dg-pph.exp
> @@ -142,13 +142,18 @@ proc dg-pph-pos { subdir test options mapflag suffix }
> {
>  	file_on_host delete "$bname.s+pph"
>      } elseif { $adiff == 1 } {
>          verbose -log "Diff obtained:\n$diff_result"
> -	set actualSum [lindex [split [exec sum << $diff_result] " "] 0]
> +
> +	#only checksum on the actual differences, ignore line numbers
> +	set checksumed_diff [exec grep -E "^(<|>).*" << $diff_result]
> +	verbose -log "Diff checksumed:\n$checksumed_diff"
> +	set actualSum [lindex [split [exec sum << $checksumed_diff] " "] 0]
>  	if { $xdiff } {
>  	    set expectedSum [lindex [split $xdiff_entry " \}"] 3]
>  	    if { $expectedSum == $actualSum } {
>  	        xfail "$nshort $options (assembly comparison)"
>  	    } else {
> -	        fail "$nshort $options (assembly comparison, sums
> $expectedSum=>$actualSum)"
> +		set sumMessage "sums $expectedSum=>$actualSum"
> +	        fail "$nshort $options (assembly comparison, $sumMessage)"
>  	    }
>  	} else {
>  	    fail "$nshort $options (assembly comparison, sum=$actualSum)"
>
> --
> This patch is available for review at http://codereview.appspot.com/4800046
>

LGTM

-- 
Lawrence Crowl
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b