| Message ID | 20131030095618.GB22154@nbbrfq.cc.univie.ac.at |
|---|---|
| State | New |
| Headers | show |
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?
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?
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.
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,
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.
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 } {