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.
(What's wrong with this quoting? Well, what if there are double quotes in the
path? Better to sidestep the issue altogether.)
Jordan
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
