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]
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
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to