This time, with the patch. From: Thompson, John Sent: Friday, March 22, 2013 3:52 PM To: 'Vane, Edwin'; Sean Silva Cc: [email protected] Subject: RE: modularize tests - please review
Thanks again for the feedback. In the enclosed patch I've reworked how modularize handles arguments, switching it to use the CommandLine.h API, and adding the file path prefixing we discussed. I tried using CommonOptionParser.h but the CommandLine.h parser seemed to be unhappy about the ConsumeAfter option I added, probably because of the positional argument that CommonOptionParser defines, so I think I did the next best thing, which is to use the underlying CommanLine.h stuff. Are there other standard tool options I should be handling? The above change allowed me to remove the "cd" from the tests. I'm a newbie with respect to the types and string handling in LLVM, so perhaps you could check if I did reasonable things with the paths and so forth in the Modularize.cpp changes. I tested this with a separate build tree on both Windows and Linux. Thanks. -John From: Vane, Edwin [mailto:[email protected]] Sent: Friday, March 22, 2013 1:07 PM To: Sean Silva; Thompson, John Cc: [email protected]<mailto:[email protected]> Subject: RE: modularize tests - please review Sorry it took me so long to get to this review. I agree with Sean regarding how the headers should be located relative to the header-list file. Using clang::HeaderSearch is probably overkill. From: [email protected]<mailto:[email protected]> [mailto:[email protected]] On Behalf Of Sean Silva Sent: Thursday, March 21, 2013 6:30 PM To: Thompson, John Cc: [email protected]<mailto:[email protected]> Subject: Re: modularize tests - please review On Wed, Mar 20, 2013 at 9:26 PM, Thompson, John <[email protected]<mailto:[email protected]>> wrote: Sean, I tried using -I to fix the path problem, but modularize doesn't use the include path when trying to read the files from the source input list. That could be changed, but it's probably better to let it remain explicit where the sources come from. Perhaps I could change modularize to look for the files relative to the list file, or add a separate option for specifying the directory to use for a reference, or perhaps both. I think it makes sense to look for the files relative to the list file, with a commandline option to override that behavior. -- Sean Silva
modularize_tests_3.patch
Description: modularize_tests_3.patch
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
