[PATCH C++/testsuite] Remove pchtest check objects and compile with current tool

[PATCH C++/testsuite] Remove pchtest check objects and compile with current tool

[Bernhard Reutner-Fischer]

[-- Attachment #1: Type: text/plain, Size: 675 bytes --]

Hi,

The pch-init leaves check objects lying around, remove them.

While at it, i noticed that i was getting warnings from the check since
it was invoced with xg++ -nostdinc++ on C source (in one of the two
iterations the check is run -- once per tool).
My suggestion is to use the correct tool to perform the pch check. Does
that make sense to you?

Regstrapped (together with the auto-removal patch just sent) on
x86_64-unknown-linux-gnu with trunk@204119 without regressions.

Ok for trunk?

gcc/testsuite/ChangeLog

2013-10-12  Bernhard Reutner-Fischer  <aldot@gcc.gnu.org>

	* lib/dg-pch.exp (pch-init): Remove pchtest check objects.
	Compile pchtest with current tool.

[-- Attachment #2: 0002-remove-pchtest-objects.patch --]
[-- Type: text/x-diff, Size: 1557 bytes --]

diff --git a/gcc/testsuite/lib/dg-pch.exp b/gcc/testsuite/lib/dg-pch.exp
index d82c669..41de454 100644
--- a/gcc/testsuite/lib/dg-pch.exp
+++ b/gcc/testsuite/lib/dg-pch.exp
@@ -18,6 +18,18 @@ load_lib copy-file.exp
 
 proc pch-init { args } {
     global pch_unsupported_debug pch_unsupported
+    global tool
+
+    set chk_content ""
+    set chk_lang "c-header"
+    switch -- "$tool" {
+	"g++" {
+		set chk_content "// C++"
+		set chk_lang "c++-header"
+	      }
+    }
+    append chk_content "\nint i;"
+    set chk_lang "-x $chk_lang"
 
     if [info exists pch_unsupported_debug] {
 	error "pch-init: pch_unsupported_debug is not empty as expected"
@@ -26,20 +38,22 @@ proc pch-init { args } {
 	error "pch-init: pch_unsupported is not empty as expected"
     }
 
-    set result [check_compile pchtest object "int i;" "-g -x c-header"]
+    set result [check_compile pchtest object "$chk_content" "-g $chk_lang"]
     set pch_unsupported_debug \
 	[regexp "debug format cannot be used with pre-compiled headers" \
 		[lindex $result 0]]
+    remove-build-file [lindex $result 1]
 
     set pch_unsupported 0
     if { $pch_unsupported_debug } {
 	verbose -log "pch is unsupported with the debug info format"
 
-	set result [check_compile pchtest object "int i;" "-x c-header"]
+	set result [check_compile pchtest object "$chk_type" "$chk_lang"]
 	    set pch_unsupported \
 		[regexp "debug format cannot be used with pre-compiled headers" \
 			[lindex $result 0]]
     }
+    remove-build-file [lindex $result 1]
 }
 
 proc pch-finish { args } {

Re: [PATCH C++/testsuite] Remove pchtest check objects and compile with current tool

[Mike Stump]

On Oct 30, 2013, at 2:56 AM, Bernhard Reutner-Fischer <rep.dot.nop@gmail.com> wrote:
> -	set result [check_compile pchtest object "int i;" "-x c-header"]
> +	set result [check_compile pchtest object "$chk_type" "$chk_lang"]

the patch uses chk_type, but, I can't find where it is being set?

Was there a significant purpose for the added C++ comment?  If not, can you remove that?  If so, can you explain?

Last question I have is the remove-build-file primitive.  I'm wondering on a canadian cross, are the files left over on the build machine, the host machine or both the build machine and the host machine?
I see people use remote_file build delete …, file_on_host delete and remove-build-file.  Some folks even use the plain file delete.  I'd hate to guess which one you need, it hurts my brain.  I think remove-build-file is safe; just don't know if it is best.  Anyone else want to weigh in?

Re: [PATCH C++/testsuite] Remove pchtest check objects and compile with current tool

[Bernhard Reutner-Fischer]

On 30 October 2013 22:47, Mike Stump <mikestump@comcast.net> wrote:
> On Oct 30, 2013, at 2:56 AM, Bernhard Reutner-Fischer <rep.dot.nop@gmail.com> wrote:
>> -     set result [check_compile pchtest object "int i;" "-x c-header"]
>> +     set result [check_compile pchtest object "$chk_type" "$chk_lang"]
>
> the patch uses chk_type, but, I can't find where it is being set?

hmz yea, that should read $chk_content
>
> Was there a significant purpose for the added C++ comment?  If not, can you remove that?  If so, can you explain?

grep -A9 "CONTENTS is" gcc/testsuite/lib/target-supports.exp
# Assume by default that CONTENTS is C code.
# Otherwise, code should contain:
# "// C++" for c++,
# "! Fortran" for Fortran code,
# "/* ObjC", for ObjC
# "// ObjC++" for ObjC++
# and "// Go" for Go
# If the tool is ObjC/ObjC++ then we overide the extension to .m/.mm to
# allow for ObjC/ObjC++ specific flags.
proc check_compile {basename type contents args} {
>
> Last question I have is the remove-build-file primitive.  I'm wondering on a canadian cross, are the files left over on the build machine, the host machine or both the build machine and the host machine?

I don't really remember, i didn't run canadian cross tests on remote
boxes since ages, TBH.

> I see people use remote_file build delete …, file_on_host delete and remove-build-file.  Some folks even use the plain file delete.  I'd hate to guess which one you need, it hurts my brain.  I think remove-build-file is safe; just don't know if it is best.

remove-build-file certainly wipes it from everywhere so seems the safe bet.
But yes, for this specific pchtest.o's one could refine the delete to
the appropriate build or host. I would think that using plain delete
is wrong everywhere though.

> Anyone else want to weigh in?

Re: [PATCH C++/testsuite] Remove pchtest check objects and compile with current tool

[Bernhard Reutner-Fischer]

On 30 October 2013 22:47, Mike Stump <mikestump@comcast.net> wrote:
> On Oct 30, 2013, at 2:56 AM, Bernhard Reutner-Fischer <rep.dot.nop@gmail.com> wrote:
>> -     set result [check_compile pchtest object "int i;" "-x c-header"]
>> +     set result [check_compile pchtest object "$chk_type" "$chk_lang"]
>
> the patch uses chk_type, but, I can't find where it is being set?

hmz yea, that should read $chk_content
>
> Was there a significant purpose for the added C++ comment?  If not, can you remove that?  If so, can you explain?

grep -A9 "CONTENTS is" gcc/testsuite/lib/target-supports.exp
# Assume by default that CONTENTS is C code.
# Otherwise, code should contain:
# "// C++" for c++,
# "! Fortran" for Fortran code,
# "/* ObjC", for ObjC
# "// ObjC++" for ObjC++
# and "// Go" for Go
# If the tool is ObjC/ObjC++ then we overide the extension to .m/.mm to
# allow for ObjC/ObjC++ specific flags.
proc check_compile {basename type contents args} {
>
> Last question I have is the remove-build-file primitive.  I'm wondering on a canadian cross, are the files left over on the build machine, the host machine or both the build machine and the host machine?

I don't really remember, i didn't run canadian cross tests on remote
boxes since ages, TBH.

> I see people use remote_file build delete …, file_on_host delete and remove-build-file.  Some folks even use the plain file delete.  I'd hate to guess which one you need, it hurts my brain.  I think remove-build-file is safe; just don't know if it is best.

remove-build-file certainly wipes it from everywhere so seems the safe bet.
But yes, for this specific pchtest.o's one could refine the delete to
the appropriate build or host. I would think that using plain delete
is wrong everywhere though.

> Anyone else want to weigh in?

Re: [PATCH C++/testsuite] Remove pchtest check objects and compile with current tool

[Mike Stump]

On Oct 30, 2013, at 3:14 PM, Bernhard Reutner-Fischer <rep.dot.nop@gmail.com> wrote:
> On 30 October 2013 22:47, Mike Stump <mikestump@comcast.net> wrote:
>> 
>> Was there a significant purpose for the added C++ comment?  If not, can you remove that?  If so, can you explain?
> 
> grep -A9 "CONTENTS is" gcc/testsuite/lib/target-supports.exp
> # Assume by default that CONTENTS is C code.
> # Otherwise, code should contain:
> # "// C++" for c++,
> # "! Fortran" for Fortran code,
> # "/* ObjC", for ObjC
> # "// ObjC++" for ObjC++
> # and "// Go" for Go
> # If the tool is ObjC/ObjC++ then we overide the extension to .m/.mm to
> # allow for ObjC/ObjC++ specific flags.
> proc check_compile {basename type contents args} {

Ah, but this is why I asked for a significant purpose?  The language of the file selects the options (flags) allowed.  The language is set in your code.  I think it was part of trying different ways to fix it, but, it turned out to be neither necessary or sufficient in the end.

Re: [PATCH C++/testsuite] Remove pchtest check objects and compile with current tool

[Bernhard Reutner-Fischer]

On 30 October 2013 23:22, Mike Stump <mikestump@comcast.net> wrote:
> On Oct 30, 2013, at 3:14 PM, Bernhard Reutner-Fischer <rep.dot.nop@gmail.com> wrote:
>> On 30 October 2013 22:47, Mike Stump <mikestump@comcast.net> wrote:
>>>
>>> Was there a significant purpose for the added C++ comment?  If not, can you remove that?  If so, can you explain?
>>
>> grep -A9 "CONTENTS is" gcc/testsuite/lib/target-supports.exp
>> # Assume by default that CONTENTS is C code.
>> # Otherwise, code should contain:
>> # "// C++" for c++,
>> # "! Fortran" for Fortran code,
>> # "/* ObjC", for ObjC
>> # "// ObjC++" for ObjC++
>> # and "// Go" for Go
>> # If the tool is ObjC/ObjC++ then we overide the extension to .m/.mm to
>> # allow for ObjC/ObjC++ specific flags.
>> proc check_compile {basename type contents args} {
>
> Ah, but this is why I asked for a significant purpose?  The language of the file selects the options (flags) allowed.  The language is set in your code.  I think it was part of trying different ways to fix it, but, it turned out to be neither necessary or sufficient in the end.

Not sure about any significant purpose, no.
I found it odd that the check did not attempt to obtain a result
without knowingly provoking an odd warning, hence these chk_ stuff.
So, what do you want me to do? I want to delete the test objects and i
don't really care if remove-build-file overdoes it or not..
pch usually fails for my crosses anyway so is disabled in the first place :P

Are you saying that these CONTENT stuff should be nuked altogether
and/or the pchtest stanza be kept as is and/or the stanza be cut to
only fire for tool==g++ ?

thanks,

Re: [PATCH C++/testsuite] Remove pchtest check objects and compile with current tool

[Mike Stump]

On Oct 31, 2013, at 1:47 AM, Bernhard Reutner-Fischer <rep.dot.nop@gmail.com> wrote:
> On 30 October 2013 23:22, Mike Stump <mikestump@comcast.net> wrote:
>> On Oct 30, 2013, at 3:14 PM, Bernhard Reutner-Fischer <rep.dot.nop@gmail.com> wrote:
>>> On 30 October 2013 22:47, Mike Stump <mikestump@comcast.net> wrote:
>>>> 
>>>> Was there a significant purpose for the added C++ comment?  If not, can you remove that?  If so, can you explain?
>>> 
>>> grep -A9 "CONTENTS is" gcc/testsuite/lib/target-supports.exp
>>> # Assume by default that CONTENTS is C code.
>>> # Otherwise, code should contain:
>>> # "// C++" for c++,
>>> # "! Fortran" for Fortran code,
>>> # "/* ObjC", for ObjC
>>> # "// ObjC++" for ObjC++
>>> # and "// Go" for Go
>>> # If the tool is ObjC/ObjC++ then we overide the extension to .m/.mm to
>>> # allow for ObjC/ObjC++ specific flags.
>>> proc check_compile {basename type contents args} {
>> 
>> Ah, but this is why I asked for a significant purpose?  The language of the file selects the options (flags) allowed.  The language is set in your code.  I think it was part of trying different ways to fix it, but, it turned out to be neither necessary or sufficient in the end.
> 
> Not sure about any significant purpose, no.

Ok, then it can be safely removed.

> So, what do you want me to do?

Remove the added comment…   and repost…

Thanks.

Re: [PATCH C++/testsuite] Remove pchtest check objects and compile with current tool

[Bernhard Reutner-Fischer]

[-- Attachment #1: Type: text/plain, Size: 3218 bytes --]

On 1 November 2013 02:15, Mike Stump <mikestump@comcast.net> wrote:
> On Oct 31, 2013, at 1:47 AM, Bernhard Reutner-Fischer <rep.dot.nop@gmail.com> wrote:
>> On 30 October 2013 23:22, Mike Stump <mikestump@comcast.net> wrote:
>>> On Oct 30, 2013, at 3:14 PM, Bernhard Reutner-Fischer <rep.dot.nop@gmail.com> wrote:
>>>> On 30 October 2013 22:47, Mike Stump <mikestump@comcast.net> wrote:
>>>>>
>>>>> Was there a significant purpose for the added C++ comment?  If not, can you remove that?  If so, can you explain?
>>>>
>>>> grep -A9 "CONTENTS is" gcc/testsuite/lib/target-supports.exp
>>>> # Assume by default that CONTENTS is C code.
>>>> # Otherwise, code should contain:
>>>> # "// C++" for c++,
>>>> # "! Fortran" for Fortran code,
>>>> # "/* ObjC", for ObjC
>>>> # "// ObjC++" for ObjC++
>>>> # and "// Go" for Go
>>>> # If the tool is ObjC/ObjC++ then we overide the extension to .m/.mm to
>>>> # allow for ObjC/ObjC++ specific flags.
>>>> proc check_compile {basename type contents args} {
>>>
>>> Ah, but this is why I asked for a significant purpose?  The language of the file selects the options (flags) allowed.  The language is set in your code.  I think it was part of trying different ways to fix it, but, it turned out to be neither necessary or sufficient in the end.
>>
>> Not sure about any significant purpose, no.
>
> Ok, then it can be safely removed.
>
>> So, what do you want me to do?
>
> Remove the added comment…   and repost…
>
> Thanks.

Attached.

Ok for trunk?

gcc/testsuite/ChangeLog

2013-10-12  Bernhard Reutner-Fischer  <aldot@gcc.gnu.org>

        * lib/dg-pch.exp (pch-init): Remove pchtest check objects.


Oh, and while double-checking the auto-wipe patch that i just
simplified, i encountered an inconvenient side-effect of checking
pristine trunk with plain
$ make -k check -j4
versus my local tree with
$ make -k check -j4 RUNTESTFLAGS="-v -v -v -v "

This bug/feature was added in a0d20ccbd97fde9c2af1f7345e3eb1313dea570f
aka svn r141337 .
From the sounds, if --target_board= is seen, then run specific test, serial.
Otherwise do the parallel stuff, i.e. check_DEJAGNU_normal_targets.
.
Still i want -v to not behave differently as not passing -v in
RUNTESTFLAGS especially regarding parallelism, so what about doing:
diff --git a/libstdc++-v3/testsuite/Makefile.am
b/libstdc++-v3/testsuite/Makefile.am
index 4c92fef..6edc410 100644
--- a/libstdc++-v3/testsuite/Makefile.am
+++ b/libstdc++-v3/testsuite/Makefile.am
@@ -108,7 +108,7 @@ $(check_DEJAGNU_normal_targets):
check-DEJAGNUnormal%: normal%/site.exp
 check-DEJAGNU $(check_DEJAGNU_normal_targets): check-DEJAGNU%: site.exp
  AR="$(AR)"; export AR; \
  RANLIB="$(RANLIB)"; export RANLIB; \
- if [ -z "$*$(filter-out --target_board=%, $(RUNTESTFLAGS))" ] \
+ if [ -z "$*$findstring --target_board=,$(RUNTESTFLAGS))" ] \
     && [ "$(filter -j, $(MFLAGS))" = "-j" ]; then \
   $(MAKE) $(AM_MAKEFLAGS) $(check_DEJAGNU_normal_targets); \
   for idx in 0 1 2 3 4 5 6 7 8 9; do \

This may run non-parallel if somebody has RUNTESTFLAGS=" -v -v
--target_board=" but at least we don't have -v --verbose -d --debug in
the filter-out.
Mike?

Thanks,

[-- Attachment #2: wipe-pchtests.01.patch.txt --]
[-- Type: text/plain, Size: 666 bytes --]

diff --git a/gcc/testsuite/lib/dg-pch.exp b/gcc/testsuite/lib/dg-pch.exp
index d82c669..7bc5c45 100644
--- a/gcc/testsuite/lib/dg-pch.exp
+++ b/gcc/testsuite/lib/dg-pch.exp
@@ -30,6 +30,7 @@ proc pch-init { args } {
     set pch_unsupported_debug \
 	[regexp "debug format cannot be used with pre-compiled headers" \
 		[lindex $result 0]]
+    remove-build-file [lindex $result 1]
 
     set pch_unsupported 0
     if { $pch_unsupported_debug } {
@@ -40,6 +41,7 @@ proc pch-init { args } {
 		[regexp "debug format cannot be used with pre-compiled headers" \
 			[lindex $result 0]]
     }
+    remove-build-file [lindex $result 1]
 }
 
 proc pch-finish { args } {

Re: [PATCH C++/testsuite] Remove pchtest check objects and compile with current tool

[Mike Stump]

On Nov 9, 2013, at 5:34 AM, Bernhard Reutner-Fischer <rep.dot.nop@gmail.com> wrote:
> Ok for trunk?

> 2013-10-12  Bernhard Reutner-Fischer  <aldot@gcc.gnu.org>
> 
>        * lib/dg-pch.exp (pch-init): Remove pchtest check objects.

Ok.

The code that does set chk_lang "c++-header", but just not the code that puts in the comment, is fine to repost as well.

> Oh, and while double-checking the auto-wipe patch that i just
> simplified, i encountered an inconvenient side-effect of checking
> pristine trunk with plain
> $ make -k check -j4
> versus my local tree with
> $ make -k check -j4 RUNTESTFLAGS="-v -v -v -v "
> 
> This bug/feature was added in a0d20ccbd97fde9c2af1f7345e3eb1313dea570f
> aka svn r141337 .
> From the sounds, if --target_board= is seen, then run specific test, serial.
> Otherwise do the parallel stuff, i.e. check_DEJAGNU_normal_targets.
> .
> Still i want -v to not behave differently as not passing -v in
> RUNTESTFLAGS especially regarding parallelism, so what about doing:
> diff --git a/libstdc++-v3/testsuite/Makefile.am
> b/libstdc++-v3/testsuite/Makefile.am
> index 4c92fef..6edc410 100644
> --- a/libstdc++-v3/testsuite/Makefile.am
> +++ b/libstdc++-v3/testsuite/Makefile.am
> @@ -108,7 +108,7 @@ $(check_DEJAGNU_normal_targets):
> check-DEJAGNUnormal%: normal%/site.exp
> check-DEJAGNU $(check_DEJAGNU_normal_targets): check-DEJAGNU%: site.exp
>  AR="$(AR)"; export AR; \
>  RANLIB="$(RANLIB)"; export RANLIB; \
> - if [ -z "$*$(filter-out --target_board=%, $(RUNTESTFLAGS))" ] \
> + if [ -z "$*$findstring --target_board=,$(RUNTESTFLAGS))" ] \
>     && [ "$(filter -j, $(MFLAGS))" = "-j" ]; then \
>   $(MAKE) $(AM_MAKEFLAGS) $(check_DEJAGNU_normal_targets); \
>   for idx in 0 1 2 3 4 5 6 7 8 9; do \
> 
> This may run non-parallel if somebody has RUNTESTFLAGS=" -v -v
> --target_board=" but at least we don't have -v --verbose -d --debug in
> the filter-out.
> Mike?

Head, hurts; can you discuss with jakub.  I just want to get out of the way and let you two decide what is best.