Hi Sean and Rafael,
Thanks for your feedback. I answered your comments below. I will also upload a
patch now, both on the LLVM and on the Clang side, addressing Rafael's concerns
regarding the streams and his virtual removal suggestion.
Regarding the testing, I had some difficulty in figuring out good tests for the
response files mechanism. Personally, to ensure that the mechanism works fine,
I change needsResponseFile to always return true and use clang in several
regular scenarios to check that the response files work.
Without this hack, to test it, I need to truly generate a very big response
file, which involves exercising the compilation of a large project (especially
to test -filelist in ld64, which cannot be tested with our -DTEST -DTEST...
approach). I guess this kind of test is more appropriate to the test suite
(compiling big projects).
One thing I can think of now is to create a new flag "-force-response-file" for
testing purposes. Is this reasonable?
Best regards,
Rafael Auler
================
Comment at: include/clang/Driver/Job.h:103
@@ +102,3 @@
+ bool useResponseFile() const {
+ return ResponseFile != nullptr && NeedsResponseFile;
+ }
----------------
silvas wrote:
> Is there ever any situation where NeedsResponseFile == true but ResponseFile
> != nullptr? In general, what is the meaning of the 4 different possibilities
> for
>
> (ResponseFile {=,!}= nullptr) x (NeedsResponseFile {=,!}= true)
In particular, we use this to protect against a user of this class that does
not call setResponseFile() even if we return true in needsResponseFile(). If
the user did not configure a response file, ResponseFile will be a nullptr and
we need to disable resp file usage.
On the other hand, if the user *did* call setResponseFile() but we don't need
it (NeedsResponseFile = false), we should also disable response file usage.
================
Comment at: test/Driver/response-file.c:17
@@ +16,3 @@
+// limit.
+// RUN: awk "BEGIN { while (count++<300000) string=string \"-DTEST \";\
+// RUN: print string }" > %t.1.txt
----------------
rafael wrote:
> Please don't introduce an use of awk :-)
>
> Can you use clang -E to produce a large file on the fly? Is 2MB really too
> big for current linux systems?
>
I'm no fan of awk, but I had a small problem with this test. Previously, I used
a shell "for" loop to generate a large response file on the fly, but this made
this test unavailable to windows machines without bash.
To make this work on Windows, I looked at what LLVM required from its Windows
users to correctly run tests, and I saw that it requires that the user installs
the gnuwin32 package with many tools, including awk. Awk was the only tool that
I could think of where I could implement a loop to synthesise this large
response file on the fly, making this test work both on unix and on windows
machines.
Sorry but I'm not sure if I understood what you said about using clang -E to
produce a large file on the fly. If you are asking whether clang -E uses
response files, the idea here -- and it is actually based on a post that you
wrote a longe time ago, giving ideas to test response files :-) -- is to pass
many -D params to the driver, which, in turn, will be forced to use a response
file when calling "clang -cc1 -E".
Yes, 2MB is enough to trigger response files in current linux systems. However,
I can make the file larger, but the time of this test will also rise (a lot,
depending on the size and if clang was compiled with -O0). The bottleneck is in
Clang's parameter processing logic, which is stressed in this test.
http://reviews.llvm.org/D4897
_______________________________________________
cfe-commits mailing list
cfe-commits@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
