Message ID | 4FD90309.1030108@mentor.com |
---|---|
State | New |
Headers | show |
On Wed, 13 Jun 2012, Janis Johnson wrote: > PCH test infrastructure compiles each test twice, with identical results > in the test summary file (assuming they both pass or both fail). This > patch adds an extra flag to each compile, "-Dcompile1" or "-Dcompile2", > to make the summary lines unique. This is a rather lame fix but at > least I added a comment. I would have said this was bug 20771, except that you committed a fix for that in 2008, so either it's a reappearance or the reason you didn't close the bug in 2008 was that your fix was known incomplete. Anyway, check for open "testsuite" bugs for the issues you are fixing.
On 06/13/2012 02:37 PM, Joseph S. Myers wrote: > On Wed, 13 Jun 2012, Janis Johnson wrote: > >> PCH test infrastructure compiles each test twice, with identical results >> in the test summary file (assuming they both pass or both fail). This >> patch adds an extra flag to each compile, "-Dcompile1" or "-Dcompile2", >> to make the summary lines unique. This is a rather lame fix but at >> least I added a comment. > > I would have said this was bug 20771, except that you committed a fix for > that in 2008, so either it's a reappearance or the reason you didn't close > the bug in 2008 was that your fix was known incomplete. Anyway, check for > open "testsuite" bugs for the issues you are fixing. > The patch for PR20771 did fix the problem (so I should have closed it) but it was undone as part of a later change to the same lines. I could either do that again, with comments this time explaining why they are slightly different, or use "-Dwith_PCH" and "-Dwithout_PCH" in the flags that will appear in the test summary. Do you have a preference? Janis
On Wed, 13 Jun 2012, Janis Johnson wrote: > The patch for PR20771 did fix the problem (so I should have closed it) > but it was undone as part of a later change to the same lines. I could > either do that again, with comments this time explaining why they are > slightly different, or use "-Dwith_PCH" and "-Dwithout_PCH" in the flags > that will appear in the test summary. Do you have a preference? I have no preference here.
On Jun 13, 2012, at 2:59 PM, Joseph S. Myers wrote: > On Wed, 13 Jun 2012, Janis Johnson wrote: > >> The patch for PR20771 did fix the problem (so I should have closed it) >> but it was undone as part of a later change to the same lines. I could >> either do that again, with comments this time explaining why they are >> slightly different, or use "-Dwith_PCH" and "-Dwithout_PCH" in the flags >> that will appear in the test summary. Do you have a preference? > > I have no preference here. Ok for mainline. My only preference was to have a comment above the line that has the -D flag telling why. You have that in your patch, so I'm happy. If you want to use -Dwith_PCH, I'm fine with either spelling.
Index: lib/dg-pch.exp =================================================================== --- lib/dg-pch.exp (revision 188482) +++ lib/dg-pch.exp (working copy) @@ -50,14 +50,16 @@ # Ensure that the PCH file is used, not the original header. file_on_host delete "$bname$suffix" - dg-test -keep-output $test "$otherflags $flags -I." "" + # The flags "-Dcompile1" and "-Dcompile2" are to distinguish the + # two compiles in test summary lines. + dg-test -keep-output $test "$otherflags $flags -I. -Dcompile1" "" file_on_host delete "$bname$suffix.gch" if { !$have_errs } { if { [ file_on_host exists "$bname.s" ] } { remote_upload host "$bname.s" "$bname.s-gch" remote_download host "$bname.s-gch" gcc_copy_files "[file rootname $test]${suffix}s" "$bname$suffix" - dg-test -keep-output $test "$otherflags $flags -I." "" + dg-test -keep-output $test "$otherflags $flags -I. -Dcompile2" "" remote_upload host "$bname.s" set tmp [ diff "$bname.s" "$bname.s-gch" ] if { $tmp == 0 } { @@ -89,4 +91,4 @@ proc dg-pch { subdir test options suffix } { return [dg-flags-pch $subdir $test "" $options $suffix] -} \ No newline at end of file +}