fix passing long argument lists to the linker, PR 15171, take 2

Rafael EspĂ­ndola rafael.espindola at gmail.com
Mon May 6 13:41:10 PDT 2013


Reverse ping :-)

I noticed you guys changed your build to use -flielist on OS X. Have
you lost interest in this patch then? If so, we should probably drop
the llvm bits too.

On 16 April 2013 10:20, Rafael EspĂ­ndola <rafael.espindola at gmail.com> wrote:
> On 15 April 2013 14:10, Nathan Froyd <nfroyd at mozilla.com> wrote:
>> Hi!
>>
>> This patch addresses PR 15171 by checking if we're constructing a
>> too-long argument list for the linker and writing it to a response file
>> if the linker supports it.
>
> * Please start function names with a lowercase letter.
> * The llvm::sys::Path object is only used to delete the file, so you
> can just say:
>
>   llvm::sys::Path(TmpPath).eraseFromDisk();
>
> * You don't need to pass the '.' to D.GetTemporaryPath. It was
> creating a file ending with "..tmp".
> * Don't use braces for single line blocks.
>
> MakeArgString Can take a Twine, so you don't need the FileArg temporary.
>
>>  A test case is included that should test the
>> newly-added behavior on at least some Windows checkouts (depending on
>> where the checkout lives on disk).  It doesn't appear that there's any
>> tests that clang parses a response file, so this test takes care of that
>> too.
>
> For the testcase I had something like the attached file in mind. By
> using the preprocessor we can create a large response file. Testing
> this on OS X showed that clang does try to use a response file, but
> the OS X linker doesn't support it. You should probably pass false to
> SupportsResponseFiles when calling from OS X and xfail the test for OS
> X.
>
>
>> Tested on linux x86-64.
>
> Since this is very host dependent, you should probably also test on
> windows and OS X at least.
>
>> -Nathan
>>
>
> Cheers,
> Rafael




More information about the cfe-commits mailing list