Hi John, this looks like a great start. Regarding documentation, we currently have no documentation about how to actually set up and use the experimental modules functionality. It is my understanding that you are working on setting up modules, and it would be great if you could share how to compile a "hello world" of modules. Currently there is a complete lack (AFAIK) of concrete information about how to use modules (e.g. what command line options to use?), yet folks are *dying* to be able to try out the new module functionality and some documentation regarding how to do that. Of course, more people banging on it will bring more bug reports, suggestions, and overall improvements.
I'm not sure why Doug initially used stdio.h for IO instead of raw_ostream, but for consistency with the rest of the codebase it would probably be best to switch over to using raw_osteam. Instead of directly using getcwd(), can you use llvm::sys::fs::current_path()? <http://llvm.org/docs/doxygen/html/namespacellvm_1_1sys_1_1fs.html#ac7b1d477269428b188cef6585fe6fdf5>. I think that should allow removing the platform-specific includes and #ifdefs, which are generally frowned upon <http://llvm.org/docs/SystemLibrary.html#don-t-include-system-headers>. > Note that I wasn’t sure if I should add something to the extra/doc directory, > but I will if you indicate such. I think we can wait for the tool to mature a bit before doing this. For now, I think it would be sufficient if you beefed up the file-level comment with a micro-manpage detailing the arguments and their meaning. One thing you may want to consider is how to test this tool. It would be nice if the initial check-in also laid the foundation for testing in clang-tools-extra/test/, so that we can make sure things don't regress. It would also serve as a basic form of developer documentation, which is appropriate at the moment since this is currently not a user-facing tool. -- Sean Silva _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
