Patchwork [pph] Ignore line number in diff checksum (issue4800046)

login
register
mail settings
Submitter Gab Charette
Date July 22, 2011, 11:24 p.m.
Message ID <20110722232426.467721C379C@gchare.mtv.corp.google.com>
Download mbox | patch
Permalink /patch/106400/
State New
Headers show

Comments

Gab Charette - July 22, 2011, 11:24 p.m.
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


--
This patch is available for review at http://codereview.appspot.com/4800046
Lawrence Crowl - July 23, 2011, 12:36 a.m.
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

Patch

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)"