On Apr 26, 2013, at 19:46 , Anton Yartsev <[email protected]> wrote:
> On 27.04.2013 5:07, Jordan Rose wrote: >> >> On Apr 26, 2013, at 11:26 , Anton Yartsev <[email protected]> wrote: >> >>> On 22.04.2013 23:51, Jordan Rose wrote: >>>> Better MinGW support is certainly welcome! A few comments, though... >>>> >>>> >>>> ================ >>>> Comment at: tools/scan-build/scan-build:1563 >>>> @@ -1562,3 +1562,3 @@ >>>> $ClangCXX = $Clang; >>>> -$ClangCXX =~ s/\-\d+\.\d+$//; >>>> -$ClangCXX .= "++"; >>>> +if($^O =~/msys/) { >>>> + $ClangCXX =~ s/.exe$/++.exe/; >>>> ---------------- >>>> Not being a native Perl hacker, I found this a bit confusing. `$OSNAME` >>>> would have been a bit friendlier. >>>> >>>> Also, this check //only// handles MinGW, correct? >>>> http://perldoc.perl.org/perlvar.html#General-Variables says that native >>>> Windows returns "MSWin32" here. >>>> >>>> ================ >>>> Comment at: tools/scan-build/c++-analyzer.win:3 >>>> @@ +2,2 @@ >>>> + >>>> +system ("ccc-analyzer @ARGV"); >>>> ---------------- >>>> The reason we have c++-analyzer at all is so that we can set run the >>>> underlying compilation with a C++ compiler. Simply forwarding >>>> c++-analyzer.win to ccc-analyzer won't actually accomplish that -- you'll >>>> have to add a flag or enviroment variable check to ccc-analyzer, which >>>> c++-analyzer.win can set before invoking ccc-analyzer. >>>> >>>> Also, are you sure this won't destroy arguments with spaces in them? (You >>>> can test with something like `"-DSTR=macro with spaces"`.) >>>> >>>> >>>> http://llvm-reviews.chandlerc.com/D703 >>> Thanx for the review, all of your fears were confirmed. >>> Attached is a new patch. >>> >>> Temporary excluded phabricator from recipients. >> >> This doesn't actually fix either problem nicely. ;-) The advantage of a >> symlink over a wrapper script is that the indirection cost is tiny...at >> least on platforms where that trick works. That said, it does seem silly to >> have both a c++-analyzer symlink and a c++-analyzer.win wrapper script. How >> about moving the bulk of ccc-analyzer into a separate .pm file, which you >> can then load from ccc-analyzer and c++-analyzer? That solves the quoting >> problem, too. > Great idea! I'll try to implement it this way. > >> >> (What's wrong with this quoting? Well, what if there are double quotes in >> the path? Better to sidestep the issue altogether.) > Arguments, when passed to or from @ARGV, are seemed to be splitted by quotes, > if any, with qoutes removal, or by spaces otherwise: > > ------- script1.pl: > foreach my $arg (@ARGV) { > print "Argument to script1.pl: $arg\n"; > } > print "\n"; > system ("perl script2.pl @ARGV"); > > ------- script2.pl: > foreach my $arg (@ARGV) { > print "Argument to script2.pl: $arg\n"; > } > > ------- command and output with single quotes: > perl script1.pl -PARAM1 "a b" -PARAM2 c > > Argument to script1.pl: -PARAM1 > Argument to script1.pl: a b > Argument to script1.pl: -PARAM2 > Argument to script1.pl: c > > Argument to script2.pl: -PARAM1 > Argument to script2.pl: a > Argument to script2.pl: b > Argument to script2.pl: -PARAM2 > Argument to script2.pl: c > > > ------- command and output with double quotes: > perl script1.pl -PARAM1 ""a b"" > > Argument to script1.pl: -PARAM1 > Argument to script1.pl: a > Argument to script1.pl: b > > Argument to script2.pl: -PARAM1 > Argument to script2.pl: a > Argument to script2.pl: b > > > ------- command and output with mixed double quotes: > perl script1.pl -PARAM1 '"a b"' > > Argument to script1.pl: -PARAM1 > Argument to script1.pl: "a b" > > Argument to script2.pl: -PARAM1 > Argument to script2.pl: a b At least in a shell you can do this: % perl script1.pl "a \" b" which leads to this: Argument to script1.pl: a " b sh: -c: line 0: unexpected EOF while looking for matching `"' sh: -c: line 1: syntax error: unexpected end of file ;-) Jordan
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
