[PATCH][Review request][analyzer] scan-build for windows

[Jordan Rose]

On Apr 26, 2013, at 19:46 , Anton Yartsev <anton.yartsev at gmail.com> wrote:

> On 27.04.2013 5:07, Jordan Rose wrote:
>> 
>> On Apr 26, 2013, at 11:26 , Anton Yartsev <anton.yartsev at gmail.com> 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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130426/cc5d1b20/attachment.html>